Right now the WindowGlobalParent only supports the documentURI but not anything around the visible URI. It would be great to also have the latter available, so eg the about:(.*)error prefix is not part of the URI.

As discussed on IRC @kmag suggested visibleURI.

Nika, how complicated would it be to get this implemented? We see a good amount of intermittent failures for Marionette tests because of this missing API. If no-one could work on it maybe you could explain what needs to be done? Thanks.

The current code here is doing document.defaultView.location.href, which in turn is getting the current URI for the page as returned from nsDocShell::GetCurrentURI. The code which maintains this state is kinda weird and sometimes hard to follow, so it might be a bit of work to come up with a clean solution to reflect it into the parent process.

The simplest solution here would be to reflect it into the parent process by sending a message from nsDocShell::SetCurrentURI, but we may want to somehow fuse it with the dispatching of the OnLocationChange web progress listener event in cases where we can do so, so that the current URI according to OnLocationChange callbacks, and the one recorded on the BrowsingContext don't get out-of-sync.

It might be worth doing some reading into the behaviour of this value to see if we can derive it in the parent process in a cleaner way than we do currently, as it generally should line up with the document URI with a few exceptions.

Tom, is this something you can work on?

(In reply to Nika Layzell [:nika] (ni? for response) from comment #2)

The simplest solution here would be to reflect it into the parent process by sending a message from nsDocShell::SetCurrentURI, but we may want to somehow fuse it with the dispatching of the OnLocationChange web progress listener event in cases where we can do so, so that the current URI according to OnLocationChange callbacks, and the one recorded on the BrowsingContext don't get out-of-sync.

Nika, could you elaborate more on "the one recorded on the BrowsingContext"? I am not sure if I understand that correctly. Thanks in advance!

Skimmed through some pieces of code, I found that we have BrowserChild::OnLocationChange and BrowserParent::RecvOnLocationChange (which calls GetBrowsingContext()->Top()->GetWebProgress()->OnLocationChange(). They are used to sync OnLocationChange event through the content and the parent processes.

I think I need to check if nsDocLoader::FireOnLocationChange which is called by nsDocShell::SetCurrentURI actually calls BrowserChild::OnLocationChange. If that is true, we can probably just set WindowGlobalParent::mVisableURI in BrowsingContextWebProgress::OnLocationChange on the parent process and thus the value won't be out-of-sync.

I think I need to check if nsDocLoader::FireOnLocationChange which is called by nsDocShell::SetCurrentURI actually calls BrowserChild::OnLocationChange. If that is true, we can probably just set WindowGlobalParent::mVisableURI in BrowsingContextWebProgress::OnLocationChange on the parent process and thus the value won't be out-of-sync.

BrowserChild adds itself into docshell's web progress listener so that it's guaranteed to be notified when nsDocShell::SetCurrentURI is called.

Taking this bug as I mention on the review as fixing it will require tweaks to our remote web progress infrastructure.

This change allows associating individual web progress events with which frame
they originate from, rather than only recording the isToplevel information as
we were before, which is necessary in order to use the OnLocationChange events
from content to track the current URI on CanonicalBrowsingContext.

Due to events in ContentChild being filtered through nsBrowserStatusFilter, some
of the callbacks will never be passed nsIRequest or nsIWebProgress pointers, and
this patch also simplifies them by removing information which is not necessary
from the IPC message.

Finally, this patch adds a number of checks to the relevant Recv callbacks in
the parent process which help ensure that it does not accept web progress events
from a content process which is no longer hosting the target BrowsingContext.
This may allow for us to stop manually suspending web progress events on process
switches, as these checks will handle this automatically based on the existing
BrowsingContext and WindowContext objects.

Previously, we would need to suspend progress events from the previous
BrowserParent, as otherwise we would receive STATE_STOP progress notifications
from the previous browser when it is destroyed, which would throw off frontend
code. With the new checks added by part 1, we will now catch these cases by
detecting that the current window global has changed, and we can get rid of this
explicit override.

Previously, we would only send web progress events from the toplevel
BrowserParent, as other frames would never have the browser-child.js framescript
loaded in them, and so would never start sending events. This change moves the
decision to begin sending events into BrowserChild itself around the same time
as it would've happened previously with the framescript.

This new callsite should still avoid sending events for the creation of the
initial about:blank document in the BrowserChild, while not skipping any other
events, as before.

This URI is intended to reflect the currentURI field on the content nsIDocShell,
and is the value used for getters like window.location.href. For most
documents, this generally matches the Document URI on WindowGlobalParent, it
does not match in specific cases such as error pages or when performing a
session restore.

The field is kept up-to-date by listening to OnLocationChange notifications
from the content process, and is ignored when the BrowsingContext is loaded in
the parent process.

This specifically tests the interesting case of loading an error page.

https://hg.mozilla.org/mozilla-central/rev/10a646d0c74a
https://hg.mozilla.org/mozilla-central/rev/1f0e13aecced
https://hg.mozilla.org/mozilla-central/rev/ce268e67dbcd
https://hg.mozilla.org/mozilla-central/rev/7560f4f2293f
https://hg.mozilla.org/mozilla-central/rev/8657e72a09f7