Closed
Bug 1268285
Opened 8 years ago
Closed 8 years ago
[e10s] Browser breaks when I open invalid link in a new tab
Categories
(Firefox :: Tabbed Browser, defect, P3)
Firefox
Tabbed Browser
Tracking
()
VERIFIED
FIXED
Firefox 49
People
(Reporter: arni2033, Assigned: Gijs)
References
Details
(Keywords: regression)
Attachments
(1 file)
58 bytes,
text/x-review-board-request
|
Felipe
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details |
>>> My Info: Win7_64, Nightly 49, 32bit, ID 20160426044609 STR_1: 1. Open new window (for your own safety), make sure there's only one tab about:home in that window 2. Type "chrome://skin/images/" without quotes in urlbar, press Alt+Enter 3. Close selected tab AR: No tabs in tabs toolbar, only [+] button ER: Window should close This is regression from bug 1243707. Regression range: > https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=8046e825980ac4080619527b72292b8e9ac1f56d&tochange=5ae810198a9c8c7f4e75cd0881bfc8d1295cbc8d STR_2 (explains how exactly the browser is broken): 1. Open new window with only 2 tabs - 2 about:home tabs. Select the second tab 2. Type "chrome://skin/images/" without quotes in urlbar, press Alt+Enter 3. Press Ctrl+PageDown 4. Press Ctrl+PageDown 5. Click menu (≡) -> Options. Switch to 1st window, click (≡)->Options. Switch back to 2nd window 6. Close two opened tabs 7. Click [+] button in tabs toolbar 8. Wait 1 minute, then close the window opened in Step 1 (if you can). Wait 5 minutes. AR: Step 3 - nothing. Step 4 - 1st tab is selected. Step 5 - nothing. Step 6 - 0 tabs in tabs toolbar. Step 7 - firefox.exe consumes 25% CPU. Step 8 - firefox.exe still consumes 25% CPU. ER: Step 3 - Switch to 1st tab. Step 4 - switch to 2nd tab. Step 5 - Options should open. Step 6 - 2nd window should close
[*STR_1 - Step 2*] ----(unknown) [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIURIFixup.createFixupURI]" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: resource:///modules/PlacesUIUtils.jsm :: PUIU_createFixedURI :: line 95" data: no] PUIU_createFixedURI() PlacesUIUtils.jsm:95 PUIU_markPageAsTyped() PlacesUIUtils.jsm:520 addToUrlbarHistory() browser.js:3840 continueOperation() urlbarBindings.xml:387 handleCommand/<() urlbarBindings.xml:377 _canonizeURL/<() urlbarBindings.xml:526 Handler.prototype.process() Promise-backend.js:937 this.PromiseWalker.walkerLoop() Promise-backend.js:816 bound () self-hosted bound bound () self-hosted ----E10SUtils.jsm:55 A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'? See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise Date: Thu Apr 28 2016 13:33:01 GMT+0300 Full Message: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIIOService2.newURI] Full Stack: JS frame :: resource://app/modules/E10SUtils.jsm :: this.E10SUtils.canLoadURIInProcess :: line 55 JS frame :: chrome://browser/content/tabbrowser.xml :: _linkBrowserToTab :: line 1852 JS frame :: chrome://browser/content/tabbrowser.xml :: addTab :: line 2033 JS frame :: chrome://browser/content/tabbrowser.xml :: loadOneTab :: line 1489 JS frame :: chrome://browser/content/utilityOverlay.js :: openLinkIn :: line 358 JS frame :: chrome://browser/content/utilityOverlay.js :: openUILinkIn :: line 196 JS frame :: chrome://browser/content/urlbarBindings.xml :: continueOperation :: line 442 JS frame :: chrome://browser/content/urlbarBindings.xml :: handleCommand/< :: line 377 JS frame :: chrome://browser/content/urlbarBindings.xml :: _canonizeURL/< :: line 526 JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: Handler.prototype.process :: line 937 JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: this.PromiseWalker.walkerLoop :: line 816 [*STR_1 - Step 3*] ----TabState.jsm:104:1 TypeError: browser is undefined
Flags: needinfo?(arni2033)
Surely, but I can't right away as I am away on a family emergency right now.
Flags: needinfo?(allassopraise)
I have tried this on latest Nightly on Mac and Windows, and cannot reproduce. I suspect many of the steps are superfluous as the process is quite convoluted, and some of them don't make sense (ie, pasting chrome://skin/images/ in urlbar and pressing Alt+Enter (or enter) simply removes the url from the bar even in pre-1243707). Can you please reduce these steps to the simplest path possible to reproduce the problem?
Flags: needinfo?(arni2033)
Comment 6•8 years ago
|
||
I can reproduce on latest Nightly49.0a1 https://hg.mozilla.org/mozilla-central/rev/86730d0a82093d705e44f33a34973d28b269f1ea Mozilla/5.0 (Windows NT 6.1; WOW64; rv:49.0) Gecko/20100101 Firefox/49.0 ID:20160428030218 STR 1. Make sure that there is a tab1, now, there is [about:home][+] 2. Paste chrome://skin/images/ in LocationBar and Alt+ENTER ---- observe: nothing happens, but error[1] is shown in Browser Console 3. Close tab1 ---- observe: there is [+] only, and error[2] is shown in Browser Console 4. Click [+] to open New Tab ---- opverve: now, [about:newtab][+], and error[3] is shown in Browser Console 5. Quit Browser from File > exit ---- observe: CPU 100% core usage, and Browser will crash[4] after for a minite [1] Key event not available on some keyboard layouts: key="c" modifiers="accel,alt"browser.xul Key event not available on some keyboard layouts: key="i" modifiers="accel,alt,shift"browser.xul [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIURIFixup.createFixupURI]" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: resource:///modules/PlacesUIUtils.jsm :: PUIU_createFixedURI :: line 95" data: no] (unknown) TypeError: browser is undefinedTabState.jsm:104:1 A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'? See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise Date: Thu Apr 28 2016 23:59:16 GMT+0900 Full Message: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIIOService2.newURI] Full Stack: JS frame :: resource://app/modules/E10SUtils.jsm :: this.E10SUtils.canLoadURIInProcess :: line 55 JS frame :: chrome://browser/content/tabbrowser.xml :: _linkBrowserToTab :: line 1852 JS frame :: chrome://browser/content/tabbrowser.xml :: addTab :: line 2033 JS frame :: chrome://browser/content/tabbrowser.xml :: loadOneTab :: line 1489 JS frame :: chrome://browser/content/utilityOverlay.js :: openLinkIn :: line 358 JS frame :: chrome://browser/content/utilityOverlay.js :: openUILinkIn :: line 196 JS frame :: chrome://browser/content/urlbarBindings.xml :: continueOperation :: line 442 JS frame :: chrome://browser/content/urlbarBindings.xml :: handleCommand/< :: line 377 JS frame :: chrome://browser/content/urlbarBindings.xml :: _canonizeURL/< :: line 526 JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: Handler.prototype.process :: line 937 JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: this.PromiseWalker.walkerLoop :: line 816E10SUtils.jsm:55 [2] TypeError: browser is undefined1TabState.jsm:104:1 [3] TypeError: browser is undefinedTabState.jsm:104:1 Warning: NetUtil.asyncFetch() requires the channel to have one of the security flags set in the loadinfo (see nsILoadInfo). Please create channel using NetUtil.newChannel() TypeError: browser is null tabbrowser.xml:3330:19 TypeError: tab.linkedBrowser is null tabbrowser.xml:3836:17 TypeError: this.browsers[i] is undefined tabbrowser.xml:411:1 TypeError: this.browsers[i] is undefined tabbrowser.xml:411:1 Warning: NetUtil.asyncFetch() requires the channel to have one of the security flags set in the loadinfo (see nsILoadInfo). Please create channel using NetUtil.newChannel() TelemetryStopwatch: key "FX_SESSION_RESTORE_COLLECT_DATA_MS" was already initialized TelemetryStopwatch.jsm:282 TelemetryStopwatch: key "FX_SESSION_RESTORE_COLLECT_DATA_LONGEST_OP_MS" was already initialized TelemetryStopwatch.jsm:282 TelemetryStopwatch: key "FX_SESSION_RESTORE_COLLECT_ALL_WINDOWS_DATA_MS" was already initialized TelemetryStopwatch.jsm:282 TypeError: browser is undefinedTabState.jsm:104:1 [4] bp-e88fe48d-95cc-43d2-94cc-3886d2160428
Comment 7•8 years ago
|
||
I was able to reproduce on windows. This requires the use of the chrome url afaict, it won't fail with a web address. 1) new window, navigate to about:home 2) chrome://skin/images/ + alt+return 3) close tab using tab x funky browser with no tabs. Click the + and you get a tab, click close window and it closes. polish, non-blocking imo.
Comment hidden (spam) |
Assignee | ||
Comment 9•8 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #7) > I was able to reproduce on windows. This requires the use of the chrome url > afaict, it won't fail with a web address. I haven't tested this, but I wouldn't be surprised if it failed in the same way with anything that's not a valid URI if you've e.g. got keyword.enabled turned off.
Comment 10•8 years ago
|
||
Still unable to reproduce on Mac or Windows. I get errors but nothing related to _linkBrowserToTab or addTab. No browser crashing. [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIURIFixup.createFixupURI]" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: resource:///modules/PlacesUIUtils.jsm :: PUIU_createFixedURI :: line 95" data: no] PUIU_createFixedURI() PlacesUIUtils.jsm:95 PUIU_markPageAsTyped() PlacesUIUtils.jsm:520 addToUrlbarHistory() browser.js:3840 continueOperation() urlbarBindings.xml:387 handleCommand/<() urlbarBindings.xml:377 _canonizeURL/<() urlbarBindings.xml:526 Handler.prototype.process() Promise-backend.js:937 this.PromiseWalker.walkerLoop() Promise-backend.js:816 bound () self-hosted bound bound () self-hosted (unknown) A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'? See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise Date: Thu Apr 28 2016 09:55:21 GMT-0600 (Mountain Standard Time) Full Message: Error: page-thumbnail:error Full Stack: observe@resource://gre/modules/BackgroundPageThumbs.jsm:112:22 BackgroundPageThumbs._onCaptureOrTimeout@resource://gre/modules/BackgroundPageThumbs.jsm:275:7 Capture.prototype._done/done@resource://gre/modules/BackgroundPageThumbs.jsm:413:7 Capture.prototype._done@resource://gre/modules/BackgroundPageThumbs.jsm:425:7 Capture.prototype.notify@resource://gre/modules/BackgroundPageThumbs.jsm:390:5 BackgroundPageThumbs.jsm:112 Warning: NetUtil.asyncFetch() requires the channel to have one of the security flags set in the loadinfo (see nsILoadInfo). Please create channel using NetUtil.newChannel() A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'? See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise Date: Thu Apr 28 2016 09:55:51 GMT-0600 (Mountain Standard Time) Full Message: Error: page-thumbnail:error Full Stack: observe@resource://gre/modules/BackgroundPageThumbs.jsm:112:22 BackgroundPageThumbs._onCaptureOrTimeout@resource://gre/modules/BackgroundPageThumbs.jsm:275:7 Capture.prototype._done/done@resource://gre/modules/BackgroundPageThumbs.jsm:413:7 Capture.prototype._done@resource://gre/modules/BackgroundPageThumbs.jsm:425:7 Capture.prototype.notify@resource://gre/modules/BackgroundPageThumbs.jsm:390:5 BackgroundPageThumbs.jsm:112
Updated•8 years ago
|
tracking-e10s:
--- → +
Priority: -- → P3
Assignee | ||
Comment 11•8 years ago
|
||
The issue here turns out to be very simple: before bug 1243707, opening a new tab created the "remote" boolean before the tab got appended to the tabbrowser. Trying to run that code: let remote = gMultiProcessBrowser && !aForceNotRemote && E10SUtils.canLoadURIInProcess(aURI, Ci.nsIXULRuntime.PROCESS_TYPE_CONTENT); actually throws because: E10SUtils.canLoadURIInProcess(aURI, Ci.nsIXULRuntime.PROCESS_TYPE_CONTENT); throws because trying to create a URI for "chrome://skin/images/" throws: [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIIOService2.newURI]" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: debugger eval code :: <TOP_LEVEL> :: line 1" data: no] That code used to run before the new tab element got stuck on the tabbrowser. Now it runs after that happens, but before we've managed to hook up a <browser>, and so we end up with a tab with no browser connected to it, and tabbrowser gets all kinds of unhappy. The trivial fix would be to move this back into addTab and pass |remote| on to _linkBrowserToTab as a given, rather than something it has to determine. Dão, does that sound sensible ?
Flags: needinfo?(dao+bmo)
Assignee | ||
Updated•8 years ago
|
status-firefox48:
--- → affected
Comment 12•8 years ago
|
||
Is there a good reason for E10SUtils.canLoadURIInProcess to throw rather than swallowing the exception and returning true?
Flags: needinfo?(dao+bmo)
Assignee | ||
Comment 13•8 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #12) > Is there a good reason for E10SUtils.canLoadURIInProcess to throw rather > than swallowing the exception and returning true? Not sure. Felipe ? Does it matter what process we load error pages in and/or can we ignore errors from canLoadURIInProcess?
Flags: needinfo?(felipc)
Comment 14•8 years ago
|
||
Regression from 48, tracking.
tracking-firefox48:
--- → +
tracking-firefox49:
--- → +
Comment 15•8 years ago
|
||
So, IIUIC, this throws only because the URL used here is invalid, right? And not because it's chrome://. In other words, a valid chrome:// url wouldn't throw, correct? I don't think it matters which process the error pages loads, as long as the page works correctly on both processes (does it have any interactive buttons or links?)
Flags: needinfo?(felipc)
Assignee | ||
Comment 16•8 years ago
|
||
(In reply to :Felipe Gomes (needinfo me!) from comment #15) > So, IIUIC, this throws only because the URL used here is invalid, right? And > not because it's chrome://. In other words, a valid chrome:// url wouldn't > throw, correct? Yes. > I don't think it matters which process the error pages loads, as long as the > page works correctly on both processes (does it have any interactive buttons > or links?) Nothing ends up getting loaded for bad chrome URIs. You end up with a tab with an empty title and about:blank loaded (when turning off e10s). Other "invalid URIs" do have buttons etc., but it looks like we only call newURI for chrome URIs. :-\
Assignee | ||
Comment 17•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/51915/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/51915/
Attachment #8751226 -
Flags: review?(felipc)
Comment 18•8 years ago
|
||
Comment on attachment 8751226 [details] MozReview Request: Bug 1268285 - E10SUtils shouldn't throw when passed invalid chrome: URIs, r?felipe https://reviewboard.mozilla.org/r/51915/#review48789
Attachment #8751226 -
Flags: review?(felipc) → review+
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Comment 20•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/45daaf6edeae
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Reporter | ||
Comment 21•8 years ago
|
||
Affected: Win7_64, Nightly 49, 32bit, ID 20160511030221 (2016-05-11) Fixed on: Win7_64, Nightly 49, 32bit, ID 20160512030253 (2016-05-12) Win7_64, Nightly 49, 32bit, ID 20160526082509 (2016-05-26) No crashes and other strangeness with browser.
Status: RESOLVED → VERIFIED
Comment 22•8 years ago
|
||
Gijs, are you going to request uplift this to 48? Thanks
Flags: needinfo?(gijskruitbosch+bugs)
Assignee | ||
Comment 23•8 years ago
|
||
Comment on attachment 8751226 [details] MozReview Request: Bug 1268285 - E10SUtils shouldn't throw when passed invalid chrome: URIs, r?felipe Approval Request Comment [Feature/regressing bug #]: bug 1243707 [User impact if declined]: tabbrowser gets confused if you open a tab with an invalid chrome: URI. [Describe test coverage new/current, TreeHerder]: the tabbrowser as a whole has coverage, this specific part edgecase does not. [Risks and why]: low risk, avoids throwing in certain situations from a utility component [String/UUID change made/needed]: no.
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #8751226 -
Flags: approval-mozilla-aurora?
Comment 24•8 years ago
|
||
Comment on attachment 8751226 [details] MozReview Request: Bug 1268285 - E10SUtils shouldn't throw when passed invalid chrome: URIs, r?felipe Fix a leak, taking it
Attachment #8751226 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 25•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/6f97167428c0
You need to log in
before you can comment on or make changes to this bug.
Description
•