Closed Bug 1243549 Opened 9 years ago Closed 9 years ago

When I run (launch, open...) Firefox for the first time during a session I get a blank page instead of my usual home page.

Categories

(Firefox :: Session Restore, defect)

44 Branch
Unspecified
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 47
Tracking Status
firefox44 - wontfix
firefox45 + verified
firefox46 --- verified
firefox47 --- verified

People

(Reporter: yesyesrun, Assigned: Yoric, NeedInfo)

References

(Depends on 1 open bug)

Details

(Keywords: regression)

Attachments

(7 files, 1 obsolete file)

(deleted), image/png
Details
(deleted), text/x-review-board-request
mconley
: review+
Yoric
: review+
Details
(deleted), text/plain
Details
(deleted), text/plain
Details
(deleted), text/x-review-board-request
mak
: review+
Yoric
: review+
Details
(deleted), patch
Yoric
: review+
Details | Diff | Splinter Review
(deleted), patch
Yoric
: review+
Details | Diff | Splinter Review
Attached image Capture.PNG (deleted) —
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0 Build ID: 20160123151951 Steps to reproduce: Turned on my computer. Launched Firefox. It automatically updated to version 44 (27th January 2016). Restarted Firefox. Actual results: Home page not loading. I get a blank page. Expected results: I should have seen my home page as usual. (If from the blank page position I click on the home page icon I get the home page properly) (If from the blank page position I click on the new window button I get another home page properly)
Does it work with a fresh profile and your settings for the homepage? https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles If yes, you can reset your current profile to keep only your private data: https://support.mozilla.org/en-US/kb/reset-firefox-to-fix-most-problems
Flags: needinfo?(yesyesrun)
Thanks for your reply Loic. I'll try that solution as soon as possible and post a feedback here. For the moment I can add that I have the same problem on another machine. Both machines running Windows 7.
Flags: needinfo?(yesyesrun)
I think it has to do with the uBlock Origin extension. If I remove that everything works. Never had problems before. The other extension I use is Https Everywhere.
Same problem if I install AdBlock Plus.
Forget about uBlock Origin and AdBlock Plus. The same issue occurs even if I remove them.
Component: Untriaged → General
OS: Unspecified → Windows 7
If you restart in safe mode (Help > Restart with add-ons disabled), does this still happen?
Component: General → Session Restore
Flags: needinfo?(yesyesrun)
(In reply to :Gijs Kruitbosch from comment #6) > If you restart in safe mode (Help > Restart with add-ons disabled), does > this still happen? I was having a similar problem in that when firefox started up I got a blank page instead of a restored session, whether I try to save/restore sessions via firefox's builtin session manager or Tab Mix Plus's manager. Similarly, about:customizer was a blank page and newly-installed addons couldn't add buttons to the addon bar. I got amusingly broken results when restarting in safe mode: 1. Click 'Restart with addons disabled' 2. Click 'Run in safe mode' 3. Get shown "Well, this is embarrassing" with a single Window 1 containing no tabs. 4. Click 'Restore'. 5. Firefox closes with no error. HOWEVER, safe mode may fix this bug: Doing this a couple of times appears to have cleared out whatever bug I was experiencing. I'm not sure if the magic number was twice or three times.
Hi guys, thanks for comments. I'm still trying to solve this issue. I tried to follow your suggestions with no stable results. Any time I try to start a standard session as before the 44 update I get this problem. Restarting in safe mode with add-ons disabled seems to work.
Flags: needinfo?(yesyesrun)
(In reply to yesyesrun from comment #8) > Hi guys, thanks for comments. I'm still trying to solve this issue. > I tried to follow your suggestions with no stable results. Any time I try to > start a standard session as before the 44 update I get this problem. > > Restarting in safe mode with add-ons disabled seems to work. So could you try permanently disabling your add-ons and enabling them one-by-one to see which one is causing the issue? It sounds like one of them is breaking startup.
Flags: needinfo?(yesyesrun)
That's one of the first things I've tried to do. Read above. Both (or maybe none) of them cause the issue.
Flags: needinfo?(yesyesrun)
Onestly, the only real connection I can see with the issue is the update to Firefox 44. No problems at all before that.
(In reply to yesyesrun from comment #10) > That's one of the first things I've tried to do. Read above. Both (or maybe > none) of them cause the issue. Well, I'm a little confused because of comment #5... are you saying that if you disable all the add-ons, you're still seeing this in normal mode? Do any errors appear in the browser console (ctrl-shift-j) when that happens ?
Flags: needinfo?(yesyesrun)
The mozillazine thread on this: http://forums.mozillazine.org/viewtopic.php?f=38&t=2986649 reports people seeing this: Full Message: TypeError: tabBrowser is null in the browser console. Are (both of) you seeing this as well?
Flags: needinfo?(quality+bugzilla)
Yes, I get that error, as well as the other errors reported in that thread.
Flags: needinfo?(quality+bugzilla)
(In reply to quality+bugzilla from comment #15) > Yes, I get that error, as well as the other errors reported in that thread. If you go to about:config, what are the values of: privacy.sanitize.sanitizeInProgress and privacy.sanitize.didShutdownSanitize ?
Flags: needinfo?(quality+bugzilla)
(In reply to :Gijs Kruitbosch from comment #16) > (In reply to quality+bugzilla from comment #15) > > Yes, I get that error, as well as the other errors reported in that thread. > > If you go to about:config, what are the values of: > > privacy.sanitize.sanitizeInProgress > > and > > privacy.sanitize.didShutdownSanitize > > ? The bug happens sporadically. Would you like values after it happens, or when it does not?
Flags: needinfo?(quality+bugzilla)
When the home page is not shown: privacy.sanitize.sanitizeInProgress does not exist privacy.sanitize.didShutdownSanitize is true
When the home page *IS* shown, those two prefs are the same.
Yoric, it seems that in some cases, if we crash / close while sanitizing, we hit the sanitizer on startup, which then breaks startup because iterating over windows it looks for gBrowser, but because we're in startup, there isn't a gBrowser variable in that window. This breaks some of the startup code (!) with this failure: 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/J ... sm/Promise Full Message: TypeError: tabBrowser is null Full Stack: Sanitizer.prototype.items.formdata.canClear@chrome://browser/content/sanitize.js:426:15 Sanitizer.prototype.canClearItem@chrome://browser/content/sanitize.js:59:7 Sanitizer.prototype.promiseCanClearItem/<@chrome://browser/content/sanitize.js:49:1 Promise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:385:5 Sanitizer.prototype.promiseCanClearItem@chrome://browser/content/sanitize.js:48:1 Sanitizer.prototype._sanitize<@chrome://browser/content/sanitize.js:165:28 TaskImpl_run@resource://gre/modules/Task.jsm:314:40 Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:934:23 this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:813:7 this.PromiseWalker.scheduleWalkerLoop/<@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:1 sanitize.js:426:0 The yields in sanitizer.onStartup() should be catching errors, and we should be fixing the assumptions here so we no longer throw errors in this case. Can you take this? [Tracking Requested - why for this release]: We're intermittently breaking homepage opening and/or session restore for people who tick the 'clear history when Firefox closes' and leave the 'form and history data' checkbox checked (default). This is a serious bug and we should consider having a ride-along for 44 if we do a point release, or otherwise we should track for 45.
Blocks: 1089695
Status: UNCONFIRMED → NEW
Component: Session Restore → Places
Ever confirmed: true
Flags: needinfo?(dteller)
Keywords: regression
Product: Firefox → Toolkit
(In reply to :Gijs Kruitbosch from comment #21) > Yoric, it seems that in some cases, if we crash / close while sanitizing... Is this due to closing/sanitizing failing in some way? > We're intermittently breaking homepage opening and/or session restore for > people who tick the 'clear history when Firefox closes' and leave the 'form > and history data' checkbox checked (default). This is a serious bug and we > should consider having a ride-along for 44 if we do a point release, or > otherwise we should track for 45. It happens when 'form and history data' checkbox is UNchecked as well.
Actually the checkbox is called 'Remember search and form history'. The bug happens when it is UNchecked as well.
(In reply to quality+bugzilla from comment #23) > Actually the checkbox is called 'Remember search and form history'. The bug > happens when it is UNchecked as well. We're talking about different checkboxes. I mean the one that is in the dialog that comes up when you click the "Settings..." button next to "Clear History when Firefox closes", which is called "Form and Search history". If the same bug happens when that is unchecked, presumably the error message that you see in the browser console changes?
(In reply to quality+bugzilla from comment #22) > (In reply to :Gijs Kruitbosch from comment #21) > > Yoric, it seems that in some cases, if we crash / close while sanitizing... > > Is this due to closing/sanitizing failing in some way? Yes, which could include it just taking too long and Firefox deciding to quit before it finishes, or the OS doing the same.
(In reply to :Gijs Kruitbosch from comment #24) > (In reply to quality+bugzilla from comment #23) > > Actually the checkbox is called 'Remember search and form history'. The bug > > happens when it is UNchecked as well. > > We're talking about different checkboxes. I mean the one that is in the > dialog that comes up when you click the "Settings..." button next to "Clear > History when Firefox closes", which is called "Form and Search history". If > the same bug happens when that is unchecked, presumably the error message > that you see in the browser console changes? Thanks for the clarification. That checkbox IS checked.
(In reply to :Gijs Kruitbosch from comment #25) > (In reply to quality+bugzilla from comment #22) > > (In reply to :Gijs Kruitbosch from comment #21) > > > Yoric, it seems that in some cases, if we crash / close while sanitizing... > > > > Is this due to closing/sanitizing failing in some way? > > Yes, which could include it just taking too long and Firefox deciding to > quit before it finishes, or the OS doing the same. That's not good at all, and may represent a significant security and/or privacy issue. I recommend a quick point release to fix this.
Thanks for both the report and the investigation. The patch (not tested yet) should fix the issue. > That's not good at all, and may represent a significant security and/or privacy issue. > > I recommend a quick point release to fix this. If you're talking about the shutdown crash, this seems to be ~40% of shutdown hangs/crashes in Firefox 44+: http://yoric.github.io/are-we-shutting-down-yet-/?version=Firefox+47.0a1&version=Firefox+46.0a2&version=Firefox+45.0b1&version=Firefox+45.0a2&version=Firefox+44.0b99&version=Firefox+44.0b9&version=Firefox+44.0b1&version=Firefox+44.0&version=Firefox+43.0b3# Looking at the stacks, I see hangs/crashes while shutting down either history or cookies. Either most likely deserves a specific bug.
Flags: needinfo?(dteller)
We need to make sure that Sanitizer.onStartup is infallible, as a failure at this stage can prevent a successful startup. This patch addresses a specific issue that may prevent successful startup, as well as strengthens the startup path by making most errors non-fatal. Review commit: https://reviewboard.mozilla.org/r/32997/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/32997/
Attachment #8714298 - Flags: review?(mak77)
(In reply to :Gijs Kruitbosch from comment #21) > 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/J ... sm/Promise This is not an Exception, it shouldn't break the calling code. It should just be pointing out the consumer didn't handle onStartup() errors. Also, is not this the opposite of what comment 20 states, there it says everything is fine when both prefs are true (that means we were interrupted in the middle of sanitizing) > Full Message: TypeError: tabBrowser is null > Full Stack: > Sanitizer.prototype.items.formdata.canClear@chrome://browser/content/ > sanitize.js:426:15 We actually removed canClear in bug 1211849 (Firefox 46), I wonder if this bug reproduces on that version. But then probably instead of breaking on CanClear, it would break directly on Clear. > The yields in sanitizer.onStartup() should be catching errors, and we should > be fixing the assumptions here so we no longer throw errors in this case. Or maybe it's the consumer that should handle errors, or we'd never notice them. I wonder if instead the problem is just the fact we walk the windows and put them in an unexpected state. That said, Sanitizer.onStartup runs in finalUIStartup, that is before the first window is opened, that sounds indeed too early to be able to do anything with windows. I wonder if we couldn't move it at browser-delayed-startup-finished or sessionstore-windows-restored.
(In reply to Marco Bonardo [::mak] from comment #30) > > The yields in sanitizer.onStartup() should be catching errors, and we should > > be fixing the assumptions here so we no longer throw errors in this case. > > Or maybe it's the consumer that should handle errors, or we'd never notice > them. I tend to agree. > I wonder if instead the problem is just the fact we walk the windows and put > them in an unexpected state. > > That said, Sanitizer.onStartup runs in finalUIStartup, that is before the > first window is opened, that sounds indeed too early to be able to do > anything with windows. > I wonder if we couldn't move it at browser-delayed-startup-finished or > sessionstore-windows-restored. Or even a bit later, when tabs (and not just windows) are actually restored. At least, this would make sense for clearing forms, because currently, they are cleared before being filled.
Attachment #8714298 - Flags: review?(mak77)
Comment on attachment 8714298 [details] MozReview Request: Bug 1243549 - SessionFile.wipe() now waits until SessionFile has been properly initialized;r?mconley https://reviewboard.mozilla.org/r/32997/#review29773 Note that this patch only applies to 46 on, previous versions also had canClear and they need a different patch. ::: browser/base/content/sanitize.js:725 (Diff revision 1) > + } catch (ex) { I see how checking tabbrowser helps, but I don't see how throwing here would interrupt the calling code since we are inside a task. Plus, if we really want to make sanitize "infallible", we should catch single sanitizations rather than stopping at the first error, so we should handle that in the for loop that goes through the various steps.
even more interesting, we are already invoking every single clear() in try/catch, we were not doing that for canClear though. This still doesn't explain which code breaks if sanitize() rejects though.
(In reply to Marco Bonardo [::mak] from comment #30) > (In reply to :Gijs Kruitbosch from comment #21) > > 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/J ... sm/Promise > > This is not an Exception, it shouldn't break the calling code. It should > just be pointing out the consumer didn't handle onStartup() errors. AIUI yielding for a Task that has an exception rejects the promise, and breaks off the task at that point. But it's true that the caller here does not seem to actually use the Task.async that is sanitizer.onStartup at all, so in principle this shouldn't break startup. Maybe it's a red herring? :-( > Also, is not this the opposite of what comment 20 states, there it says > everything is fine when both prefs are true (that means we were interrupted > in the middle of sanitizing) It didn't say they are both true, it says they are the same. The other pref should contain a JSON state of what to still clear if we were interrupted, and if that didn't include formdata, it makes sense that we wouldn't run the code that is rejecting/throwing here. I assumed that neither pref existed / both were empty. > > Full Message: TypeError: tabBrowser is null > > Full Stack: > > Sanitizer.prototype.items.formdata.canClear@chrome://browser/content/ > > sanitize.js:426:15 > > We actually removed canClear in bug 1211849 (Firefox 46), I wonder if this > bug reproduces on that version. But then probably instead of breaking on > CanClear, it would break directly on Clear. > > > The yields in sanitizer.onStartup() should be catching errors, and we should > > be fixing the assumptions here so we no longer throw errors in this case. > > Or maybe it's the consumer that should handle errors, or we'd never notice > them. > > I wonder if instead the problem is just the fact we walk the windows and put > them in an unexpected state. This sounds plausible. > That said, Sanitizer.onStartup runs in finalUIStartup, that is before the > first window is opened, that sounds indeed too early to be able to do > anything with windows. > I wonder if we couldn't move it at browser-delayed-startup-finished or > sessionstore-windows-restored. I expect that sanitize should run before we open windows, but that might be incompatible with having a 'fast' startup in any useful sense of the word 'fast'...
(In reply to :Gijs Kruitbosch from comment #34) > I expect that sanitize should run before we open windows, but that might be > incompatible with having a 'fast' startup in any useful sense of the word > 'fast'... The problem with that is we have code in Sanitize.js that expects to act on windows, but we run it when there are no windows around, both on shutdown (see bug 1244650) and on startup. So first of all we should state if the sanitizer is expected to run with or without windows around.
(In reply to :Gijs Kruitbosch from comment #34) > AIUI yielding for a Task that has an exception rejects the promise, and > breaks off the task at that point. But it's true that the caller here does > not seem to actually use the Task.async that is sanitizer.onStartup at all, > so in principle this shouldn't break startup. Maybe it's a red herring? :-( It may be a red herring. But I can't tell off-hand. I think unconsciously you debugged bug 1244650, especially if you did that on Nightly. Indeed I suspect by removing canClear, now clear() happens early on startup and we get the tabBrowser problem, but still it won't interrupt init. I'll keep investigating bug 1244650, having an actual regression range here would probably speed up things, provided the issue is reproducible at will, and would clarify whether a sanitize change is really involved.
I confirm that the exception of comment 21 is basically logged and ignored, so it cannot directly cause the bug at hand.
In the event that another detail is helpful: I am seeing this bug quite a bit on on systems where browser.sessionstore.restore_on_demand is set to false. I do not have sufficient data to know if that data point is correlated or causal to the bug.
This bug may impact four essential tasks that need to always be successful: 1. Launch Firefox 2. Show homepage on launch (when option selected) 3. Sanitize (clear) Firefox on close (when option selected) 4. Close Firefox
Actually, this bug probably impacts only 1 and 2. At this stage, 3. is pretty likely to be entirely unrelated, at this stage, and it's bug 1244650. Also, 4. aka comment 28 is pretty likely to be entirely unrelated. It deserves its own two bugs. Unfortunately, we are pretty much back to having no clue about this bug. Any chance we could have a regression range?
Flags: needinfo?(quality+bugzilla)
I'll try and see if I can find STR and if fixing the error of comment 21 helps at all, but I'm a bit skeptical. A regression range would be much more convenient.
So far, I haven't been able to reproduce. Steps: * Open Firefox 43. * Create a new profile. * Set a home page. * Quit Firefox 43. * Open Firefox 44. * Home page loads. What am I missing?
(In reply to David Rajchenbach-Teller [:Yoric] (please use "needinfo") from comment #40) > Actually, this bug probably impacts only 1 and 2. > > At this stage, 3. is pretty likely to be entirely unrelated, at this stage, > and it's bug 1244650. > > Also, 4. aka comment 28 is pretty likely to be entirely unrelated. It > deserves its own two bugs. > > Unfortunately, we are pretty much back to having no clue about this bug. Any > chance we could have a regression range? Thanks. If I'm understanding you correctly, #3 already has a bug report filed. If so, that's good. It sounds like #4 needs to have 2 bugs filed. Can you (or someone else) do that, so they don't get lost in the shuffle? If you are asking me for a regression range, your request is above my paygrade. IOW, I don't think I have the knowledge to perform that task. If you provide me with details, I can look into it, and do my best. I'm booked for the next 24+ hours, so I won't see your response until then, at the earliest. If someone else already has that knowledge, perhaps they can fulfill your request more quickly. When testing patches, I recommend also testing them when extensions such as Click&Clean are installed. See https://addons.mozilla.org/firefox/addon/clickclean/ The bug is independent of extensions like that one, but that extension can influence shutdown sanitization, and has about 100K users, so compatibility should be verified.
(In reply to David Rajchenbach-Teller [:Yoric] (please use "needinfo") from comment #42) > So far, I haven't been able to reproduce. > > Steps: > * Open Firefox 43. > * Create a new profile. > * Set a home page. > * Quit Firefox 43. > * Open Firefox 44. > * Home page loads. > > What am I missing? Try setting it to clear everything on Firefox exit. There are some reports that it only happens with certain home pages, but I think it's more likely that it only happens sporadically, so it *appears* to only happen with certain home pages.
Flags: needinfo?(quality+bugzilla)
I get about a 60% failure rate of trying to load the home page. (Or, on the bright side, a 40% success rate.)
(In reply to quality+bugzilla from comment #44) > (In reply to David Rajchenbach-Teller [:Yoric] (please use "needinfo") from > comment #42) > > So far, I haven't been able to reproduce. > > > > Steps: > > * Open Firefox 43. > > * Create a new profile. > > * Set a home page. > > * Quit Firefox 43. > > * Open Firefox 44. > > * Home page loads. > > > > What am I missing? > > Try setting it to clear everything on Firefox exit. Done that. So far, no success. Can I keep relaunching my Firefox 44 or do I need to keep creating new profiles for each test?
If you have any more quick questions, just ask. I'll check back in 10 minutes to answer your questions, and then I'll be AFK for 24+ hours.
Attached file Shutdown spew (deleted) —
Quite possibly unrelated, but I get lots of shutdown errors in sanitize.
(In reply to David Rajchenbach-Teller [:Yoric] (please use "needinfo") from comment #46) > Can I keep relaunching my Firefox 44 or do I need to keep creating new > profiles for each test? You can keep relaunching Firefox 44.0. No need to create a new profile for each test. You can also try setting browser.sessionstore.restore_on_demand to false.
Attached file Startup spew (deleted) —
... and lots of startup spew. Tim, this message sounds familiar. Haven't you worked on a bug that had them at some point?
Flags: needinfo?(ttaubert)
I've seen a lot of that spew as well. OK... I'm going AFK. I'll do my best to check back as soon as I can. I expect a minimum of 24 hours AFK.
Ah, I have traced this startup spew to another problem which might be closer to our issue. * If sanitization has failed during shutdown, it attempts again to sanitize during startup. This happens more often than it used to, because of 1/ bug fixes; 2/ the bug of comment 28. * Sanitization during startup doesn't wait until Session Restore has properly started to sanitize the session. So sanitization of Session Restore file fails. This has probably always been the case, except we never noticed. * For some reason I do not understand, it attempts to sanitize several times. * I suspect that this can cause problems during startup, as sanitization and Session Restore race to use/remove the files of Session Restore. I'm not 100% sure that this is the issue at hand, but this is definitely a bug that needs to be fixed.
Flags: needinfo?(ttaubert)
Comment on attachment 8714298 [details] MozReview Request: Bug 1243549 - SessionFile.wipe() now waits until SessionFile has been properly initialized;r?mconley Review request updated; see interdiff: https://reviewboard.mozilla.org/r/32997/diff/1-2/
Attachment #8714298 - Attachment description: MozReview Request: Bug 1243549 - Making Sanitizer.onStartup infallible;r?mak → MozReview Request: Bug 1243549 - SessionFile.wipe() now waits until SessionFile has been properly initialized;r?mconley
Attachment #8714298 - Flags: review?(mconley)
During startup, sanitization is sometimes called before windows are ready. This can cause interesting issues and error messages, as it attempts to walk all the tabs, before the tabs are setup. This patch makes sure that we do not rely upon the presence of tabbrowser while performing sanitization. Review commit: https://reviewboard.mozilla.org/r/33237/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33237/
Attachment #8714759 - Flags: review?(mak77)
I expect that the two patches I have just attached will solve both the issue and the error message of comment 21. We will still need to investigate sanitization shutdown hangs.
Build Identifier: https://hg.mozilla.org/releases/mozilla-release/rev/9b79adf82b132f5e1fcfee4813a47b5b38bd7521 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0 ID:20160123151951 Reproducible: 100% Steps to reproduce: 1. Open Firefox 44 2. Create a new profile 3. Ooen about:preferences Disable "Always check if Firefox is your default browser" Set https://www.yahoo.com/ to a home page Select "Privacy" pane > Select "Use custom settings "for history" > Enable "Clear history when Firefox closes" 5. Search "mozilla" using Search Bar. (without quatation marks) 6. Restart Firefox 44. 7. Search "mozilla" using Search Bar. (without quatation marks) 8. Restart Firefox 44. --- observe the problem 9. Repeat step8 --- the problem is persistent Actual results: Home page not loading. I get a blank page. And, the problem is persistent. Error in Browser Console: "DevToolsUtils.dbg_assert is deprecated! Use DevToolsUtils.assert instead! dbg_assert@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/DevToolsUtils.js:449:13 ObjectActor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/object.js:57:1 WCA_objectGrip@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/webconsole.js:454:17 createValueGrip@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/object.js:1913:1 WCA_createValueGrip@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/webconsole.js:413:12 WCA_prepareConsoleMessageForRemote/result.arguments<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/webconsole.js:1561:14 WCA_prepareConsoleMessageForRemote@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/webconsole.js:1559:24 WCA_onGetCachedMessages/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/webconsole.js:754:27 WCA_onGetCachedMessages@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/webconsole.js:746:11 DSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1599:15 LocalDebuggerTransport.prototype.send/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:569:11 makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/DevToolsUtils.js:87:14 makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/DevToolsUtils.js:87:14 " DevToolsUtils.js:452 Could not write session state file TypeError: this.Paths is null Stack trace: Agent.write@resource:///modules/sessionstore/SessionWorker.js:161:9 worker.dispatch@resource:///modules/sessionstore/SessionWorker.js:21:24 anonymous/AbstractWorker.prototype.handleMessage@resource://gre/modules/workers/PromiseWorker.js:122:16 @resource:///modules/sessionstore/SessionWorker.js:30:41 Agent.write@resource:///modules/sessionstore/SessionWorker.js:161:9 worker.dispatch@resource:///modules/sessionstore/SessionWorker.js:21:24 anonymous/AbstractWorker.prototype.handleMessage@resource://gre/modules/workers/PromiseWorker.js:122:16 @resource:///modules/sessionstore/SessionWorker.js:30:41 SessionFile.jsm:299 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: Tue Feb 02 2016 22:08:53 GMT+0900 Full Message: TypeError: this.Paths is null Full Stack: Agent.wipe@resource:///modules/sessionstore/SessionWorker.js:296:7 worker.dispatch@resource:///modules/sessionstore/SessionWorker.js:21:24 anonymous/AbstractWorker.prototype.handleMessage@resource://gre/modules/workers/PromiseWorker.js:122:16 @resource:///modules/sessionstore/SessionWorker.js:30:41 SessionWorker.js:296:0 1454418544114 Services.HealthReport.HealthReporter WARN Saved state file does not exist.
This seems to confirm that my fix is sufficient to resolve the issue.
Sylvestre, any idea of the gravity of this bug?
Flags: needinfo?(sledru)
Not sure but happy to take an uplift in 45.
Flags: needinfo?(sledru)
There is a full topic of this issue here: http://forums.mozillazine.org/viewtopic.php?f=38&t=2986649 I saw a report on the French community board too.
Attachment #8714759 - Flags: review?(mak77) → review+
Comment on attachment 8714759 [details] MozReview Request: Bug 1243549 - Making sure that startup sanitization doesn't throw because it can't find a tabbrowser;r=mak https://reviewboard.mozilla.org/r/33237/#review29959
Hey alex_mayorga - does this not sound a lot like what you were experiencing with bug 1217904? You might want to try reproducing once the patches from this land, and if it goes away, duping 1217904 off to this bug.
Flags: needinfo?(alex_mayorga)
Attachment #8714298 - Flags: review?(mconley) → review+
Comment on attachment 8714298 [details] MozReview Request: Bug 1243549 - SessionFile.wipe() now waits until SessionFile has been properly initialized;r?mconley https://reviewboard.mozilla.org/r/32997/#review30001 Looks good - thanks Yoric!
Unfortunately, my patch seems to orange a number of tests. I'm investigating.
Assignee: nobody → dteller
mconley, for some reason, my patch doesn't behave on e10s. Otoh, it is targeted at FF44. How hard should I investigate this?
Flags: needinfo?(mconley)
(In reply to David Rajchenbach-Teller [:Yoric] (please use "needinfo") from comment #71) > mconley, for some reason, my patch doesn't behave on e10s. Otoh, it is > targeted at FF44. How hard should I investigate this? Can you give me more detail? It's targeted for FF44, but according to the tracking flags, affects versions all the way to Nightly 47. Out of curiosity, when you say "doesn't behave", what do you mean? We don't currently support e10s on release (44), so things that break is there are fine - I'm just interested in knowing how it affects 45 (where we're running an e10s-enabled Telemetry experiment) and 46/47 (where it's on by default).
Flags: needinfo?(mconley) → needinfo?(dteller)
mconley, see comment 72 for the Try Run. This is a patch on top of FF44. I am currently testing a patch for more recent versions.
Flags: needinfo?(dteller)
Yoric, would you mind pushing the changes to sanitize.js, since those are unlikely to cause issues and they will likely bitrot bug 1244650?
Flags: needinfo?(dteller)
Component: Places → Session Restore
Product: Toolkit → Firefox
Comment on attachment 8714298 [details] MozReview Request: Bug 1243549 - SessionFile.wipe() now waits until SessionFile has been properly initialized;r?mconley Review request updated; see interdiff: https://reviewboard.mozilla.org/r/32997/diff/2-3/
Comment on attachment 8714759 [details] MozReview Request: Bug 1243549 - Making sure that startup sanitization doesn't throw because it can't find a tabbrowser;r=mak Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33237/diff/1-2/
Attachment #8714759 - Attachment description: MozReview Request: Bug 1243549 - Making sure that startup sanitization doesn't throw because it can't find a tabbrowser;r?mak → MozReview Request: Bug 1243549 - Making sure that startup sanitization doesn't throw because it can't find a tabbrowser;r=mak
Comment on attachment 8714298 [details] MozReview Request: Bug 1243549 - SessionFile.wipe() now waits until SessionFile has been properly initialized;r?mconley https://reviewboard.mozilla.org/r/32997/#review30299 Already reviewed.
Comment on attachment 8714759 [details] MozReview Request: Bug 1243549 - Making sure that startup sanitization doesn't throw because it can't find a tabbrowser;r=mak https://reviewboard.mozilla.org/r/33237/#review30301 Already reviewed.
Attachment #8714759 - Flags: review+
Yoric, Mak, Mike: Have we fully understand the fix here? Is it verified and stable enough to be uplifted to m-r in the next few hours? Do you think this is something critical enough to be taken as a ride-along in 44.0.1? I am trying to decide whether I should hold 44.0.1 on this or go ahead without it. Thanks!
Flags: needinfo?(mconley)
Flags: needinfo?(mak77)
Flags: needinfo?(dteller)
(In reply to Ritu Kothari (:ritu) from comment #82) > Yoric, Mak, Mike: Have we fully understand the fix here? Is it verified and > stable enough to be uplifted to m-r in the next few hours? Do you think this > is something critical enough to be taken as a ride-along in 44.0.1? I am > trying to decide whether I should hold 44.0.1 on this or go ahead without > it. Thanks! I'm afraid it's only landed on the fx-team branch, so hasn't hit any of our Nightly population yet. I'm not fully confident that we could land this on mozilla-release directly and not experience unforeseen regressions without more time to bake on Nightly. If, however, Yoric is sufficiently confident that this is a safe patch (which he might be - he's likely more familiar with the nuances of SessionFile than I am), then that's good enough for me.
Flags: needinfo?(mconley)
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Flags: needinfo?(mak77)
I'd be happy for a few days of testing on Nightly. This touches SessionFile, which is quite sensitive. Plus it's a JS patch, so who knows what kind of `undefined` will pop up?
Flags: needinfo?(dteller)
Approval Request Comment [Feature/regressing bug #]: Probably bug 1089695. [User impact if declined]: « When I run (launch, open...) Firefox for the first time during a session I get a blank page instead of my usual home page » for users who clean up cookies or history during shutdown. Also, risks of Session Restore dataloss. [Describe test coverage new/current, TreeHerder]: Currently in Nightly. Passes all non-e10s tests on 44, all tests on 46+. Not tested on 45 yet. [Risks and why]: The patch is simple. However, if it fails miserably, we'll end up with Session Restore dataloss for a few users. This is why I'd like to let it cook a few nights first. [String/UUID change made/needed]:
Attachment #8716336 - Flags: review+
Attachment #8716336 - Flags: approval-mozilla-beta?
Attachment #8716336 - Flags: approval-mozilla-aurora?
Attachment #8716336 - Attachment description: SessionFile.wipe() now waits until SessionFile has been properly initialized → SessionFile.wipe() now waits until SessionFile has been properly initialized (uplift edition).
Attachment #8716336 - Flags: approval-mozilla-beta?
Same objective as previous patch, but there shouldn't be any risk involved.
Attachment #8716338 - Flags: review+
Attachment #8716338 - Flags: approval-mozilla-beta?
Attachment #8716338 - Attachment description: Making sure that startup sanitization doesn't throw because it can't find a tabbrowser → Making sure that startup sanitization doesn't throw because it can't find a tabbrowser (uplift edition)
Comment on attachment 8716336 [details] [diff] [review] SessionFile.wipe() now waits until SessionFile has been properly initialized (uplift edition). (sorry, wrong approval flags)
Attachment #8716336 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
David, this uplift assumes bug 1245065 will also be uplifted, that may have a different timing. In the meanwhile sanitize would be broken due to the coding mistake not properly returning the promise. What about moving that fix here and let bug 1245065 just handle cookies? It would be simpler to track also for release drivers. You could land just that small piece here (I just added a review comment on that bug, but it's r=mak regardless).
Flags: needinfo?(dteller)
(In reply to Marco Bonardo [::mak] from comment #89) > David, this uplift assumes bug 1245065 will also be uplifted, that may have > a different timing. > In the meanwhile sanitize would be broken due to the coding mistake not > properly returning the promise. > > What about moving that fix here and let bug 1245065 just handle cookies? It > would be simpler to track also for release drivers. You could land just that > small piece here (I just added a review comment on that bug, but it's r=mak > regardless). Oops, right, I landed that fix on the wrong bug. Thanks for spotting that.
Flags: needinfo?(dteller)
See comment 85 for Approval Request Comment.
Attachment #8716338 - Attachment is obsolete: true
Attachment #8716338 - Flags: approval-mozilla-beta?
Attachment #8716649 - Flags: review+
Attachment #8716649 - Flags: approval-mozilla-beta?
David, I will start the build of beta 4 today. Beta 5 gtb is next Thursday. Will it be enough for you? Thanks
Hey alex_mayorga, this patch has been on Nightly for a few days. Have you seen the session restore bug within that time?
(In reply to Sylvestre Ledru [:sylvestre] from comment #92) > David, I will start the build of beta 4 today. Beta 5 gtb is next Thursday. > Will it be enough for you? Thanks I'm not sure I understand the question. Are you asking me if we should uplift to Beta5 ?
Flags: needinfo?(sledru)
I have received feedback that cookies are not being deleted on Firefox exit, even when the appropriate setting is enabled. Would that issue be included in this bug, or is that a separate issue that needs to be investigated, and possibly a separate bug filed?
Flags: needinfo?(dteller)
I must clarify. The reports were for 44.0.0, not for your patch.
(In reply to quality+bugzilla from comment #95) > I have received feedback that cookies are not being deleted on Firefox exit, > even when the appropriate setting is enabled. Would that issue be included > in this bug, or is that a separate issue that needs to be investigated, and > possibly a separate bug filed? That's bug 1245578.
Flags: needinfo?(dteller)
Comment on attachment 8716649 [details] [diff] [review] Making sure that startup sanitization doesn't throw because it can't find a tabbrowser A priori, no regression, let's take it! Should be in 45 beta 5. Merci David!
Flags: needinfo?(sledru)
Attachment #8716649 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8716336 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Not critical enough for a 44 dot release
Comment on attachment 8716336 [details] [diff] [review] SessionFile.wipe() now waits until SessionFile has been properly initialized (uplift edition). [Triage Comment] discussing on irc with Yoric, we need that in aurora too.
Attachment #8716336 - Flags: approval-mozilla-aurora+
Attachment #8716649 - Flags: approval-mozilla-aurora+
The patch that just landed is the wrong one, it has the same bug as Nightly. this one => https://hg.mozilla.org/releases/mozilla-aurora/rev/35bfade35158 That was exactly my fear in comment 89.
Flags: needinfo?(dteller)
In comment 105 I just landed the fix for the broken patch that made Nightly. I still have to fix the broken landing on Aurora and Beta. Aurora should just need an uplift of this last patch, while beta needs additionally the same fix in the canClear method.
Flags: needinfo?(dteller) → needinfo?(mak77)
Flags: qe-verify+
Reproduced the initial issue on Windows 7 x64 using the str from comment 57 with Firefox 44.0.2, build ID: 20160210153822. Confirming the fix on: *latest 47.0a1 Nightly, build ID 20160216030245 *latest 46.0a2 Aurora, build ID 20160216004009 *Fx 45.0b6, build ID 20160215141016.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
QA Contact: cornel.ionce
I backed out the sanitize.js bits in bug 1248489 along with all the other related changes in bug 1248489 https://hg.mozilla.org/releases/mozilla-beta/rev/6eb8ca2c57ec
ehr, the backout is only for Firefox 45.
Depends on: 1251347
¡Hola Mike! It is most likely irrelevant, but I haven't seen https://bugzilla.mozilla.org/show_bug.cgi?id=1217904 as of lately. ¡Gracias! Alex
Flags: needinfo?(alex_mayorga)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: