Tab switching using Ctrl+Tab is broken yet again in e10s

RESOLVED FIXED in Firefox 35

Status

()

Firefox
Tabbed Browser
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Away for a while, Assigned: Away for a while)

Tracking

unspecified
Firefox 35
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(e10s?)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

3 years ago
Created attachment 8489538 [details]
Callstack

This time it's:

TypeError: aWindow is null PageThumbs.jsm:384
(Assignee)

Comment 1

3 years ago
Created attachment 8489539 [details] [diff] [review]
Disable taking tab previews in more places in e10s; r=dao
(Assignee)

Updated

3 years ago
Attachment #8489539 - Flags: review?(dao)
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #0)
> Created attachment 8489538 [details]
> Callstack
> 
> This time it's:
> 
> TypeError: aWindow is null PageThumbs.jsm:384

Where is this called from and why is it not covered by <http://hg.mozilla.org/mozilla-central/annotate/989c1c75889c/browser/base/content/browser-thumbnails.js#l128>?
Comment on attachment 8489539 [details] [diff] [review]
Disable taking tab previews in more places in e10s; r=dao

This doesn't look right to me. Neither updatePreviews nor updatePreview directly mess with the content window and the path that would do it should already be covered.
Attachment #8489539 - Flags: review?(dao) → review-
(Assignee)

Comment 4

3 years ago
(In reply to Dão Gottwald [:dao] from comment #2)
> (In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from
> comment #0)
> > Created attachment 8489538 [details]
> > Callstack
> > 
> > This time it's:
> > 
> > TypeError: aWindow is null PageThumbs.jsm:384
> 
> Where is this called from and why is it not covered by
> <http://hg.mozilla.org/mozilla-central/annotate/989c1c75889c/browser/base/
> content/browser-thumbnails.js#l128>?

I attached the screenshot as an attempt to answer this question.

(In reply to Dão Gottwald [:dao] from comment #3)
> Comment on attachment 8489539 [details] [diff] [review]
> Disable taking tab previews in more places in e10s; r=dao
> 
> This doesn't look right to me. Neither updatePreviews nor updatePreview
> directly mess with the content window and the path that would do it should
> already be covered.

Please suggest an alternative fix then.  I don't understand how else we would fix this.
Flags: needinfo?(dao)
It seems like tabPreviews_capture should guard the PageThumbs_captureToCanvas call with a gMultiProcessBrowser check.
Flags: needinfo?(dao)
It could also just return early:

if (gMultiProcessBrowser)
  return new Image;
(Assignee)

Updated

3 years ago
Attachment #8489539 - Attachment is obsolete: true
(Assignee)

Comment 7

3 years ago
Created attachment 8490402 [details] [diff] [review]
Don't attempt to capture a tab preview in e10s; r=dao
(Assignee)

Updated

3 years ago
Attachment #8490402 - Flags: review?(dao)

Updated

3 years ago
Attachment #8490402 - Flags: review?(dao) → review+
(Assignee)

Comment 8

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/580401e8e95e
(Assignee)

Updated

3 years ago
Depends on: 698371
https://hg.mozilla.org/mozilla-central/rev/580401e8e95e
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
You need to log in before you can comment on or make changes to this bug.