Closed Bug 1327942 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: Security: Process Sandboxing, defect)

53 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla54
Tracking Status
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 --- disabled
firefox54 --- verified

People

(Reporter: arni2033, Assigned: bobowen)

References

Details

(Keywords: regression, Whiteboard: sbwc2, sblc3, sbmc2,[e10s-multi:M2])

Attachments

(2 files)

>>>   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
No longer blocks: 1277113
[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?
Blocks: 1147911
Flags: needinfo?(bobowencode)
Version: Trunk → 53 Branch
Component: Untriaged → Security: Process Sandboxing
Product: Firefox → Core
Tracking 53+ - new regression that has regression window identified.
(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.
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
Flags: needinfo?(bobowencode)
Whiteboard: sbwc2, sblc3, sbmc2,[e10s-multi:M2]
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.
(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.
Assignee: nobody → bobowencode
Status: NEW → ASSIGNED
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.
Attachment #8830186 - Flags: review?(gijskruitbosch+bugs) → review+
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.");
});
Attachment #8830188 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by bobowencode@gmail.com:
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
(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.
https://hg.mozilla.org/mozilla-central/rev/f236eee4d240
https://hg.mozilla.org/mozilla-central/rev/39fc2de38a96
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Does this need an Aurora approval request?
Flags: needinfo?(bobowencode)
(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.
Flags: needinfo?(bobowencode)
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?
Flags: needinfo?(bobowencode)
(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.
Flags: needinfo?(bobowencode)
(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).
Flags: needinfo?(bobowencode)
(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.
Flags: needinfo?(bobowencode)
(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.
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)
QA Whiteboard: [bugday-20170301]
Based on Comments 17, 21 and 22, setting the status-firefox54: to verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.