Bug 1170488 - browser.documentURI goes out of sync when doing same-document navigation in e10s
I suspect this is related to
bug 1173215
- Google Photos is, I believe, using history.pushState to manipulate
the browser history, and we're getting confused about what the current
URI is.
This is what's failing in browser-plugins.js's showClickToPlayNotification:
showClickToPlayNotification
:
function
(browser, plugins, showNow, principal,
host, location) {
// It is possible that we've received a message from the frame script to show
// a click to play notification for a principal that no longer matches the one
// that the browser's content now has assigned (ie, the browser has browsed away
// after the message was sent, but before the message was received). In that case,
// we should just ignore the message.
if ( ! principal.equals(browser.contentPrincipal)) {
return ;
}
// Data URIs, when linked to from some page, inherit the principal of that
// page. That means that we also need to compare the actual locations to
// ensure we aren't getting a message from a Data URI that we're no longer
// looking at.
let receivedURI = BrowserUtils.makeURI(location);
if ( ! browser.documentURI.equalsExceptRef(receivedURI)) {
return ;
host, location) {
// It is possible that we've received a message from the frame script to show
// a click to play notification for a principal that no longer matches the one
// that the browser's content now has assigned (ie, the browser has browsed away
// after the message was sent, but before the message was received). In that case,
// we should just ignore the message.
if ( ! principal.equals(browser.contentPrincipal)) {
return ;
}
// Data URIs, when linked to from some page, inherit the principal of that
// page. That means that we also need to compare the actual locations to
// ensure we aren't getting a message from a Data URI that we're no longer
// looking at.
let receivedURI = BrowserUtils.makeURI(location);
if ( ! browser.documentURI.equalsExceptRef(receivedURI)) {
return ;
}
Confirm that history.pushState is being used by the test case.
Yes, this appears to be the case.
Oooh - smaug has a patch for bug 1173215. Let's see if that fixes this!
Huh… it doesn't. :( Why not?
*sigh*. The URLs still don't match. Let me see if I can get a minimal test case here...
The failure is that the location that we're getting sent up via
this.content.document.location.href does not match the
browser.documentURI.spec.
Redirects
to: https://photos.google.com/share/AF1QipOmq9kJvQLc02UoDJuewyWH9K-3kBmREke-n7bqkXCBFekIndYCHDQN_AeqQvZFaw?key=YmVib0lYMUZ2Q3BDZi1jVTlNaUcya1U1WnYwZ1JR
PushState to URL: https://photos.google.com/share/AF1QipOmq9kJvQLc02UoDJuewyWH9K-3kBmREke-n7bqkXCBFekIndYCHDQN_AeqQvZFaw?key=YmVib0lYMUZ2Q3BDZi1jVTlNaUcya1U1WnYwZ1JR
PushState to URL: https://photos.google.com/share/AF1QipOmq9kJvQLc02UoDJuewyWH9K-3kBmREke-n7bqkXCBFekIndYCHDQN_AeqQvZFaw?key=YmVib0lYMUZ2Q3BDZi1jVTlNaUcya1U1WnYwZ1JR
PushState to URL: https://photos.google.com/share/AF1QipOmq9kJvQLc02UoDJuewyWH9K-3kBmREke-n7bqkXCBFekIndYCHDQN_AeqQvZFaw?key=YmVib0lYMUZ2Q3BDZi1jVTlNaUcya1U1WnYwZ1JR
Click on video
PushState to URL: /share/AF1QipOmq9kJvQLc02UoDJuewyWH9K-3kBmREke-n7bqkXCBFekIndYCHDQN_AeqQvZFaw/photo/AF1QipPq9LaXsIpH8fjccjbK-N5A1473RdJnaPlzXRC5?key=YmVib0lYMUZ2Q3BDZi1jVTlNaUcya1U1WnYwZ1JR
message arrives in parent from child:
location from child
= "https://photos.google.com/share/AF1QipOmq9kJvQLc02UoDJuewyWH9K-3kBmREke-n7bqkXCBFekIndYCHDQN_AeqQvZFaw/photo/AF1QipPq9LaXsIpH8fjccjbK-N5A1473RdJnaPlzXRC5?key=YmVib0lYMUZ2Q3BDZi1jVTlNaUcya1U1WnYwZ1JR"
but browser.documentURI is:
https://photos.google.com/share/AF1QipOmq9kJvQLc02UoDJuewyWH9K-3kBmREke-n7bqkXCBFekIndYCHDQN_AeqQvZFaw?key=YmVib0lYMUZ2Q3BDZi1jVTlNaUcya1U1WnYwZ1JR
Huh, ok, so I've got a working test case for e10s… not for non-e10s yet though. Weird...
Ah, make sure to try it in an iframe!! Wait. No. That doesn't make any
sense - the iframe is embedded in the same parent document… :/
WAAAAIT - this appears to be working in non-e10s mode now! Woo! Asking the reporter to confirm.
And if that's the case, then I have a working test case for it being busted in e10s (which I still see).
Ahhhh… so I think I need to better understand the relationship between
location and documentURI. That appears to be in flux here.
When browser-child.js handles onLocationChange in WebProgressListener
for pushState, it looks as if content.document.documentURIObject.spec is
pointing at the original URI that the document was at before the
pushState.
So the easiest thing would be to set documentURI to be the same as the
location we're setting to. But I need to examine what single process
Firefox does.
The test: create and inject a frame script that does the following:
1) Adds a listener for onLocationChange
2) Dumps out the value of content.document.documentURI.spec
Then inject that script into a non-e10s browser, and make that browser do the pushstate thing. Observe the output.
If the printed out spec doesn't match the final URI from the pushState, then we're apparently behaving properly. :(
Ok, let's get started.
CONCLUSION:
It seems to be the case that in single-process mode, in
onLocationChange, when firing after a pushState, the
content.document.documentURIObject.spec has not yet been updated.
But after escaping that function, in the parent, the
gBrowser.selectedBrowser.documentURIObject.spec has been updated. So
that must occur elsewhere. Right - it happens right after:
// Step 6: If the document's URI changed, update document's URI and update
// global history.
//
// We need to call FireOnLocationChange so that the browser's address bar
// gets updated and the back button is enabled, but we only need to
// explicitly call FireOnLocationChange if we're not calling SetCurrentURI,
// since SetCurrentURI will call FireOnLocationChange for us.
//
// Both SetCurrentURI(...) and FireDummyOnLocationChange() pass
// nullptr for aRequest param to FireOnLocationChange(...). Such an update
// notification is allowed only when we know docshell is not loading a new
// document and it requires LOCATION_CHANGE_SAME_DOCUMENT flag. Otherwise,
// FireOnLocationChange(...) breaks security UI.
if (!
equalURIs
) {
SetCurrentURI
(
newURI
, nullptr, true, LOCATION_CHANGE_SAME_DOCUMENT);
<-- FIRES ONLOCATIONCHANGE
document
->
SetDocumentURI
(
newURI
);
<-- UPDATED DOCUMENT URI
AddURIVisit
(
newURI
,
oldURI
,
oldURI
, 0);
// AddURIVisit doesn't set the title for the new URI in global history,
// so do that here.
if (
mUseGlobalHistory
&& !
mInPrivateBrowsing
) {
nsCOMPtr
<
IHistory
>
history
=
services
::
GetHistoryService
();
Ok, cool. So, hypothesis:
In multi-process Firefox, after a pushState, browser.documentURI.spec
will always be one URI stale. As, in, it will always be out of date by
one URI.
YES, that appears to be the case. OK, got it.
Writing it up in the bug.
Renaming: Summary: Click-to-Play for Adobe Flash not working when
loading video not directly → browser.documentURI goes out of sync when
doing same-document navigation in e10s
My test passes, and the Flash thing works if I update the document URI
before
we do onLocationChange… so let me try that.