Bug 1590816 - Move NotifyVisited to BaseHistory. r=mak,lina

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

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

docshell/base/BaseHistory.cpp

@@ -101,7 +101,7 @@ BaseHistory::RegisterVisitedCallback(nsIURI* aURI, Link* aLink) {
     return NS_OK;
   }
 
-  TrackedURI& trackedURI = entry.OrInsert([] { return TrackedURI {}; });
+  TrackedURI& trackedURI = entry.OrInsert([] { return TrackedURI{}; });
 
   // Sanity check that Links are not registered more than once for a given URI.
   // This will not catch a case where it is registered for two different URIs.
@@ -155,4 +155,44 @@ BaseHistory::UnregisterVisitedCallback(nsIURI* aURI, Link* aLink) {
   return NS_OK;
 }
 
-} // namespace mozilla
+NS_IMETHODIMP
+BaseHistory::NotifyVisited(nsIURI* aURI) {
+  MOZ_ASSERT(NS_IsMainThread());
+  NS_ENSURE_ARG(aURI);
+
+  // NOTE: This can be run within the SystemGroup, and thus cannot directly
+  // interact with webpages.
+  nsAutoScriptBlocker scriptBlocker;
+
+  auto entry = mTrackedURIs.Lookup(aURI);
+  if (!entry) {
+    // If we have no observers for this URI, we have nothing to notify about.
+    return NS_OK;
+  }
+
+  TrackedURI& trackedURI = entry.Data();
+  trackedURI.mVisited = true;
+
+  // If we have a key, it should have at least one observer.
+  MOZ_ASSERT(!trackedURI.mLinks.IsEmpty());
+
+  // Dispatch an event to each document which has a Link observing this URL.
+  // These will fire asynchronously in the correct DocGroup.
+
+  // FIXME(emilio): Maybe a hashtable for this? An array could be bad.
+  nsTArray<Document*> seen;  // Don't dispatch duplicate runnables.
+  ObserverArray::BackwardIterator iter(trackedURI.mLinks);
+  while (iter.HasMore()) {
+    Link* link = iter.GetNext();
+    Document* doc = GetLinkDocument(*link);
+    if (seen.Contains(doc)) {
+      continue;
+    }
+    seen.AppendElement(doc);
+    DispatchNotifyVisited(aURI, doc);
+  }
+
+  return NS_OK;
+}
+
+}  // namespace mozilla

docshell/base/BaseHistory.h

@@ -13,28 +13,15 @@
 namespace mozilla {
 
 class BaseHistory : public IHistory {
+ public:
+  NS_IMETHODIMP RegisterVisitedCallback(nsIURI*, dom::Link*) final;
+  NS_IMETHODIMP UnregisterVisitedCallback(nsIURI*, dom::Link*) final;
+  NS_IMETHODIMP NotifyVisited(nsIURI* aURI) final;
+
  protected:
   static constexpr const size_t kTrackedUrisInitialSize = 64;
 
-  BaseHistory()
-    : mTrackedURIs(kTrackedUrisInitialSize) {}
-
-  /**
-   * Mark all links for the given URI in the given document as visited. Used
-   * within NotifyVisited.
-   */
-  void NotifyVisitedForDocument(nsIURI*, dom::Document*);
-
-  /**
-   * Dispatch a runnable for the document passed in which will call
-   * NotifyVisitedForDocument with the correct URI and Document.
-   */
-  void DispatchNotifyVisited(nsIURI*, dom::Document*);
-
-  // We implement the link tracking ourselves, and delegate to our subclasses by
-  // using StartTrackingURI and StopTrackingURI
-  NS_IMETHODIMP RegisterVisitedCallback(nsIURI*, dom::Link*) final;
-  NS_IMETHODIMP UnregisterVisitedCallback(nsIURI*, dom::Link*) final;
+  BaseHistory() : mTrackedURIs(kTrackedUrisInitialSize) {}
 
   // Starts a visited query, that eventually could call NotifyVisited if
   // appropriate.
@@ -56,10 +43,23 @@ class BaseHistory : public IHistory {
     }
   };
 
-  nsDataHashtable<nsURIHashKey, TrackedURI> mTrackedURIs;
+ private:
+  /**
+   * Mark all links for the given URI in the given document as visited. Used
+   * within NotifyVisited.
+   */
+  void NotifyVisitedForDocument(nsIURI*, dom::Document*);
 
+  /**
+   * Dispatch a runnable for the document passed in which will call
+   * NotifyVisitedForDocument with the correct URI and Document.
+   */
+  void DispatchNotifyVisited(nsIURI*, dom::Document*);
+
+ protected:
+  nsDataHashtable<nsURIHashKey, TrackedURI> mTrackedURIs;
 };
 
-} // namespace mozilla
+}  // namespace mozilla
 
 #endif

mobile/android/components/geckoview/GeckoViewHistory.cpp

@@ -397,31 +397,6 @@ GeckoViewHistory::SetURITitle(nsIURI* aURI, const nsAString& aTitle) {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
-NS_IMETHODIMP
-GeckoViewHistory::NotifyVisited(nsIURI* aURI) {
-  if (NS_WARN_IF(!aURI)) {
-    return NS_OK;
-  }
-
-  if (auto entry = mTrackedURIs.Lookup(aURI)) {
-    TrackedURI& trackedURI = entry.Data();
-    trackedURI.mVisited = true;
-    nsTArray<Document*> seen;
-    nsTObserverArray<Link*>::BackwardIterator iter(trackedURI.mLinks);
-    while (iter.HasMore()) {
-      Link* link = iter.GetNext();
-      Document* doc = GetLinkDocument(*link);
-      if (seen.Contains(doc)) {
-        continue;
-      }
-      seen.AppendElement(doc);
-      DispatchNotifyVisited(aURI, doc);
-    }
-  }
-
-  return NS_OK;
-}
-
 /**
  * Called from the session handler for the history delegate, with visited
  * statuses for all requested URIs.
@@ -592,7 +567,7 @@ void GeckoViewHistory::HandleVisitedState(
 
   // We might still have child processes even if e10s is disabled, so always
   // check if we're tracking any links in the parent, and notify them if so.
-  if (mTrackedURIs.Count() > 0) {
+  if (!mTrackedURIs.IsEmpty()) {
     for (const VisitedURI& visitedURI : aVisitedURIs) {
       if (visitedURI.mVisited) {
         Unused << NS_WARN_IF(NS_FAILED(NotifyVisited(visitedURI.mURI)));

mobile/android/components/geckoview/GeckoViewHistory.h

@@ -40,7 +40,6 @@ class GeckoViewHistory final : public mozilla::BaseHistory,
   NS_IMETHOD VisitURI(nsIWidget*, nsIURI*, nsIURI* aLastVisitedURI,
                       uint32_t aFlags) final;
   NS_IMETHOD SetURITitle(nsIURI*, const nsAString&) final;
-  NS_IMETHOD NotifyVisited(nsIURI*) override;
 
   // BaseHistory
   mozilla::Result<mozilla::Ok, nsresult> StartVisitedQuery(nsIURI*) final;

toolkit/components/places/History.cpp

@@ -1479,46 +1479,6 @@ void History::NotifyVisitedParent(const nsTArray<URIParams>& aURIs) {
   }
 }
 
-NS_IMETHODIMP
-History::NotifyVisited(nsIURI* aURI) {
-  MOZ_ASSERT(NS_IsMainThread());
-  NS_ENSURE_ARG(aURI);
-  // NOTE: This can be run within the SystemGroup, and thus cannot directly
-  // interact with webpages.
-
-  nsAutoScriptBlocker scriptBlocker;
-
-  auto entry = mTrackedURIs.Lookup(aURI);
-  if (!entry) {
-    // If we have no observers for this URI, we have nothing to notify about.
-    return NS_OK;
-  }
-
-  TrackedURI& trackedURI = entry.Data();
-  trackedURI.mVisited = true;
-
-  // If we have a key, it should have at least one observer.
-  MOZ_ASSERT(!trackedURI.mLinks.IsEmpty());
-
-  // Dispatch an event to each document which has a Link observing this URL.
-  // These will fire asynchronously in the correct DocGroup.
-
-  // FIXME(emilio): Maybe a hashtable for this? An array could be bad.
-  nsTArray<Document*> seen;  // Don't dispatch duplicate runnables.
-  ObserverArray::BackwardIterator iter(trackedURI.mLinks);
-  while (iter.HasMore()) {
-    Link* link = iter.GetNext();
-    Document* doc = GetLinkDocument(*link);
-    if (seen.Contains(doc)) {
-      continue;
-    }
-    seen.AppendElement(doc);
-    DispatchNotifyVisited(aURI, doc);
-  }
-
-  return NS_OK;
-}
-
 class ConcurrentStatementsHolder final : public mozIStorageCompletionCallback {
  public:
   NS_DECL_ISUPPORTS

toolkit/components/places/History.h

@@ -58,7 +58,6 @@ class History final : public BaseHistory,
   NS_IMETHOD VisitURI(nsIWidget*, nsIURI*, nsIURI* aLastVisitedURI,
                       uint32_t aFlags) final;
   NS_IMETHOD SetURITitle(nsIURI*, const nsAString&) final;
-  NS_IMETHOD NotifyVisited(nsIURI*) override;
 
   // BaseHistory
   Result<Ok, nsresult> StartVisitedQuery(nsIURI*) final;