Backed out changeset c77c46ac90a5 (bug 1552536) by xeonchen's request
authorBogdan Tara <btara@mozilla.com>
Mon, 20 May 2019 17:49:27 +0300
changeset 474532 82b9eaa4679754eb3ff38e6472a3d9f77600affa
parent 474531 6519e35004283dc67e83ce49090480c724fac5bf
child 474533 36ada63cb9fb9ff56e7556cfc1db8cfad2c46a2d
push id36042
push userdvarga@mozilla.com
push dateTue, 21 May 2019 04:19:40 +0000
treeherdermozilla-central@ca560ff55451 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1552536
milestone69.0a1
backs outc77c46ac90a55891e1f67ff18fb16029883cc0e6
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset c77c46ac90a5 (bug 1552536) by xeonchen's request
dom/base/ContentBlockingLog.cpp
toolkit/components/telemetry/core/TelemetryOriginData.inc
toolkit/components/telemetry/tests/gtest/TestOrigins.cpp
--- a/dom/base/ContentBlockingLog.cpp
+++ b/dom/base/ContentBlockingLog.cpp
@@ -16,19 +16,16 @@ static LazyLogModule gContentBlockingLog
 #define LOG(fmt, ...) \
   MOZ_LOG(gContentBlockingLog, mozilla::LogLevel::Debug, (fmt, ##__VA_ARGS__))
 
 typedef mozilla::Telemetry::OriginMetricID OriginMetricID;
 
 namespace mozilla {
 namespace dom {
 
-// sync with TelemetryOriginData.inc
-NS_NAMED_LITERAL_CSTRING(kDummyOriginHash, "PAGELOAD");
-
 // randomly choose 1% users included in the content blocking measurement
 // based on their client id.
 static constexpr double kRatioReportUser = 0.01;
 
 // randomly choose 0.14% documents when the page is unload.
 static constexpr double kRatioReportDocument = 0.0014;
 
 static bool IsReportingPerUserEnabled() {
@@ -109,40 +106,37 @@ static void ReportOriginSingleHash(Origi
 
 void ContentBlockingLog::ReportLog() {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (!IsReportingEnabled()) {
     return;
   }
   LOG("ContentBlockingLog::ReportLog [this=%p]", this);
-  const bool testMode =
-      StaticPrefs::telemetry_origin_telemetry_test_mode_enabled();
-  OriginMetricID metricId =
-      testMode ? OriginMetricID::ContentBlocking_Blocked_TestOnly
-               : OriginMetricID::ContentBlocking_Blocked;
-  ReportOriginSingleHash(metricId, kDummyOriginHash);
 
   for (const auto& originEntry : mLog) {
     if (!originEntry.mData) {
       continue;
     }
 
     for (const auto& logEntry : Reversed(originEntry.mData->mLogs)) {
       if (logEntry.mType !=
               nsIWebProgressListener::STATE_COOKIES_BLOCKED_TRACKER ||
           logEntry.mTrackingFullHashes.IsEmpty()) {
         continue;
       }
 
       const bool isBlocked = logEntry.mBlocked;
       Maybe<StorageAccessGrantedReason> reason = logEntry.mReason;
+      const bool testMode =
+          StaticPrefs::telemetry_origin_telemetry_test_mode_enabled();
 
-      metricId = testMode ? OriginMetricID::ContentBlocking_Blocked_TestOnly
-                          : OriginMetricID::ContentBlocking_Blocked;
+      OriginMetricID metricId =
+          testMode ? OriginMetricID::ContentBlocking_Blocked_TestOnly
+                   : OriginMetricID::ContentBlocking_Blocked;
       if (!isBlocked) {
         MOZ_ASSERT(reason.isSome());
         switch (reason.value()) {
           case StorageAccessGrantedReason::eStorageAccessAPI:
             metricId =
                 testMode
                     ? OriginMetricID::
                           ContentBlocking_StorageAccessAPIExempt_TestOnly
--- a/toolkit/components/telemetry/core/TelemetryOriginData.inc
+++ b/toolkit/components/telemetry/core/TelemetryOriginData.inc
@@ -1,11 +1,8 @@
-// dummy origin
-ORIGIN("PAGELOAD", "PAGELOAD")
-
 ORIGIN("advertstream.com", "lzPiT1FuoHNMKQ1Hw8AaTi68TokOB24ciFBqmCk62ek=")
 ORIGIN("kitaramedia.com", "r+U9PL3uMrjCKe8/T8goY9MHPA+6JckC3R+/1R9TQKA=")
 ORIGIN("questionmarket.com", "3KCO/qN+KmApmfH3RaXAmdR65Z/TRfrr6pds7aDKn1c=")
 ORIGIN("3lift.com", "33Xrix7c41Jc9q3InjMWHq+yKVoa/u2IB511kr4X+Ro=")
 ORIGIN("affine.tv", "wKqJRMTORbe8f4TO5LTGaTlt4yZVKlyomX2Exvfcv6A=")
 ORIGIN("longtailvideo.com", "B0tCrpMgOedsm2p2cLJcHrITf/amX5cxB+90PrEpJ8c=")
 ORIGIN("undertonenetworks.com", "uaBhUaQhVEMpY9MctAg2QNOszcD6VUq2Gkf44GOp3tA=")
 ORIGIN("rhythmnewmedia.com", "PmyV0h3/zQb41TFz6eMHhmHA7KoTtMCSiFWjhQhu22A=")
--- a/toolkit/components/telemetry/tests/gtest/TestOrigins.cpp
+++ b/toolkit/components/telemetry/tests/gtest/TestOrigins.cpp
@@ -15,60 +15,42 @@
 
 using namespace mozilla;
 using namespace TelemetryTestHelpers;
 using mozilla::Telemetry::OriginMetricID;
 using ::testing::_;
 using ::testing::AtLeast;
 using ::testing::StrEq;
 
-NS_NAMED_LITERAL_CSTRING(kTelemetryTest1Metric, "telemetry.test_test1");
-
-NS_NAMED_LITERAL_CSTRING(kDummyOrigin, "PAGELOAD");
-NS_NAMED_LITERAL_CSTRING(kDoubleclickOrigin, "doubleclick.net");
-NS_NAMED_LITERAL_CSTRING(kDoubleclickOriginHash,
-                         "uXNT1PzjAVau8b402OMAIGDejKbiXfQX5iXvPASfO/s=");
-NS_NAMED_LITERAL_CSTRING(kFacebookOrigin, "fb.com");
-NS_NAMED_LITERAL_CSTRING(kUnknownOrigin1,
-                         "this origin isn't known to Origin Telemetry");
-NS_NAMED_LITERAL_CSTRING(kUnknownOrigin2, "neither is this one");
-
-// Properly prepare the prio prefs
-// (Sourced from PrioEncoder.cpp from when it was being prototyped)
-NS_NAMED_LITERAL_CSTRING(
-    prioKeyA,
-    "35AC1C7576C7C6EDD7FED6BCFC337B34D48CB4EE45C86BEEFB40BD8875707733");
-NS_NAMED_LITERAL_CSTRING(
-    prioKeyB,
-    "26E6674E65425B823F1F1D5F96E3BB3EF9E406EC7FBA7DEF8B08A35DD135AF50");
-
 // Test that we can properly record origin stuff using the C++ API.
 TEST_F(TelemetryTestFixture, RecordOrigin) {
   AutoJSContextWithGlobal cx(mCleanGlobal);
   JSContext* aCx = cx.GetJSContext();
 
   Unused << mTelemetry->ClearOrigins();
 
-  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1, kDummyOrigin);
+  const nsLiteralCString doubleclick("doubleclick.net");
+  const nsLiteralCString telemetryTest1("telemetry.test_test1");
+
+  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1, doubleclick);
 
   JS::RootedValue originSnapshot(aCx);
   GetOriginSnapshot(aCx, &originSnapshot);
 
   ASSERT_FALSE(originSnapshot.isNullOrUndefined())
   << "Origin snapshot must not be null/undefined.";
 
   JS::RootedValue origins(aCx);
   JS::RootedObject snapshotObj(aCx, &originSnapshot.toObject());
-  ASSERT_TRUE(
-      JS_GetProperty(aCx, snapshotObj, kTelemetryTest1Metric.get(), &origins))
+  ASSERT_TRUE(JS_GetProperty(aCx, snapshotObj, telemetryTest1.get(), &origins))
   << "telemetry.test_test1 must be in the snapshot.";
 
   JS::RootedObject originsObj(aCx, &origins.toObject());
   JS::RootedValue count(aCx);
-  ASSERT_TRUE(JS_GetProperty(aCx, originsObj, kDummyOrigin.get(), &count));
+  ASSERT_TRUE(JS_GetProperty(aCx, originsObj, doubleclick.get(), &count));
   ASSERT_TRUE(count.isInt32() && count.toInt32() == 1)
   << "Must have recorded the origin exactly once.";
 
   // Now test that the snapshot didn't clear things out.
   GetOriginSnapshot(aCx, &originSnapshot);
   ASSERT_FALSE(originSnapshot.isNullOrUndefined());
   JS::RootedObject unemptySnapshotObj(aCx, &originSnapshot.toObject());
   JS::Rooted<JS::IdVector> ids(aCx, JS::IdVector(aCx));
@@ -77,37 +59,36 @@ TEST_F(TelemetryTestFixture, RecordOrigi
 }
 
 TEST_F(TelemetryTestFixture, RecordOriginTwiceAndClear) {
   AutoJSContextWithGlobal cx(mCleanGlobal);
   JSContext* aCx = cx.GetJSContext();
 
   Unused << mTelemetry->ClearOrigins();
 
-  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1,
-                          kDoubleclickOrigin);
-  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1,
-                          kDoubleclickOrigin);
+  const nsLiteralCString doubleclick("doubleclick.net");
+  const nsLiteralCString telemetryTest1("telemetry.test_test1");
+
+  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1, doubleclick);
+  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1, doubleclick);
 
   JS::RootedValue originSnapshot(aCx);
   GetOriginSnapshot(aCx, &originSnapshot, true /* aClear */);
 
   ASSERT_FALSE(originSnapshot.isNullOrUndefined())
   << "Origin snapshot must not be null/undefined.";
 
   JS::RootedValue origins(aCx);
   JS::RootedObject snapshotObj(aCx, &originSnapshot.toObject());
-  ASSERT_TRUE(
-      JS_GetProperty(aCx, snapshotObj, kTelemetryTest1Metric.get(), &origins))
+  ASSERT_TRUE(JS_GetProperty(aCx, snapshotObj, telemetryTest1.get(), &origins))
   << "telemetry.test_test1 must be in the snapshot.";
 
   JS::RootedObject originsObj(aCx, &origins.toObject());
   JS::RootedValue count(aCx);
-  ASSERT_TRUE(
-      JS_GetProperty(aCx, originsObj, kDoubleclickOrigin.get(), &count));
+  ASSERT_TRUE(JS_GetProperty(aCx, originsObj, doubleclick.get(), &count));
   ASSERT_TRUE(count.isInt32() && count.toInt32() == 2)
   << "Must have recorded the origin exactly twice.";
 
   // Now check that snapshotting with clear actually cleared it.
   GetOriginSnapshot(aCx, &originSnapshot);
   ASSERT_FALSE(originSnapshot.isNullOrUndefined());
   JS::RootedObject emptySnapshotObj(aCx, &originSnapshot.toObject());
   JS::Rooted<JS::IdVector> ids(aCx, JS::IdVector(aCx));
@@ -116,117 +97,135 @@ TEST_F(TelemetryTestFixture, RecordOrigi
 }
 
 TEST_F(TelemetryTestFixture, RecordOriginTwiceMixed) {
   AutoJSContextWithGlobal cx(mCleanGlobal);
   JSContext* aCx = cx.GetJSContext();
 
   Unused << mTelemetry->ClearOrigins();
 
-  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1,
-                          kDoubleclickOrigin);
-  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1,
-                          kDoubleclickOriginHash);
+  const nsLiteralCString doubleclick("doubleclick.net");
+  const nsLiteralCString doubleclickHash(
+      "uXNT1PzjAVau8b402OMAIGDejKbiXfQX5iXvPASfO/s=");
+  const nsLiteralCString telemetryTest1("telemetry.test_test1");
 
+  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1, doubleclick);
+  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1, doubleclickHash);
+
+  // Properly prepare the prio prefs
+  // (Sourced from PrioEncoder.cpp from when it was being prototyped)
+  const nsLiteralCString prioKeyA(
+      "35AC1C7576C7C6EDD7FED6BCFC337B34D48CB4EE45C86BEEFB40BD8875707733");
+  const nsLiteralCString prioKeyB(
+      "26E6674E65425B823F1F1D5F96E3BB3EF9E406EC7FBA7DEF8B08A35DD135AF50");
   Preferences::SetCString("prio.publicKeyA", prioKeyA);
   Preferences::SetCString("prio.publicKeyB", prioKeyB);
 
   nsTArray<Tuple<nsCString, nsCString>> encodedStrings;
-  GetEncodedOriginStrings(
-      aCx, kTelemetryTest1Metric + NS_LITERAL_CSTRING("-%u"), encodedStrings);
+  GetEncodedOriginStrings(aCx, telemetryTest1 + NS_LITERAL_CSTRING("-%u"),
+                          encodedStrings);
   ASSERT_EQ(2 * TelemetryOrigin::SizeOfPrioDatasPerMetric(),
             encodedStrings.Length());
 
   JS::RootedValue originSnapshot(aCx);
   GetOriginSnapshot(aCx, &originSnapshot, true /* aClear */);
 
   ASSERT_FALSE(originSnapshot.isNullOrUndefined())
   << "Origin snapshot must not be null/undefined.";
 
   JS::RootedValue origins(aCx);
   JS::RootedObject snapshotObj(aCx, &originSnapshot.toObject());
-  ASSERT_TRUE(
-      JS_GetProperty(aCx, snapshotObj, kTelemetryTest1Metric.get(), &origins))
+  ASSERT_TRUE(JS_GetProperty(aCx, snapshotObj, telemetryTest1.get(), &origins))
   << "telemetry.test_test1 must be in the snapshot.";
 
   JS::RootedObject originsObj(aCx, &origins.toObject());
   JS::RootedValue count(aCx);
-  ASSERT_TRUE(
-      JS_GetProperty(aCx, originsObj, kDoubleclickOrigin.get(), &count));
+  ASSERT_TRUE(JS_GetProperty(aCx, originsObj, doubleclick.get(), &count));
   ASSERT_TRUE(count.isInt32() && count.toInt32() == 2)
   << "Must have recorded the origin exactly twice.";
 }
 
 TEST_F(TelemetryTestFixture, RecordUnknownOrigin) {
   AutoJSContextWithGlobal cx(mCleanGlobal);
   JSContext* aCx = cx.GetJSContext();
 
   Unused << mTelemetry->ClearOrigins();
 
-  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1, kUnknownOrigin1);
+  const nsLiteralCString telemetryTest1("telemetry.test_test1");
+  const nsLiteralCString unknown("this origin isn't known to Origin Telemetry");
+  const nsLiteralCString unknown2("neither is this one");
+
+  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1, unknown);
 
   JS::RootedValue originSnapshot(aCx);
   GetOriginSnapshot(aCx, &originSnapshot);
 
   ASSERT_FALSE(originSnapshot.isNullOrUndefined())
   << "Origin snapshot must not be null/undefined.";
 
   JS::RootedValue origins(aCx);
   JS::RootedObject snapshotObj(aCx, &originSnapshot.toObject());
-  ASSERT_TRUE(
-      JS_GetProperty(aCx, snapshotObj, kTelemetryTest1Metric.get(), &origins))
+  ASSERT_TRUE(JS_GetProperty(aCx, snapshotObj, telemetryTest1.get(), &origins))
   << "telemetry.test_test1 must be in the snapshot.";
 
   JS::RootedObject originsObj(aCx, &origins.toObject());
   JS::RootedValue count(aCx);
   ASSERT_TRUE(JS_GetProperty(aCx, originsObj, "__UNKNOWN__", &count));
   ASSERT_TRUE(count.isInt32() && count.toInt32() == 1)
   << "Must have recorded the unknown origin exactly once.";
 
   // Record a second, different unknown origin and ensure only one is stored.
-  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1, kUnknownOrigin2);
+  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1, unknown2);
 
   GetOriginSnapshot(aCx, &originSnapshot);
 
   ASSERT_FALSE(originSnapshot.isNullOrUndefined())
   << "Origin snapshot must not be null/undefined.";
 
   JS::RootedObject snapshotObj2(aCx, &originSnapshot.toObject());
-  ASSERT_TRUE(
-      JS_GetProperty(aCx, snapshotObj2, kTelemetryTest1Metric.get(), &origins))
+  ASSERT_TRUE(JS_GetProperty(aCx, snapshotObj2, telemetryTest1.get(), &origins))
   << "telemetry.test_test1 must be in the snapshot.";
 
   JS::RootedObject originsObj2(aCx, &origins.toObject());
   JS::RootedValue count2(aCx);
   ASSERT_TRUE(JS_GetProperty(aCx, originsObj2, "__UNKNOWN__", &count2));
   ASSERT_TRUE(count2.isInt32() && count2.toInt32() == 1)
   << "Must have recorded the unknown origin exactly once.";
 }
 
 TEST_F(TelemetryTestFixture, EncodedSnapshot) {
   AutoJSContextWithGlobal cx(mCleanGlobal);
   JSContext* aCx = cx.GetJSContext();
 
   Unused << mTelemetry->ClearOrigins();
 
-  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1,
-                          kDoubleclickOrigin);
-  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1, kUnknownOrigin1);
+  const nsLiteralCString doubleclick("doubleclick.net");
+  const nsLiteralCString unknown("this origin isn't known to Origin Telemetry");
+  const nsLiteralCString telemetryTest1("telemetry.test_test1");
+
+  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1, doubleclick);
+  Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1, unknown);
 
+  // Properly prepare the prio prefs
+  // (Sourced from PrioEncoder.cpp from when it was being prototyped)
+  const nsLiteralCString prioKeyA(
+      "35AC1C7576C7C6EDD7FED6BCFC337B34D48CB4EE45C86BEEFB40BD8875707733");
+  const nsLiteralCString prioKeyB(
+      "26E6674E65425B823F1F1D5F96E3BB3EF9E406EC7FBA7DEF8B08A35DD135AF50");
   Preferences::SetCString("prio.publicKeyA", prioKeyA);
   Preferences::SetCString("prio.publicKeyB", prioKeyB);
 
   nsTArray<Tuple<nsCString, nsCString>> firstStrings;
-  GetEncodedOriginStrings(
-      aCx, kTelemetryTest1Metric + NS_LITERAL_CSTRING("-%u"), firstStrings);
+  GetEncodedOriginStrings(aCx, telemetryTest1 + NS_LITERAL_CSTRING("-%u"),
+                          firstStrings);
 
   // Now snapshot a second time and ensure the encoded payloads change.
   nsTArray<Tuple<nsCString, nsCString>> secondStrings;
-  GetEncodedOriginStrings(
-      aCx, kTelemetryTest1Metric + NS_LITERAL_CSTRING("-%u"), secondStrings);
+  GetEncodedOriginStrings(aCx, telemetryTest1 + NS_LITERAL_CSTRING("-%u"),
+                          secondStrings);
 
   const auto sizeOfPrioDatasPerMetric =
       TelemetryOrigin::SizeOfPrioDatasPerMetric();
   ASSERT_EQ(sizeOfPrioDatasPerMetric, firstStrings.Length());
   ASSERT_EQ(sizeOfPrioDatasPerMetric, secondStrings.Length());
 
   for (size_t i = 0; i < sizeOfPrioDatasPerMetric; ++i) {
     auto& aStr = Get<0>(firstStrings[i]);
@@ -261,30 +260,30 @@ class MockObserver final : public nsIObs
 };
 
 NS_IMPL_ISUPPORTS(MockObserver, nsIObserver);
 
 TEST_F(TelemetryTestFixture, OriginTelemetryNotifiesTopic) {
   Unused << mTelemetry->ClearOrigins();
 
   const char* kTopic = "origin-telemetry-storage-limit-reached";
+  NS_NAMED_LITERAL_CSTRING(doubleclick, "doubleclick.net");
+  NS_NAMED_LITERAL_CSTRING(fb, "fb.com");
 
   MockObserver* mo = new MockObserver();
   nsCOMPtr<nsIObserver> nsMo(mo);
   EXPECT_CALL(*mo, Mobserve(StrEq(kTopic))).Times(1);
 
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
   ASSERT_TRUE(os);
   os->AddObserver(nsMo, kTopic, false);
 
   const size_t size = ceil(10.0 / TelemetryOrigin::SizeOfPrioDatasPerMetric());
   for (size_t i = 0; i < size; ++i) {
     if (i < size - 1) {
       // Let's ensure we only notify the once.
-      Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1,
-                              kFacebookOrigin);
+      Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1, fb);
     }
-    Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1,
-                            kDoubleclickOrigin);
+    Telemetry::RecordOrigin(OriginMetricID::TelemetryTest_Test1, doubleclick);
   }
 
   os->RemoveObserver(nsMo, kTopic);
 }