Bug 1332770 - Fix the google4 provider is showing up as other in some telemetry pings. r=francois

MozReview-Commit-ID: KbpYAnf6qxd

--HG--
extra : rebase_source : fe524ca5afbd1b8b08b80198eeb43db5f84f677e

toolkit/components/url-classifier/nsUrlClassifierDBService.cpp

@@ -631,8 +631,11 @@ nsUrlClassifierDBServiceWorker::FinishUpdate()
      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();
 
@@ -989,7 +992,7 @@ nsUrlClassifierLookupCallback::LookupComplete(nsTArray<LookupResult>* results)
       // 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))) {
 

toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp

@@ -24,6 +24,7 @@
 #include "mozilla/Telemetry.h"
 #include "nsContentUtils.h"
 #include "nsIURLFormatter.h"
+#include "Classifier.h"
 
 static const char* gQuitApplicationMessage = "quit-application";
 
@@ -97,6 +98,7 @@ nsUrlClassifierStreamUpdater::DownloadDone()
   mSuccessCallback = nullptr;
   mUpdateErrorCallback = nullptr;
   mDownloadErrorCallback = nullptr;
+  mTelemetryProvider.Truncate();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -283,6 +285,14 @@ nsUrlClassifierStreamUpdater::DownloadUpdates(
     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;
@@ -635,17 +645,11 @@ nsUrlClassifierStreamUpdater::OnStartRequest(nsIRequest *request,
            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;
@@ -656,7 +660,7 @@ nsUrlClassifierStreamUpdater::OnStartRequest(nsIRequest *request,
       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) {

toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.h

@@ -73,7 +73,10 @@ private:
   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;
@@ -98,6 +101,10 @@ private:
   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_

toolkit/components/url-classifier/nsUrlClassifierUtils.cpp

@@ -279,7 +279,9 @@ nsUrlClassifierUtils::GetProvider(const nsACString& aTableName,
 {
   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();
@@ -296,7 +298,8 @@ nsUrlClassifierUtils::GetTelemetryProvider(const nsACString& aTableName,
   // 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"));
   }
 

toolkit/components/url-classifier/nsUrlClassifierUtils.h

@@ -10,6 +10,8 @@
 #include "nsClassHashtable.h"
 #include "nsIObserver.h"
 
+#define TESTING_TABLE_PROVIDER_NAME "test"
+
 class nsUrlClassifierUtils final : public nsIUrlClassifierUtils,
                                    public nsIObserver
 {