1327942 - Browser can't load the page (urlbar and title twitch between 2 values) if it tries to access renamed local file

Status: VERIFIED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

[arni2033]

>>>   My Info:   Win7_64, Nightly 53, 32bit, ID 20161212030206 (2016-12-12)
STR_1:
1. Save file https://bugzilla.mozilla.org/skins/contrib/Mozilla/tabzilla.png
2. Open new tab, open the file (saved in Step 1) in that tab
3. Rename the file to "tabzilla 1.png"
4. Reload the tab opened in Step 2

AR:  Url and tab title constantly blink between visited url and "". Content area stays blank
ER:  Url and tab title shouldn't blink. Content area should display error page


STR_2:  (original)
1. Save file https://bugzilla.mozilla.org/skins/contrib/Mozilla/tabzilla.png
2. Open new tab, open the file (saved in Step 1) in that tab
3. Navigate to about:newtab or about:preferences in that tab
4. Rename the file to "tabzilla 1.png"
5. In the tab opened in Step 2, click Back button in urlbar

AR:  Url and tab title constantly blink between 2 urls visited in the tab. Content area stays blank
ER:  Url and tab title shouldn't blink. Content area should display the page opened in Step 3

Comment 1

[Ciprian Muresan [:cmuresan], Ecosystem QA]

[Tracking Requested - why for this release]:

Narrowed inbound regression window from [900960e6, 0637dd27] (3 revisions) to [49228a69, 0637dd27] (2 revisions) (~1 steps left)
Oh noes, no (more) inbound revisions :(
Last good revision: 49228a69b071bc200360aa43845b42b996759479
First bad revision: 0637dd270ef14763921d3099b6f6d5780fa702f6
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=49228a69b071bc200360aa43845b42b996759479&tochange=0637dd270ef14763921d3099b6f6d5780fa702f6

Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1147911

Bob, can you take a look at this?

Comment 2

[Marcia Knous [:marcia]]

Tracking 53+ - new regression that has regression window identified.

Comment 3

[:Gijs (he/him)]

(In reply to Marcia Knous [:marcia - use ni] from comment #2)
> Tracking 53+ - new regression that has regression window identified.

The feature in question is curtailed to Nightly, so right now this won't ride the trains with 53 (see bug 1321430). It's not clear to me this needs tracking.

Comment 4

[Bob Owen (:bobowen)]

Dealing with release issues at the moment, I'll add this to our next sandbox milestones, so should get to this fairly soon.

As Gijs says, don't think this needs tracking, unless it also occurs with:
browser.tabs.remote.separateFileUriProcess=false

Comment 5

[Ciprian Muresan [:cmuresan], Ecosystem QA]

User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID 	20170104133516

The issue is not reproducible with browser.tabs.remote.separateFileUriProcess=false.

Comment 6

[Bob Owen (:bobowen)]

(In reply to Ciprian Muresan [:cmuresan] from comment #5)
> User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:53.0)
> Gecko/20100101 Firefox/53.0
> Build ID 	20170104133516
> 
> The issue is not reproducible with
> browser.tabs.remote.separateFileUriProcess=false.

Thanks, I'm going to say we shouldn't track this then, as it will be off for 53 when it goes to Aurora.

I also know what is causing this, shouldn't be too hard to fix.
The about:neterror page that it tries to load when it can't find the file in the child process, can't load in that process.
I think it should be able to.

Comment 7

[Bob Owen (:bobowen)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=83b44ea328ec20e8a072fe097b6ce19f886a6cc5

Comment 8

[Bob Owen (:bobowen)]

Attachment: Part 1: Allow about pages that can load in non-remote and remote browser to load in any remote type

Comment 9

[Bob Owen (:bobowen)]

Attachment: Part 2: Check that reload of deleted file loads correct error page

Comment 10

[:Gijs (he/him)]

Comment on attachment 8830186 [details] [diff] [review]
Part 1: Allow about pages that can load in non-remote and remote browser to load in any remote type

Review of attachment 8830186 [details] [diff] [review]:
-----------------------------------------------------------------

D'oh.

Comment 11

[:Gijs (he/him)]

Comment on attachment 8830188 [details] [diff] [review]
Part 2: Check that reload of deleted file loads correct error page

Review of attachment 8830188 [details] [diff] [review]:
-----------------------------------------------------------------

Yay tests!

::: browser/base/content/test/tabs/browser_reload_deleted_file.js
@@ +28,5 @@
> +  disappearingPage.remove(false);
> +  document.getElementById("urlbar-reload-button").doCommand();
> +  yield BrowserTestUtils.waitForErrorPage(tab.linkedBrowser);
> +  ok(content.document.documentURI.startsWith("about:neterror"),
> +    "Check that a neterror page was loaded.");

Nit: can we use a content task here? `content.document` is a CPOW if this loads in a remote process. So:

yield ContentTask.spawn(tab.linkedBrowser, null, function() {
  ok(content.document.documentURI.startsWith("about:neterror"), 
     "Check that a neterror page was loaded.");
});

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f236eee4d240
Part 1: Allow about pages that can load in non-remote and remote browser to load in any remote type. r=gijs
https://hg.mozilla.org/integration/mozilla-inbound/rev/39fc2de38a96
Part 2: Check that reload of deleted file loads correct error page. r=gijs

Comment 13

[Bob Owen (:bobowen)]

(In reply to :Gijs from comment #11)

> > +  ok(content.document.documentURI.startsWith("about:neterror"),
> > +    "Check that a neterror page was loaded.");
> 
> Nit: can we use a content task here? `content.document` is a CPOW if this
> loads in a remote process. So:
> 
> yield ContentTask.spawn(tab.linkedBrowser, null, function() {
>   ok(content.document.documentURI.startsWith("about:neterror"), 
>      "Check that a neterror page was loaded.");
> });

Thanks gijs, I fixed that before pushing.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f236eee4d240
https://hg.mozilla.org/mozilla-central/rev/39fc2de38a96

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Does this need an Aurora approval request?

Comment 16

[Bob Owen (:bobowen)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #15)
> Does this need an Aurora approval request?

No, we don't need an uplift because the separate file content process is only preffed on in Nightly and comment 5 confirmed that we don't see the issue when preffed off.

Comment 17

[Simona Badau, Desktop QA]

Reproduced the initial issue on Nightly 53.0a1 (Build ID: 20161220030215)on Windows 10 x64.

Verified as fixed using the latest Nightly 54.0a1 (Build ID: 20170227030203) on Windows 10 x64 and Ubuntu 16.04.

But, on Mac OS X, in step 5 (STR 2), the behavior is a little different than on Windows and Ubuntu. On Mac OS X, the tabzilla image is properly loaded after clicking on the Back button (even if the local file was renamed). It takes a refresh of the page for the "File not found" error to be displayed.

Bob, is this expected in any way?

Comment 18

[Bob Owen (:bobowen)]

(In reply to Simona B [:simonab ] from comment #17)
> Reproduced the initial issue on Nightly 53.0a1 (Build ID: 20161220030215)on
> Windows 10 x64.
> 
> Verified as fixed using the latest Nightly 54.0a1 (Build ID: 20170227030203)
> on Windows 10 x64 and Ubuntu 16.04.
> 
> But, on Mac OS X, in step 5 (STR 2), the behavior is a little different than
> on Windows and Ubuntu. On Mac OS X, the tabzilla image is properly loaded
> after clicking on the Back button (even if the local file was renamed). It
> takes a refresh of the page for the "File not found" error to be displayed.
> 
> Bob, is this expected in any way?

Hmm, I'm not sure, I get the File not found straight away on my Macbook.

Have you tried with the pref disabled (browser.tabs.remote.separateFileUriProcess=false)?
If you get the same behaviour then I think it's probably unrelated.

Comment 19

[Simona Badau, Desktop QA]

(In reply to Bob Owen (:bobowen) from comment #18)
> Hmm, I'm not sure, I get the File not found straight away on my Macbook.
> 
> Have you tried with the pref disabled
> (browser.tabs.remote.separateFileUriProcess=false)?
> If you get the same behaviour then I think it's probably unrelated.

Could this be because of some of some Mac OS X settings? On a different Mac OS X machine this is working properly while on my machine this is failing. What is strange is that if I set the preference browser.tabs.remote.separateFileUriProcess to false on my machine I'm getting the right behavior (File not found error is displayed after renaming and clicking on the Back button).

Comment 20

[Bob Owen (:bobowen)]

(In reply to Simona B [:simonab ] from comment #19)

> Could this be because of some of some Mac OS X settings? On a different Mac
> OS X machine this is working properly while on my machine this is failing.
> What is strange is that if I set the preference
> browser.tabs.remote.separateFileUriProcess to false on my machine I'm
> getting the right behavior (File not found error is displayed after renaming
> and clicking on the Back button).

Maybe, might not even be a Mac OS X specific setting, I'm guessing that this is something to do with using the cache.

Either way this seems pretty minor, but it would be nice to know what's going on, can you file a follow-up bug please.

Comment 21

[Simona Badau, Desktop QA]

(In reply to Bob Owen (:bobowen) from comment #20)
> Maybe, might not even be a Mac OS X specific setting, I'm guessing that this
> is something to do with using the cache.
> 
> Either way this seems pretty minor, but it would be nice to know what's
> going on, can you file a follow-up bug please.

I can't reproduce this issue anymore (not even on the same profile where this was reproducible). I'll keep my eyes opened and if it's back I will file the follow up bug.

Comment 22

[Nazir Ahmed Sabbir [:NaSb]]

Successfully reproduce this bug on Nightly 53.0a1 (2016-12-12) (64-bit) (Build ID: 20161212030206) by the following Comment 0's instruction!

This Bug is now Verified as Fixed on Latest Firefox Nightly 54.0a1 (2017-03-02) (64-bit)

Build ID: 20170302110226
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0
OS: Linux 4.8.0-39-generic; Ubuntu 16.04.2 (64 Bit)

Comment 23

[Simona Badau, Desktop QA]

Based on Comments 17, 21 and 22, setting the status-firefox54: to verified.