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

VERIFIED FIXED in Firefox 45

Status

()

VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: yesyesrun, Assigned: Yoric, NeedInfo)

Tracking

(Depends on: 1 bug, {regression})

44 Branch
Firefox 47
Unspecified
Windows 7
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44- wontfix, firefox45+ verified, firefox46 verified, firefox47 verified)

Details

Attachments

(7 attachments, 1 obsolete attachment)

110.69 KB, image/png
Details
58 bytes, text/x-review-board-request
mconley
: review+
Yoric
: review+
Details
4.17 KB, text/plain
Details
2.06 KB, text/plain
Details
58 bytes, text/x-review-board-request
mak
: review+
Yoric
: review+
Details
5.68 KB, patch
Yoric
: review+
Details | Diff | Splinter Review
6.99 KB, patch
Yoric
: review+
Details | Diff | Splinter Review
(Reporter)

Description

3 years ago
Created attachment 8712894 [details]
Capture.PNG

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)

Comment 1

3 years ago
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)
(Reporter)

Comment 2

3 years ago
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)
(Reporter)

Comment 3

3 years ago
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.
(Reporter)

Comment 4

3 years ago
Same problem if I install AdBlock Plus.
(Reporter)

Comment 5

3 years ago
Forget about uBlock Origin and AdBlock Plus. The same issue occurs even if I remove them.
(Reporter)

Updated

3 years ago
Component: Untriaged → General
OS: Unspecified → Windows 7

Comment 6

3 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

3 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.
(Reporter)

Comment 8

3 years ago
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

3 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

3 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

3 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

3 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)

Updated

3 years ago
Duplicate of this bug: 1244577

Comment 14

3 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

3 years ago
Yes, I get that error, as well as the other errors reported in that thread.
Flags: needinfo?(quality+bugzilla)

Comment 16

3 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 18

3 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

3 years ago
When the home page is not shown:
privacy.sanitize.sanitizeInProgress does not exist
privacy.sanitize.didShutdownSanitize is true

Comment 20

3 years ago
When the home page *IS* shown, those two prefs are the same.

Comment 21

3 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
See Also: → bug 1071182

Comment 22

3 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

3 years ago
Actually the checkbox is called 'Remember search and form history'.  The bug happens when it is UNchecked as well.

Comment 24

3 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

3 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

3 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

3 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.
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)
Created attachment 8714298 [details]
MozReview Request: Bug 1243549 - SessionFile.wipe() now waits until SessionFile has been properly initialized;r?mconley

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.

Comment 34

3 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'...
See Also: → bug 1244650
(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.
Keywords: regressionwindow-wanted
I confirm that the exception of comment 21 is basically logged and ignored, so it cannot directly cause the bug at hand.

Comment 38

3 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

3 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
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?

Comment 43

3 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

3 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

3 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.)
(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

3 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.
Created attachment 8714751 [details]
Shutdown spew

Quite possibly unrelated, but I get lots of shutdown errors in sanitize.

Comment 49

3 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.
Created attachment 8714753 [details]
Startup spew

... 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

3 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.
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)
Created attachment 8714759 [details]
MozReview Request: Bug 1243549 - Making sure that startup sanitization doesn't throw because it can't find a tabbrowser;r=mak

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.
Filed the shutdown hangs as bug 1245065.
See Also: → bug 1245065

Comment 57

3 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.
Thanks a lot.
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)

Comment 62

3 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.
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
Depends on: 1245101
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.
Ah, simple typo.
Assignee: nobody → dteller
Sylvestre: if you need data on gravity, here it is: http://forums.mozillazine.org/viewtopic.php?f=38&t=2986649
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)

Comment 84

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a768b93e7f0f
https://hg.mozilla.org/mozilla-central/rev/044c7060349e
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox47: affected → fixed
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)
Created attachment 8716336 [details] [diff] [review]
SessionFile.wipe() now waits until SessionFile has been properly initialized (uplift edition).

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?
Created attachment 8716338 [details] [diff] [review]
Making sure that startup sanitization doesn't throw because it can't find a tabbrowser (uplift edition)

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)
Created attachment 8716649 [details] [diff] [review]
Making sure that startup sanitization doesn't throw because it can't find a tabbrowser

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)

Comment 95

3 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

3 years ago
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
status-firefox44: affected → wontfix
tracking-firefox44: ? → -
tracking-firefox45: ? → +
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+

Updated

3 years ago
Duplicate of this bug: 1248176
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
status-firefox45: fixed → verified
status-firefox46: fixed → verified
status-firefox47: fixed → verified
Flags: qe-verify+
QA Contact: cornel.ionce

Updated

3 years ago
Duplicate of this bug: 1249762

Updated

3 years ago
Duplicate of this bug: 1249994
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.

Updated

3 years ago
Depends on: 1251347

Comment 116

3 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.