Bug 1583271 - Part 3: Remove the disabled assertion and add a mechanism to capture the first non-auto:blank frame r=nika

We are not simply excluding all about:blanks because there might be some
about:blank that user really visits. But for others we don't want to include
the first about:blank because when a BrowsingContext is loaded, and if the
principal matches, the first document loaded in it will share the inner window.

Differential Revision: https://phabricator.services.mozilla.com/D47067

--HG--
extra : moz-landing-system : lando

mozglue/baseprofiler/core/platform.cpp

@@ -350,17 +350,27 @@ class CorePS {
 
   static void AppendRegisteredPage(PSLockRef,
                                    RefPtr<PageInformation>&& aRegisteredPage) {
-    // Disabling this assertion for now until we fix the same page registration
-    // issue. See Bug 1542918.
-#  if 0
     struct RegisteredPageComparator {
       PageInformation* aA;
       bool operator()(PageInformation* aB) const { return aA->Equals(aB); }
     };
-    MOZ_RELEASE_ASSERT(std::none_of(
+
+    auto foundPageIter = std::find_if(
         sInstance->mRegisteredPages.begin(), sInstance->mRegisteredPages.end(),
-        RegisteredPageComparator{aRegisteredPage.get()}));
-#  endif
+        RegisteredPageComparator{aRegisteredPage.get()});
+
+    if (foundPageIter != sInstance->mRegisteredPages.end()) {
+      if ((*foundPageIter)->Url() == "about:blank") {
+        // When a BrowsingContext is loaded, the first url loaded in it will be
+        // about:blank, and if the principal matches, the first document loaded
+        // in it will share an inner window. That's why we should delete the
+        // intermittent about:blank if they share the inner window.
+        sInstance->mRegisteredPages.erase(foundPageIter);
+      } else {
+        // Do not register the same page again.
+        return;
+      }
+    }
     MOZ_RELEASE_ASSERT(
         sInstance->mRegisteredPages.append(std::move(aRegisteredPage)));
   }
@@ -3069,6 +3079,9 @@ void profiler_register_page(uint64_t aBrowsingContextID,
 
   PSAutoLock lock;
 
+  // When a Browsing context is first loaded, the first url loaded in it will be
+  // about:blank. Because of that, this call keeps the first non-about:blank
+  // registration of window and discards the previous one.
   RefPtr<PageInformation> pageInfo = new PageInformation(
       aBrowsingContextID, aInnerWindowID, aUrl, aIsSubFrame);
   CorePS::AppendRegisteredPage(lock, std::move(pageInfo));

mozglue/baseprofiler/public/BaseProfiler.h

@@ -329,7 +329,9 @@ MFBT_API void profiler_unregister_thread();
 //
 // We register pages for each navigations but we do not register
 // history.pushState or history.replaceState since they correspond to the same
-// Inner Window ID.
+// Inner Window ID. When a Browsing context is first loaded, the first url
+// loaded in it will be about:blank. Because of that, this call keeps the first
+// non-about:blank registration of window and discards the previous one.
 //
 //   "aBrowsingContextID" is the ID of the browsing context that document
 //                        belongs to. That's used to determine the tab of that

tools/profiler/core/platform.cpp

@@ -423,17 +423,28 @@ class CorePS {
 
   static void AppendRegisteredPage(PSLockRef,
                                    RefPtr<PageInformation>&& aRegisteredPage) {
-    // Disabling this assertion for now until we fix the same page registration
-    // issue. See Bug 1542918.
-#if 0
     struct RegisteredPageComparator {
       PageInformation* aA;
       bool operator()(PageInformation* aB) const { return aA->Equals(aB); }
     };
-    MOZ_RELEASE_ASSERT(std::none_of(
+
+    auto foundPageIter = std::find_if(
         sInstance->mRegisteredPages.begin(), sInstance->mRegisteredPages.end(),
-        RegisteredPageComparator{aRegisteredPage.get()}));
-#endif
+        RegisteredPageComparator{aRegisteredPage.get()});
+
+    if (foundPageIter != sInstance->mRegisteredPages.end()) {
+      if ((*foundPageIter)->Url().EqualsLiteral("about:blank")) {
+        // When a BrowsingContext is loaded, the first url loaded in it will be
+        // about:blank, and if the principal matches, the first document loaded
+        // in it will share an inner window. That's why we should delete the
+        // intermittent about:blank if they share the inner window.
+        sInstance->mRegisteredPages.erase(foundPageIter);
+      } else {
+        // Do not register the same page again.
+        return;
+      }
+    }
+
     MOZ_RELEASE_ASSERT(
         sInstance->mRegisteredPages.append(std::move(aRegisteredPage)));
   }
@@ -3994,6 +4005,9 @@ void profiler_register_page(uint64_t aBrowsingContextID,
 
   PSAutoLock lock(gPSMutex);
 
+  // When a Browsing context is first loaded, the first url loaded in it will be
+  // about:blank. Because of that, this call keeps the first non-about:blank
+  // registration of window and discards the previous one.
   RefPtr<PageInformation> pageInfo = new PageInformation(
       aBrowsingContextID, aInnerWindowID, aUrl, aIsSubFrame);
   CorePS::AppendRegisteredPage(lock, std::move(pageInfo));

tools/profiler/public/GeckoProfiler.h

@@ -363,7 +363,9 @@ void profiler_unregister_thread();
 //
 // We register pages for each navigations but we do not register
 // history.pushState or history.replaceState since they correspond to the same
-// Inner Window ID.
+// Inner Window ID. When a Browsing context is first loaded, the first url
+// loaded in it will be about:blank. Because of that, this call keeps the first
+// non-about:blank registration of window and discards the previous one.
 //
 //   "aBrowsingContextID" is the ID of the browsing context that document
 //                        belongs to. That's used to determine the tab of that