From 462ada88ab0828a9b0181b07030d4572ca766dae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 25 Oct 2019 13:02:20 +0000 Subject: [PATCH] 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 | 44 ++++++++++++++++++- docshell/base/BaseHistory.h | 42 +++++++++--------- .../components/geckoview/GeckoViewHistory.cpp | 27 +----------- .../components/geckoview/GeckoViewHistory.h | 1 - toolkit/components/places/History.cpp | 40 ----------------- toolkit/components/places/History.h | 1 - 6 files changed, 64 insertions(+), 91 deletions(-) diff --git a/docshell/base/BaseHistory.cpp b/docshell/base/BaseHistory.cpp index 0cb2c28c5691..0630d08e7d18 100644 --- a/docshell/base/BaseHistory.cpp +++ b/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 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 diff --git a/docshell/base/BaseHistory.h b/docshell/base/BaseHistory.h index 322a9114d458..b326713cace0 100644 --- a/docshell/base/BaseHistory.h +++ b/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 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 mTrackedURIs; }; -} // namespace mozilla +} // namespace mozilla #endif diff --git a/mobile/android/components/geckoview/GeckoViewHistory.cpp b/mobile/android/components/geckoview/GeckoViewHistory.cpp index d768496e2b2d..6e5d5028e1a4 100644 --- a/mobile/android/components/geckoview/GeckoViewHistory.cpp +++ b/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 seen; - nsTObserverArray::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))); diff --git a/mobile/android/components/geckoview/GeckoViewHistory.h b/mobile/android/components/geckoview/GeckoViewHistory.h index 067182e819b2..0243b7fb0094 100644 --- a/mobile/android/components/geckoview/GeckoViewHistory.h +++ b/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 StartVisitedQuery(nsIURI*) final; diff --git a/toolkit/components/places/History.cpp b/toolkit/components/places/History.cpp index 8101c31aef4d..48d7dbee2141 100644 --- a/toolkit/components/places/History.cpp +++ b/toolkit/components/places/History.cpp @@ -1479,46 +1479,6 @@ void History::NotifyVisitedParent(const nsTArray& 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 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 diff --git a/toolkit/components/places/History.h b/toolkit/components/places/History.h index be225fec18c9..9b1fbab58f7f 100644 --- a/toolkit/components/places/History.h +++ b/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 StartVisitedQuery(nsIURI*) final;