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)
Tracking
()
VERIFIED
FIXED
Firefox 47
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+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Yoric
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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.
Forget about uBlock Origin and AdBlock Plus. The same issue occurs even if I remove them.
Comment 6•9 years ago
|
||
If you restart in safe mode (Help > Restart with add-ons disabled), does this still happen?
Component: General → Session Restore
Flags: needinfo?(yesyesrun)
Comment 7•9 years ago
|
||
(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)
Comment 9•9 years ago
|
||
(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)
Reporter | ||
Comment 10•9 years ago
|
||
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)
Reporter | ||
Comment 11•9 years ago
|
||
Onestly, the only real connection I can see with the issue is the update to Firefox 44. No problems at all before that.
Comment 12•9 years ago
|
||
(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)
Comment 14•9 years ago
|
||
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)
Comment 15•9 years ago
|
||
Yes, I get that error, as well as the other errors reported in that thread.
Flags: needinfo?(quality+bugzilla)
Comment 16•9 years ago
|
||
(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)
Comment 17•9 years ago
|
||
There is also a report of the same bug here:
http://www.ghacks.net/2016/01/25/firefox-44-find-out-what-is-new/#comment-3802776
Comment 18•9 years ago
|
||
(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)
Comment 19•9 years ago
|
||
When the home page is not shown:
privacy.sanitize.sanitizeInProgress does not exist
privacy.sanitize.didShutdownSanitize is true
Comment 20•9 years ago
|
||
When the home page *IS* shown, those two prefs are the same.
Comment 21•9 years ago
|
||
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
status-firefox44:
--- → affected
status-firefox45:
--- → affected
status-firefox46:
--- → affected
status-firefox47:
--- → affected
tracking-firefox44:
--- → ?
tracking-firefox45:
--- → ?
Component: Session Restore → Places
Ever confirmed: true
Flags: needinfo?(dteller)
Keywords: regression
Product: Firefox → Toolkit
Comment 22•9 years ago
|
||
(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.
Comment 23•9 years ago
|
||
Actually the checkbox is called 'Remember search and form history'. The bug happens when it is UNchecked as well.
Comment 24•9 years ago
|
||
(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?
Comment 25•9 years ago
|
||
(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.
Comment 26•9 years ago
|
||
(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.
Comment 27•9 years ago
|
||
(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.
Assignee | ||
Comment 28•9 years ago
|
||
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)
Assignee | ||
Comment 29•9 years ago
|
||
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)
Comment 30•9 years ago
|
||
(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.
Assignee | ||
Comment 31•9 years ago
|
||
(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.
Updated•9 years ago
|
Attachment #8714298 -
Flags: review?(mak77)
Comment 32•9 years ago
|
||
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.
Comment 33•9 years ago
|
||
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.
Comment 34•9 years ago
|
||
(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'...
Updated•9 years ago
|
Comment 35•9 years ago
|
||
(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.
Comment 36•9 years ago
|
||
(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.
Keywords: regressionwindow-wanted
Assignee | ||
Comment 37•9 years ago
|
||
I confirm that the exception of comment 21 is basically logged and ignored, so it cannot directly cause the bug at hand.
Comment 38•9 years ago
|
||
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.
Comment 39•9 years ago
|
||
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
Assignee | ||
Comment 40•9 years ago
|
||
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)
Assignee | ||
Comment 41•9 years ago
|
||
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.
Assignee | ||
Comment 42•9 years ago
|
||
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?
Comment 43•9 years ago
|
||
(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.
Comment 44•9 years ago
|
||
(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)
Comment 45•9 years ago
|
||
I get about a 60% failure rate of trying to load the home page. (Or, on the bright side, a 40% success rate.)
Assignee | ||
Comment 46•9 years ago
|
||
(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?
Comment 47•9 years ago
|
||
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.
Assignee | ||
Comment 48•9 years ago
|
||
Quite possibly unrelated, but I get lots of shutdown errors in sanitize.
Comment 49•9 years ago
|
||
(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.
Assignee | ||
Comment 50•9 years ago
|
||
... 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)
Comment 51•9 years ago
|
||
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.
Assignee | ||
Comment 52•9 years ago
|
||
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)
Assignee | ||
Comment 53•9 years ago
|
||
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)
Assignee | ||
Comment 54•9 years ago
|
||
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)
Assignee | ||
Comment 55•9 years ago
|
||
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.
Assignee | ||
Comment 56•9 years ago
|
||
Filed the shutdown hangs as bug 1245065.
Comment 57•9 years ago
|
||
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.
Assignee | ||
Comment 58•9 years ago
|
||
Thanks a lot.
Assignee | ||
Comment 59•9 years ago
|
||
This seems to confirm that my fix is sufficient to resolve the issue.
Assignee | ||
Comment 60•9 years ago
|
||
Sylvestre, any idea of the gravity of this bug?
Flags: needinfo?(sledru)
Comment 62•9 years ago
|
||
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.
Updated•9 years ago
|
Attachment #8714759 -
Flags: review?(mak77) → review+
Comment 63•9 years ago
|
||
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
Comment 64•9 years ago
|
||
Regression window using str in comment57:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=441f0297151adfb84169396d58989b795f1984fb&tochange=bbefa0967bad3e180a48d81a29d1e95e2534d320
Regressed by Bug 1089695
Keywords: regressionwindow-wanted
Assignee | ||
Comment 65•9 years ago
|
||
If anybody is eager to test, binaries will show up here: https://archive.mozilla.org/pub/firefox/try-builds/dteller@mozilla.com-f37c0c999c8f19d75cd0f87f1984dc67863538a9/
Comment 66•9 years ago
|
||
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)
Updated•9 years ago
|
Updated•9 years ago
|
Attachment #8714298 -
Flags: review?(mconley) → review+
Comment 67•9 years ago
|
||
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!
Assignee | ||
Comment 68•9 years ago
|
||
Unfortunately, my patch seems to orange a number of tests. I'm investigating.
Assignee | ||
Comment 69•9 years ago
|
||
Ah, simple typo.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → dteller
Assignee | ||
Comment 70•9 years ago
|
||
Sylvestre: if you need data on gravity, here it is: http://forums.mozillazine.org/viewtopic.php?f=38&t=2986649
Assignee | ||
Comment 71•9 years ago
|
||
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)
Assignee | ||
Comment 72•9 years ago
|
||
Comment 73•9 years ago
|
||
(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)
Assignee | ||
Comment 74•9 years ago
|
||
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)
Comment 75•9 years ago
|
||
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)
Updated•9 years ago
|
Component: Places → Session Restore
Product: Toolkit → Firefox
Assignee | ||
Comment 76•9 years ago
|
||
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/
Assignee | ||
Comment 77•9 years ago
|
||
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
Assignee | ||
Updated•9 years ago
|
Attachment #8714298 -
Flags: review+
Assignee | ||
Comment 78•9 years ago
|
||
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.
Assignee | ||
Comment 79•9 years ago
|
||
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+
Assignee | ||
Comment 80•9 years ago
|
||
Flags: needinfo?(dteller)
Keywords: checkin-needed
Comment 81•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/a768b93e7f0f
https://hg.mozilla.org/integration/fx-team/rev/044c7060349e
Keywords: checkin-needed
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)
Comment 83•9 years ago
|
||
(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)
Comment 84•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a768b93e7f0f
https://hg.mozilla.org/mozilla-central/rev/044c7060349e
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Updated•9 years ago
|
Flags: needinfo?(mak77)
Assignee | ||
Comment 85•9 years ago
|
||
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)
Assignee | ||
Comment 86•9 years ago
|
||
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?
Assignee | ||
Updated•9 years ago
|
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?
Assignee | ||
Comment 87•9 years ago
|
||
Same objective as previous patch, but there shouldn't be any risk involved.
Attachment #8716338 -
Flags: review+
Attachment #8716338 -
Flags: approval-mozilla-beta?
Assignee | ||
Updated•9 years ago
|
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)
Assignee | ||
Comment 88•9 years ago
|
||
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?
Comment 89•9 years ago
|
||
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)
Assignee | ||
Comment 90•9 years ago
|
||
(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)
Assignee | ||
Comment 91•9 years ago
|
||
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?
Comment 92•9 years ago
|
||
David, I will start the build of beta 4 today. Beta 5 gtb is next Thursday. Will it be enough for you? Thanks
Comment 93•9 years ago
|
||
Hey alex_mayorga, this patch has been on Nightly for a few days. Have you seen the session restore bug within that time?
Assignee | ||
Comment 94•9 years ago
|
||
(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 ?
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(sledru)
Comment 95•9 years ago
|
||
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)
Comment 96•9 years ago
|
||
I must clarify. The reports were for 44.0.0, not for your patch.
Assignee | ||
Comment 97•9 years ago
|
||
(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)
Assignee | ||
Comment 98•9 years ago
|
||
Actually, bug 1245065.
Comment 99•9 years ago
|
||
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+
Updated•9 years ago
|
Attachment #8716336 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 100•9 years ago
|
||
Not critical enough for a 44 dot release
Comment 101•9 years ago
|
||
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+
Updated•9 years ago
|
Attachment #8716649 -
Flags: approval-mozilla-aurora+
Comment 102•9 years ago
|
||
bugherder uplift |
Comment 103•9 years ago
|
||
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)
Comment 104•9 years ago
|
||
bugherder uplift |
Comment 106•9 years ago
|
||
bugherder uplift |
Comment 107•9 years ago
|
||
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.
Updated•9 years ago
|
Flags: needinfo?(dteller) → needinfo?(mak77)
Comment 108•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/25694899bc03
https://hg.mozilla.org/releases/mozilla-beta/rev/fb1653512406
Ok, we should be fine now.
Flags: needinfo?(mak77)
Updated•9 years ago
|
Flags: qe-verify+
Comment 109•9 years ago
|
||
bugherder |
Comment 111•9 years ago
|
||
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
Comment 114•9 years ago
|
||
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
Comment 115•9 years ago
|
||
ehr, the backout is only for Firefox 45.
Comment 116•9 years ago
|
||
¡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.
Description
•