Bug 1590816 - Move Register and UnregisterVisitedCallback to BaseHistory. r=mak,lina

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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Emilio Cobos Álvarez 2019-10-25 13:02:18 +00:00
parent 7c0ed40aed
commit b1cb605f8c
7 changed files with 138 additions and 163 deletions

View File

@ -65,5 +65,94 @@ void BaseHistory::NotifyVisitedForDocument(nsIURI* aURI, dom::Document* aDoc) {
}
}
NS_IMETHODIMP
BaseHistory::RegisterVisitedCallback(nsIURI* aURI, Link* aLink) {
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(aURI, "Must pass a non-null URI!");
if (XRE_IsContentProcess()) {
MOZ_ASSERT(aLink, "Must pass a non-null Link!");
}
// Obtain our array of observers for this URI.
auto entry = mTrackedURIs.LookupForAdd(aURI);
MOZ_DIAGNOSTIC_ASSERT(!entry || !entry.Data().mLinks.IsEmpty(),
"An empty key was kept around in our hashtable!");
if (!entry) {
// This is the first request for this URI, thus we must query its visited
// state.
auto result = StartVisitedQuery(aURI);
if (result.isErr()) {
entry.OrRemove();
return result.unwrapErr();
}
}
if (!aLink) {
// In IPC builds, we are passed a nullptr Link from
// ContentParent::RecvStartVisitedQuery. All of our code after this point
// assumes aLink is non-nullptr, so we have to return now.
MOZ_DIAGNOSTIC_ASSERT(XRE_IsParentProcess(),
"We should only ever get a null Link "
"in the parent process!");
// We don't want to remove if we're tracking other links.
if (!entry) {
entry.OrRemove();
}
return NS_OK;
}
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.
MOZ_DIAGNOSTIC_ASSERT(!trackedURI.mLinks.Contains(aLink),
"Already tracking this Link object!");
// Start tracking our Link.
trackedURI.mLinks.AppendElement(aLink);
// If this link has already been visited, we cannot synchronously mark
// ourselves as visited, so instead we fire a runnable into our docgroup,
// which will handle it for us.
if (trackedURI.mVisited) {
DispatchNotifyVisited(aURI, GetLinkDocument(*aLink));
}
return NS_OK;
}
NS_IMETHODIMP
BaseHistory::UnregisterVisitedCallback(nsIURI* aURI, Link* aLink) {
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(aURI, "Must pass a non-null URI!");
MOZ_ASSERT(aLink, "Must pass a non-null Link object!");
// Get the array, and remove the item from it.
auto entry = mTrackedURIs.Lookup(aURI);
if (!entry) {
// GeckoViewHistory sometimes, for somewhat dubious reasons, removes links
// from mTrackedURIs.
#ifndef MOZ_WIDGET_ANDROID
MOZ_ASSERT_UNREACHABLE("Trying to unregister URI that wasn't registered!");
#endif
return NS_ERROR_UNEXPECTED;
}
ObserverArray& observers = entry.Data().mLinks;
if (!observers.RemoveElement(aLink)) {
#ifndef MOZ_WIDGET_ANDROID
MOZ_ASSERT_UNREACHABLE("Trying to unregister node that wasn't registered!");
#endif
return NS_ERROR_UNEXPECTED;
}
// If the array is now empty, we should remove it from the hashtable.
if (observers.IsEmpty()) {
entry.Remove();
CancelVisitedQueryIfPossible(aURI);
}
return NS_OK;
}
} // namespace mozilla

View File

@ -6,6 +6,7 @@
#define mozilla_BaseHistory_h
#include "IHistory.h"
#include "mozilla/Result.h"
/* A base class for history implementations that implement link coloring. */
@ -22,8 +23,7 @@ class BaseHistory : public IHistory {
* Mark all links for the given URI in the given document as visited. Used
* within NotifyVisited.
*/
void NotifyVisitedForDocument(nsIURI*,
dom::Document*);
void NotifyVisitedForDocument(nsIURI*, dom::Document*);
/**
* Dispatch a runnable for the document passed in which will call
@ -31,6 +31,18 @@ class BaseHistory : public IHistory {
*/
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;
// Starts a visited query, that eventually could call NotifyVisited if
// appropriate.
virtual Result<Ok, nsresult> StartVisitedQuery(nsIURI*) = 0;
// Cancels a visited query, if it is at all possible, because we know we won't
// use the results anymore.
virtual void CancelVisitedQueryIfPossible(nsIURI*) = 0;
static dom::Document* GetLinkDocument(dom::Link&);

View File

@ -142,16 +142,6 @@ class IHistory : public nsISupports {
NS_DEFINE_STATIC_IID_ACCESSOR(IHistory, IHISTORY_IID)
#define NS_DECL_IHISTORY \
NS_IMETHOD RegisterVisitedCallback(nsIURI* aURI, \
mozilla::dom::Link* aContent) override; \
NS_IMETHOD UnregisterVisitedCallback(nsIURI* aURI, \
mozilla::dom::Link* aContent) override; \
NS_IMETHOD VisitURI(nsIWidget* aWidget, nsIURI* aURI, \
nsIURI* aLastVisitedURI, uint32_t aFlags) override; \
NS_IMETHOD SetURITitle(nsIURI* aURI, const nsAString& aTitle) override; \
NS_IMETHOD NotifyVisited(nsIURI* aURI) override;
} // namespace mozilla
#endif // mozilla_IHistory_h_

View File

@ -10,6 +10,7 @@
#include "nsXULAppAPI.h"
#include "mozilla/ClearOnShutdown.h"
#include "mozilla/ResultExtensions.h"
#include "mozilla/dom/ContentParent.h"
#include "mozilla/dom/Element.h"
@ -222,75 +223,18 @@ GeckoViewHistory::Notify(nsITimer* aTimer) {
return NS_OK;
}
NS_IMETHODIMP
GeckoViewHistory::RegisterVisitedCallback(nsIURI* aURI, Link* aLink) {
if (!aLink || !aURI) {
return NS_OK;
Result<Ok, nsresult> GeckoViewHistory::StartVisitedQuery(nsIURI* aURI) {
mNewURIs.PutEntry(aURI);
if (!mQueryVisitedStateTimer) {
mQueryVisitedStateTimer = NS_NewTimer();
}
auto entry = mTrackedURIs.LookupForAdd(aURI);
if (entry) {
// Start tracking the link for this URI.
TrackedURI& trackedURI = entry.Data();
trackedURI.mLinks.AppendElement(aLink);
if (trackedURI.mVisited) {
// If we already know that the URI was visited, update the link state now.
DispatchNotifyVisited(aURI, GetLinkDocument(*aLink));
}
} else {
// Otherwise, track the link, and start the timer to request the visited
// status from the history delegate for this and any other new URIs. If the
// delegate reports that the URI is unvisited, we'll keep tracking the link,
// and update its state from `VisitedCallback` once it's visited. If the URI
// is already visited, `GetVisitedCallback` will update this and all other
// visited links, and stop tracking them.
entry.OrInsert([aLink]() {
TrackedURI trackedURI;
trackedURI.mLinks.AppendElement(aLink);
return trackedURI;
});
mNewURIs.PutEntry(aURI);
if (!mQueryVisitedStateTimer) {
mQueryVisitedStateTimer = NS_NewTimer();
}
Unused << NS_WARN_IF(NS_FAILED(mQueryVisitedStateTimer->InitWithCallback(
this, GET_VISITS_WAIT_MS, nsITimer::TYPE_ONE_SHOT)));
}
return NS_OK;
// FIXME(emilio): Do we really want to initialize the timer all the time?
return ToResult(mQueryVisitedStateTimer->InitWithCallback(
this, GET_VISITS_WAIT_MS, nsITimer::TYPE_ONE_SHOT));
}
NS_IMETHODIMP
GeckoViewHistory::UnregisterVisitedCallback(nsIURI* aURI, Link* aLink) {
if (!aLink || !aURI) {
return NS_OK;
}
if (auto entry = mTrackedURIs.Lookup(aURI)) {
TrackedURI& trackedURI = entry.Data();
if (!trackedURI.mLinks.IsEmpty()) {
nsTObserverArray<Link*>::BackwardIterator iter(trackedURI.mLinks);
while (iter.HasMore()) {
Link* link = iter.GetNext();
if (link == aLink) {
iter.Remove();
break;
}
}
}
if (trackedURI.mLinks.IsEmpty()) {
// If the list of tracked links is empty, remove the entry for the URI.
// We'll need to query the history delegate again the next time we look
// up the visited status for this URI.
entry.Remove();
}
}
void GeckoViewHistory::CancelVisitedQueryIfPossible(nsIURI* aURI) {
mNewURIs.RemoveEntry(aURI);
return NS_OK;
}
/**

View File

@ -33,10 +33,19 @@ class GeckoViewHistory final : public mozilla::BaseHistory,
public nsINamed {
public:
NS_DECL_ISUPPORTS
NS_DECL_IHISTORY
NS_DECL_NSITIMERCALLBACK
NS_DECL_NSINAMED
// IHistory
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;
void CancelVisitedQueryIfPossible(nsIURI*) final;
static already_AddRefed<GeckoViewHistory> GetSingleton();
GeckoViewHistory();

View File

@ -1932,6 +1932,10 @@ void History::AppendToRecentlyVisitedURIs(nsIURI* aURI, bool aHidden) {
}
}
Result<Ok, nsresult> History::StartVisitedQuery(nsIURI* aURI) {
return ToResult(VisitedQuery::Start(aURI));
}
////////////////////////////////////////////////////////////////////////////////
//// IHistory
@ -2068,90 +2072,6 @@ History::VisitURI(nsIWidget* aWidget, nsIURI* aURI, nsIURI* aLastVisitedURI,
return NS_OK;
}
NS_IMETHODIMP
History::RegisterVisitedCallback(nsIURI* aURI, Link* aLink) {
MOZ_ASSERT(NS_IsMainThread());
NS_ASSERTION(aURI, "Must pass a non-null URI!");
if (XRE_IsContentProcess()) {
MOZ_ASSERT(aLink, "Must pass a non-null Link!");
}
// Obtain our array of observers for this URI.
auto entry = mTrackedURIs.LookupForAdd(aURI);
MOZ_DIAGNOSTIC_ASSERT(!entry || !entry.Data().mLinks.IsEmpty(),
"An empty key was kept around in our hashtable!");
if (!entry) {
// We are the first Link node to ask about this URI, or there are no pending
// Links wanting to know about this URI. Therefore, we should query the
// database now.
nsresult rv = VisitedQuery::Start(aURI);
// In IPC builds, we are passed a nullptr Link from
// ContentParent::RecvStartVisitedQuery. Since we won't be adding a nullptr
// entry to our list of observers, and the code after this point assumes
// that aLink is non-nullptr, we will need to return now.
if (NS_FAILED(rv) || !aLink) {
entry.OrRemove();
return rv;
}
}
// In IPC builds, we are passed a nullptr Link from
// ContentParent::RecvStartVisitedQuery. All of our code after this point
// assumes aLink is non-nullptr, so we have to return now.
else if (!aLink) {
MOZ_DIAGNOSTIC_ASSERT(XRE_IsParentProcess(),
"We should only ever get a null Link "
"in the parent process!");
return NS_OK;
}
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.
MOZ_DIAGNOSTIC_ASSERT(!trackedURI.mLinks.Contains(aLink),
"Already tracking this Link object!");
// Start tracking our Link.
trackedURI.mLinks.AppendElement(aLink);
// If this link has already been visited, we cannot synchronously mark
// ourselves as visited, so instead we fire a runnable into our docgroup,
// which will handle it for us.
if (trackedURI.mVisited) {
DispatchNotifyVisited(aURI, GetLinkDocument(*aLink));
}
return NS_OK;
}
NS_IMETHODIMP
History::UnregisterVisitedCallback(nsIURI* aURI, Link* aLink) {
MOZ_ASSERT(NS_IsMainThread());
// TODO: aURI is sometimes null - see bug 548685
NS_ASSERTION(aURI, "Must pass a non-null URI!");
NS_ASSERTION(aLink, "Must pass a non-null Link object!");
// Get the array, and remove the item from it.
auto entry = mTrackedURIs.Lookup(aURI);
if (!entry) {
MOZ_ASSERT_UNREACHABLE("Trying to unregister URI that wasn't registered!");
return NS_ERROR_UNEXPECTED;
}
ObserverArray& observers = entry.Data().mLinks;
if (!observers.RemoveElement(aLink)) {
MOZ_ASSERT_UNREACHABLE("Trying to unregister node that wasn't registered!");
return NS_ERROR_UNEXPECTED;
}
// If the array is now empty, we should remove it from the hashtable.
if (observers.IsEmpty()) {
entry.Remove();
}
return NS_OK;
}
NS_IMETHODIMP
History::SetURITitle(nsIURI* aURI, const nsAString& aTitle) {
MOZ_ASSERT(NS_IsMainThread());

View File

@ -50,11 +50,22 @@ class History final : public BaseHistory,
public nsIMemoryReporter {
public:
NS_DECL_THREADSAFE_ISUPPORTS
NS_DECL_IHISTORY
NS_DECL_MOZIASYNCHISTORY
NS_DECL_NSIOBSERVER
NS_DECL_NSIMEMORYREPORTER
// IHistory
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;
void CancelVisitedQueryIfPossible(nsIURI*) final {
// TODO(bug 1591393): This could be worth it? Needs some measurement.
}
History();
/**