Bug 1332770 - Fix the google4 provider is showing up as other in some telemetry pings. r?francois
MozReview-Commit-ID: KbpYAnf6qxd
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -626,18 +626,21 @@ nsUrlClassifierDBServiceWorker::FinishUp
urlUtil->GetTelemetryProvider(mUpdateTables.SafeElementAt(0, EmptyCString()), provider);
nsresult updateStatus = mUpdateStatus;
if (NS_FAILED(mUpdateStatus)) {
updateStatus = NS_ERROR_GET_MODULE(mUpdateStatus) == NS_ERROR_MODULE_URL_CLASSIFIER ?
mUpdateStatus : NS_ERROR_UC_UPDATE_UNKNOWN;
}
- Telemetry::Accumulate(Telemetry::URLCLASSIFIER_UPDATE_ERROR, provider,
- NS_ERROR_GET_CODE(updateStatus));
+ // Do not record telemetry for testing tables.
+ if (!provider.Equals(TESTING_TABLE_PROVIDER_NAME)) {
+ Telemetry::Accumulate(Telemetry::URLCLASSIFIER_UPDATE_ERROR, provider,
+ NS_ERROR_GET_CODE(updateStatus));
+ }
mMissCache.Clear();
if (NS_SUCCEEDED(mUpdateStatus)) {
LOG(("Notifying success: %d", mUpdateWaitSec));
mUpdateObserver->UpdateSuccess(mUpdateWaitSec);
} else if (NS_ERROR_NOT_IMPLEMENTED == mUpdateStatus) {
LOG(("Treating NS_ERROR_NOT_IMPLEMENTED a successful update "
@@ -984,17 +987,17 @@ nsUrlClassifierLookupCallback::LookupCom
NS_ENSURE_SUCCESS(rv, rv);
LOG(("The match from %s needs to be completed at %s",
result.mTableName.get(), gethashUrl.get()));
// gethashUrls may be empty in 2 cases: test tables, and on startup where
// we may have found a prefix in an existing table before the listmanager
// has registered the table. In the second case we should not call
// complete.
if ((!gethashUrl.IsEmpty() ||
- StringBeginsWith(result.mTableName, NS_LITERAL_CSTRING("test-"))) &&
+ StringBeginsWith(result.mTableName, NS_LITERAL_CSTRING("test"))) &&
mDBService->GetCompleter(result.mTableName,
getter_AddRefs(completer))) {
// TODO: Figure out how long the partial hash should be sent
// for completion. See Bug 1323953.
nsAutoCString partialHash;
if (StringEndsWith(result.mTableName, NS_LITERAL_CSTRING("-proto"))) {
// We send the complete partial hash for v4 at the moment.
--- a/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
@@ -19,16 +19,17 @@
#include "mozilla/BasePrincipal.h"
#include "mozilla/ErrorNames.h"
#include "mozilla/Logging.h"
#include "nsIInterfaceRequestor.h"
#include "mozilla/LoadContext.h"
#include "mozilla/Telemetry.h"
#include "nsContentUtils.h"
#include "nsIURLFormatter.h"
+#include "Classifier.h"
static const char* gQuitApplicationMessage = "quit-application";
// Limit the list file size to 32mb
const uint32_t MAX_FILE_SIZE = (32 * 1024 * 1024);
#undef LOG
@@ -92,16 +93,17 @@ nsUrlClassifierStreamUpdater::DownloadDo
LOG(("nsUrlClassifierStreamUpdater::DownloadDone [this=%p]", this));
mIsUpdating = false;
mPendingUpdates.Clear();
mDownloadError = false;
mSuccessCallback = nullptr;
mUpdateErrorCallback = nullptr;
mDownloadErrorCallback = nullptr;
+ mTelemetryProvider.Truncate();
}
///////////////////////////////////////////////////////////////////////////////
// nsIUrlClassifierStreamUpdater implementation
nsresult
nsUrlClassifierStreamUpdater::FetchUpdate(nsIURI *aUpdateUrl,
const nsACString & aRequestPayload,
@@ -278,16 +280,24 @@ nsUrlClassifierStreamUpdater::DownloadUp
request->mDownloadErrorCallback = aDownloadErrorCallback;
return NS_OK;
}
if (NS_FAILED(rv)) {
return rv;
}
+ nsCOMPtr<nsIUrlClassifierUtils> urlUtil =
+ do_GetService(NS_URLCLASSIFIERUTILS_CONTRACTID);
+
+ nsTArray<nsCString> tables;
+ mozilla::safebrowsing::Classifier::SplitTables(aRequestTables, tables);
+ urlUtil->GetTelemetryProvider(tables.SafeElementAt(0, EmptyCString()),
+ mTelemetryProvider);
+
mSuccessCallback = aSuccessCallback;
mUpdateErrorCallback = aUpdateErrorCallback;
mDownloadErrorCallback = aDownloadErrorCallback;
mIsUpdating = true;
*_retval = true;
LOG(("FetchUpdate: %s", aUpdateUrl.Data()));
@@ -630,38 +640,32 @@ nsUrlClassifierStreamUpdater::OnStartReq
if (NS_SUCCEEDED(rv) && uri) {
uri->GetAsciiSpec(spec);
}
LOG(("nsUrlClassifierStreamUpdater::OnStartRequest "
"(status=%s, uri=%s, this=%p)", errorName.get(),
spec.get(), this));
}
- nsCOMPtr<nsIUrlClassifierUtils> urlUtil =
- do_GetService(NS_URLCLASSIFIERUTILS_CONTRACTID);
-
- nsCString provider;
- urlUtil->GetTelemetryProvider(mStreamTable, provider);
-
if (NS_FAILED(status)) {
// Assume we're overloading the server and trigger backoff.
downloadError = true;
mozilla::Telemetry::Accumulate(mozilla::Telemetry::URLCLASSIFIER_UPDATE_REMOTE_STATUS2,
- provider, 15 /* unknown response code */);
+ mTelemetryProvider, 15 /* unknown response code */);
} else {
bool succeeded = false;
rv = httpChannel->GetRequestSucceeded(&succeeded);
NS_ENSURE_SUCCESS(rv, rv);
uint32_t requestStatus;
rv = httpChannel->GetResponseStatus(&requestStatus);
NS_ENSURE_SUCCESS(rv, rv);
mozilla::Telemetry::Accumulate(mozilla::Telemetry::URLCLASSIFIER_UPDATE_REMOTE_STATUS2,
- provider, HTTPStatusToBucket(requestStatus));
+ mTelemetryProvider, HTTPStatusToBucket(requestStatus));
LOG(("nsUrlClassifierStreamUpdater::OnStartRequest %s (%d)", succeeded ?
"succeeded" : "failed", requestStatus));
if (!succeeded) {
// 404 or other error, pass error status back
strStatus.AppendInt(requestStatus);
downloadError = true;
}
}
--- a/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.h
@@ -68,17 +68,20 @@ private:
// Fetches the next request, from mPendingRequests
nsresult FetchNextRequest();
bool mIsUpdating;
bool mInitialized;
bool mDownloadError;
bool mBeganStream;
+
+ // Note that mStreamTable is only used by v2, it is empty for v4 update.
nsCString mStreamTable;
+
nsCOMPtr<nsIChannel> mChannel;
nsCOMPtr<nsIUrlClassifierDBService> mDBService;
nsCOMPtr<nsITimer> mTimer;
struct PendingRequest {
nsCString mTables;
nsCString mRequestPayload;
bool mIsPostRequest;
@@ -93,11 +96,15 @@ private:
nsCString mUrl;
nsCString mTable;
};
nsTArray<PendingUpdate> mPendingUpdates;
nsCOMPtr<nsIUrlClassifierCallback> mSuccessCallback;
nsCOMPtr<nsIUrlClassifierCallback> mUpdateErrorCallback;
nsCOMPtr<nsIUrlClassifierCallback> mDownloadErrorCallback;
+
+ // The provider for current update request and should be only used by telemetry
+ // since it would show up as "other" for any other providers.
+ nsCString mTelemetryProvider;
};
#endif // nsUrlClassifierStreamUpdater_h_
--- a/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp
@@ -274,34 +274,37 @@ nsUrlClassifierUtils::ConvertListNameToT
}
NS_IMETHODIMP
nsUrlClassifierUtils::GetProvider(const nsACString& aTableName,
nsACString& aProvider)
{
MutexAutoLock lock(mProviderDictLock);
nsCString* provider = nullptr;
- if (mProviderDict.Get(aTableName, &provider)) {
+ if (StringBeginsWith(aTableName, NS_LITERAL_CSTRING("test"))) {
+ aProvider = NS_LITERAL_CSTRING(TESTING_TABLE_PROVIDER_NAME);
+ } else if (mProviderDict.Get(aTableName, &provider)) {
aProvider = provider ? *provider : EmptyCString();
} else {
aProvider = EmptyCString();
}
return NS_OK;
}
NS_IMETHODIMP
nsUrlClassifierUtils::GetTelemetryProvider(const nsACString& aTableName,
nsACString& aProvider)
{
GetProvider(aTableName, aProvider);
// Filter out build-in providers: mozilla, google, google4
// Empty provider is filtered as "other"
if (!NS_LITERAL_CSTRING("mozilla").Equals(aProvider) &&
!NS_LITERAL_CSTRING("google").Equals(aProvider) &&
- !NS_LITERAL_CSTRING("google4").Equals(aProvider)) {
+ !NS_LITERAL_CSTRING("google4").Equals(aProvider) &&
+ !NS_LITERAL_CSTRING(TESTING_TABLE_PROVIDER_NAME).Equals(aProvider)) {
aProvider.Assign(NS_LITERAL_CSTRING("other"));
}
return NS_OK;
}
NS_IMETHODIMP
nsUrlClassifierUtils::GetProtocolVersion(const nsACString& aProvider,
--- a/toolkit/components/url-classifier/nsUrlClassifierUtils.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierUtils.h
@@ -5,16 +5,18 @@
#ifndef nsUrlClassifierUtils_h_
#define nsUrlClassifierUtils_h_
#include "nsAutoPtr.h"
#include "nsIUrlClassifierUtils.h"
#include "nsClassHashtable.h"
#include "nsIObserver.h"
+#define TESTING_TABLE_PROVIDER_NAME "test"
+
class nsUrlClassifierUtils final : public nsIUrlClassifierUtils,
public nsIObserver
{
public:
typedef nsClassHashtable<nsCStringHashKey, nsCString> ProviderDictType;
private:
/**