Closed Bug 1129266 Opened 8 years ago Closed 7 years ago

[e10s] Don't display the tab crash badge on not-yet-restored tabs.

Categories

(Firefox :: General, defect)

37 Branch
x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: johan.charlez, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [STR in comment 7])

Attachments

(1 file)

Updating the summary to better suit the bug.
Summary: [e10s] The tab crash icon might confuse and alarm users. → [e10s] Don't display the tab crash badge on not-yet-restored tabs.
Blocks: 1112304, fxe10s
No longer blocks: e10s
I'm a little confused by what you're asking for. The red icon displays when the process used to display the tab has crashed (right now all tabs use the same process). It goes away when you restore the tab, either by loading something new in it or clicking one of the buttons to restore that tab or every tab.

What are you suggesting we do instead?
(In reply to Dave Townsend [:mossop] from comment #2)
> I'm a little confused by what you're asking for. The red icon displays when
> the process used to display the tab has crashed (right now all tabs use the
> same process). It goes away when you restore the tab, either by loading
> something new in it or clicking one of the buttons to restore that tab or
> every tab.
> 
> What are you suggesting we do instead?

I realise I must've described this poorly, and I shouldn't have reported several bugs in one. Doh, my bad. :)

The first two tabs in the screenshot have not yet been restored (not loaded). I'm suggesting that the tab crash badge is not displayed on these. While the tab-process crashed, these tabs didn't, so there shouldn't be any need to inform the user about these tabs.
(In reply to Johan C from comment #3)
> (In reply to Dave Townsend [:mossop] from comment #2)
> > I'm a little confused by what you're asking for. The red icon displays when
> > the process used to display the tab has crashed (right now all tabs use the
> > same process). It goes away when you restore the tab, either by loading
> > something new in it or clicking one of the buttons to restore that tab or
> > every tab.
> > 
> > What are you suggesting we do instead?
> 
> I realise I must've described this poorly, and I shouldn't have reported
> several bugs in one. Doh, my bad. :)
> 
> The first two tabs in the screenshot have not yet been restored (not
> loaded). I'm suggesting that the tab crash badge is not displayed on these.
> While the tab-process crashed, these tabs didn't, so there shouldn't be any
> need to inform the user about these tabs.

What does "not yet restored mean"? Do you mean the process crashed, you clicked restore all tabs but you haven't yet gone to those tabs to make them reload?
(In reply to Dave Townsend [:mossop] from comment #4)
> What does "not yet restored mean"? Do you mean the process crashed, you
> clicked restore all tabs but you haven't yet gone to those tabs to make them
> reload?

The "Don’t load tabs until selected"-preference is checked. The contents of the first two tabs were never loaded into in RAM.
(In reply to Johan C from comment #5)
> (In reply to Dave Townsend [:mossop] from comment #4)
> > What does "not yet restored mean"? Do you mean the process crashed, you
> > clicked restore all tabs but you haven't yet gone to those tabs to make them
> > reload?
> 
> The "Don’t load tabs until selected"-preference is checked. The contents of
> the first two tabs were never loaded into in RAM.

Oh I see after startup. So that's an interesting case and I'm not sure what we should do about it. Technically the tab is actually crashed, even though we haven't restored the tab yet it is still connected to the remote process, just displaying about:blank.

The way things stand now if the content process crashes and then you switch to the pending tab it still displays the crashed tab UI and doesn't restore, so it doesn't make sense to hide the crashed icon for these tabs. However it might make change that and instead make them immediately restore when switched to with no additional user interaction (they couldn't have caused the process crash after all) and then we could hide the crashed icon for these tabs.

Michael, what are your thoughts?
Flags: needinfo?(mmaslaney)
Marking the bug description as obsolete, adding steps to reproduce this bug with this comment.
----------------------------------------------------------------------------------------------

STR:
1. Make sure "Don’t load tabs until selected" is checked.
2. Open at least two tabs and restart Firefox.
3. Make sure that a least one tab has yet to be restored (the contents of this tab should not be loaded into RAM).
4. Crash the content process.


Expected:
The tabs that have yet to be restored should not have a tab crash badge. These tabs shouldn't crash, or at the very least there shouldn't be a need to inform the user that these tabs have crashed.


Actual:
(See screenshot - the first two tabs were never restored.)
Not yet restored tabs crash. Clicking one of these tabs to restore it will also restore its contents (even with "Don’t load tabs until selected" checked).
(In reply to Johan C from comment #7)
> Actual:
> Not yet restored tabs crash. Clicking one of these tabs to restore it will
> also restore its contents (even with "Don’t load tabs until selected"
> checked).

This doesn't match what I found when I tried. Clicking one of the tabs didn't restore it, it just showed the tab crashed UI, which is what I'd expect based on the code.
(In reply to Dave Townsend [:mossop] from comment #6)
> Oh I see after startup. So that's an interesting case and I'm not sure what
> we should do about it. Technically the tab is actually crashed, even though
> we haven't restored the tab yet it is still connected to the remote process,
> just displaying about:blank.
Yeah, sorry for the poor description :(


(In reply to Dave Townsend [:mossop] from comment #6)
> The way things stand now if the content process crashes and then you switch
> to the pending tab it still displays the crashed tab UI and doesn't restore,
(In reply to Dave Townsend [:mossop] from comment #8)
> This doesn't match what I found when I tried. Clicking one of the tabs
> didn't restore it, it just showed the tab crashed UI, which is what I'd
> expect based on the code.
Interesting, one of them restored when I selected it. The badge disappeared automatically from the other non-loaded tab after a while.
Whiteboard: [STR in comment 7]
(In reply to Johan C from comment #9)
> (In reply to Dave Townsend [:mossop] from comment #8)
> > This doesn't match what I found when I tried. Clicking one of the tabs
> > didn't restore it, it just showed the tab crashed UI, which is what I'd
> > expect based on the code.
> Interesting, one of them restored when I selected it. The badge disappeared
> automatically from the other non-loaded tab after a while.

That's odd. You don't happen to be running any add-on that interacts with session restore at all? Alternatively did the crash happen immediately when the tabs were loading at startup? There might be an edge case where if a tab crashes before session restore has put another tab into the pending state then maybe that might cause what you describe.
(In reply to Dave Townsend [:mossop] from comment #10)
> That's odd. You don't happen to be running any add-on that interacts with
> session restore at all?
I believe https://crash-stats.mozilla.com/report/index/7a36d1a8-253a-42b9-8aad-47ea42150204 is the crash report. I only had/have adblock plus and restartless restart installed.

> Alternatively did the crash happen immediately when
> the tabs were loading at startup? There might be an edge case where if a tab
> crashes before session restore has put another tab into the pending state
> then maybe that might cause what you describe.
I'm afraid I don't remember, but since the tab spinner was spinning there are two possibilies:
  1. I had just started Nightly.
  2. Nightly was already running and when Windows ran out of RAM the tab spinner appeared (If this is even possible).
Is there a log with timestamps for startup/shutdown etc. I can view/attach/send?
(In reply to Johan C from comment #11)
> https://crash-stats.mozilla.com/report/index/7a36d1a8-253a-42b9-8aad-
> 47ea42150204 is the crash report. I only had/have adblock plus and
> restartless restart installed.
Or it's this one: https://crash-stats.mozilla.com/report/index/c9bf21f0-9061-4365-b74c-6f0ca2150204.
Mike, didn't this get fixed as described in the summary / comment #7 ? At least, I recall you doing work in this area...
Flags: needinfo?(mmaslaney) → needinfo?(mconley)
Yes, this got fixed as part of bug 1209689.
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(mconley)
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.