Bug 1590816 - Various naming and comment cleanups, as suggested in review comments. r=mak

Depends on D50266

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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Emilio Cobos Álvarez 2019-10-25 13:02:23 +00:00
parent a4a8b9ded2
commit ea222cccf7
3 changed files with 68 additions and 66 deletions

View File

@ -44,12 +44,12 @@ void BaseHistory::NotifyVisitedForDocument(nsIURI* aURI, dom::Document* aDoc) {
return;
}
TrackedURI& trackedURI = entry.Data();
ObservingLinks& links = entry.Data();
{
// Update status of each Link node. We iterate over the array backwards so
// we can remove the items as we encounter them.
ObserverArray::BackwardIterator iter(trackedURI.mLinks);
ObserverArray::BackwardIterator iter(links.mLinks);
while (iter.HasMore()) {
Link* link = iter.GetNext();
if (GetLinkDocument(*link) == aDoc) {
@ -60,7 +60,7 @@ void BaseHistory::NotifyVisitedForDocument(nsIURI* aURI, dom::Document* aDoc) {
}
// If we don't have any links left, we can remove the array.
if (trackedURI.mLinks.IsEmpty()) {
if (links.mLinks.IsEmpty()) {
entry.Remove();
}
}
@ -101,20 +101,20 @@ BaseHistory::RegisterVisitedCallback(nsIURI* aURI, Link* aLink) {
return NS_OK;
}
TrackedURI& trackedURI = entry.OrInsert([] { return TrackedURI{}; });
ObservingLinks& links = entry.OrInsert([] { return ObservingLinks{}; });
// 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),
MOZ_DIAGNOSTIC_ASSERT(!links.mLinks.Contains(aLink),
"Already tracking this Link object!");
// Start tracking our Link.
trackedURI.mLinks.AppendElement(aLink);
links.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) {
if (links.mKnownVisited) {
DispatchNotifyVisited(aURI, GetLinkDocument(*aLink));
}
@ -170,18 +170,18 @@ BaseHistory::NotifyVisited(nsIURI* aURI) {
return NS_OK;
}
TrackedURI& trackedURI = entry.Data();
trackedURI.mVisited = true;
ObservingLinks& links = entry.Data();
links.mKnownVisited = true;
// If we have a key, it should have at least one observer.
MOZ_ASSERT(!trackedURI.mLinks.IsEmpty());
MOZ_ASSERT(!links.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.
// TODO(bug 1591090): Maybe a hashtable for this? An array could be bad.
nsTArray<Document*> seen; // Don't dispatch duplicate runnables.
ObserverArray::BackwardIterator iter(trackedURI.mLinks);
ObserverArray::BackwardIterator iter(links.mLinks);
while (iter.HasMore()) {
Link* link = iter.GetNext();
Document* doc = GetLinkDocument(*link);

View File

@ -32,9 +32,9 @@ class BaseHistory : public IHistory {
virtual void CancelVisitedQueryIfPossible(nsIURI*) = 0;
using ObserverArray = nsTObserverArray<dom::Link*>;
struct TrackedURI {
struct ObservingLinks {
ObserverArray mLinks;
bool mVisited = false;
bool mKnownVisited = false;
size_t SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const {
return mLinks.ShallowSizeOfExcludingThis(aMallocSizeOf);
@ -55,7 +55,13 @@ class BaseHistory : public IHistory {
void DispatchNotifyVisited(nsIURI*, dom::Document*);
protected:
nsDataHashtable<nsURIHashKey, TrackedURI> mTrackedURIs;
// A map from URI to links that depend on that URI, and whether that URI is
// known-to-be-visited already.
//
// FIXME(emilio, bug 1506842): The known-to-be-visited stuff seems it could
// cause timeable differences, probably we should store whether it's
// known-to-be-unvisited too.
nsDataHashtable<nsURIHashKey, ObservingLinks> mTrackedURIs;
};
} // namespace mozilla

View File

@ -96,40 +96,38 @@ void GeckoViewHistory::QueryVisitedStateInContentProcess() {
newURIsIter.Next()) {
nsIURI* uri = newURIsIter.Get()->GetKey();
if (auto entry = mTrackedURIs.Lookup(uri)) {
TrackedURI& trackedURI = entry.Data();
if (!trackedURI.mLinks.IsEmpty()) {
nsTObserverArray<Link*>::BackwardIterator linksIter(trackedURI.mLinks);
while (linksIter.HasMore()) {
Link* link = linksIter.GetNext();
ObservingLinks& links = entry.Data();
nsTObserverArray<Link*>::BackwardIterator linksIter(links.mLinks);
while (linksIter.HasMore()) {
Link* link = linksIter.GetNext();
BrowserChild* browserChild = nullptr;
nsIWidget* widget =
nsContentUtils::WidgetForContent(link->GetElement());
if (widget) {
browserChild = widget->GetOwningBrowserChild();
}
if (!browserChild) {
// We need the link's tab child to find the matching window in the
// parent process, so stop tracking it if it doesn't have one.
linksIter.Remove();
continue;
}
BrowserChild* browserChild = nullptr;
nsIWidget* widget =
nsContentUtils::WidgetForContent(link->GetElement());
if (widget) {
browserChild = widget->GetOwningBrowserChild();
}
if (!browserChild) {
// We need the link's tab child to find the matching window in the
// parent process, so stop tracking it if it doesn't have one.
linksIter.Remove();
continue;
}
// Add to the list of new URIs for this document, or make a new entry.
bool hasEntry = false;
for (NewURIEntry& entry : newEntries) {
if (entry.mBrowserChild == browserChild) {
entry.AddURI(uri);
hasEntry = true;
break;
}
}
if (!hasEntry) {
newEntries.AppendElement(NewURIEntry(browserChild, uri));
// Add to the list of new URIs for this document, or make a new entry.
bool hasEntry = false;
for (NewURIEntry& entry : newEntries) {
if (entry.mBrowserChild == browserChild) {
entry.AddURI(uri);
hasEntry = true;
break;
}
}
if (!hasEntry) {
newEntries.AppendElement(NewURIEntry(browserChild, uri));
}
}
if (trackedURI.mLinks.IsEmpty()) {
if (links.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.
@ -170,33 +168,31 @@ void GeckoViewHistory::QueryVisitedStateInParentProcess() {
newURIsIter.Next()) {
nsIURI* uri = newURIsIter.Get()->GetKey();
if (auto entry = mTrackedURIs.Lookup(uri)) {
TrackedURI& trackedURI = entry.Data();
if (!trackedURI.mLinks.IsEmpty()) {
nsTObserverArray<Link*>::BackwardIterator linksIter(trackedURI.mLinks);
while (linksIter.HasMore()) {
Link* link = linksIter.GetNext();
ObservingLinks& links = entry.Data();
nsTObserverArray<Link*>::BackwardIterator linksIter(links.mLinks);
while (linksIter.HasMore()) {
Link* link = linksIter.GetNext();
nsIWidget* widget =
nsContentUtils::WidgetForContent(link->GetElement());
if (!widget) {
linksIter.Remove();
continue;
}
nsIWidget* widget =
nsContentUtils::WidgetForContent(link->GetElement());
if (!widget) {
linksIter.Remove();
continue;
}
bool hasEntry = false;
for (NewURIEntry& entry : newEntries) {
if (entry.mWidget == widget) {
entry.AddURI(uri);
hasEntry = true;
break;
}
}
if (!hasEntry) {
newEntries.AppendElement(NewURIEntry(widget, uri));
bool hasEntry = false;
for (NewURIEntry& entry : newEntries) {
if (entry.mWidget == widget) {
entry.AddURI(uri);
hasEntry = true;
break;
}
}
if (!hasEntry) {
newEntries.AppendElement(NewURIEntry(widget, uri));
}
}
if (trackedURI.mLinks.IsEmpty()) {
if (links.mLinks.IsEmpty()) {
entry.Remove();
}
}