Lazy-browser-tabs: modify tabbrowser call sites to handle lazy-browser tabs

RESOLVED DUPLICATE of bug 1345098

Status

()

Firefox
Tabbed Browser
P2
enhancement
RESOLVED DUPLICATE of bug 1345098
a year ago
9 months ago

People

(Reporter: Kevin Jones, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(9 attachments)

(Reporter)

Description

a year ago
Support bug for bug 906076

Before implementing full optimization of lazy-browser tabs in SessionStore, call sites which access tab.linkedBrowser must be prepared so tabs in lazy-browser state are handled properly.

This bug will provide a series of patches to do that.
(Reporter)

Updated

a year ago
Blocks: 906076
Priority: -- → P2
(Reporter)

Comment 1

a year ago
Created attachment 8774678 [details] [diff] [review]
Patch - browser.js
Attachment #8774678 - Flags: review?(dao+bmo)
(Reporter)

Comment 2

a year ago
Created attachment 8774679 [details] [diff] [review]
Patch - browser-addons.js
Attachment #8774679 - Flags: review?(dao+bmo)
(Reporter)

Comment 3

a year ago
Created attachment 8774680 [details] [diff] [review]
Patch - WebContentConverter.js
Attachment #8774680 - Flags: review?(dao+bmo)
(Reporter)

Comment 4

a year ago
Created attachment 8774681 [details] [diff] [review]
Patch - search.xml
Attachment #8774681 - Flags: review?(dao+bmo)
(Reporter)

Comment 5

a year ago
Created attachment 8774683 [details] [diff] [review]
Patch - TabStateFlusher.jsm
Attachment #8774683 - Flags: review?(dao+bmo)
(Reporter)

Comment 6

a year ago
Created attachment 8774685 [details] [diff] [review]
Patch - ContentCrashHandlers.jsm
Attachment #8774685 - Flags: review?(dao+bmo)
Comment on attachment 8774678 [details] [diff] [review]
Patch - browser.js

Rather than changing all gBrowser.browsers loops to gBrowser.tabs loops, I think we should add a flag on the browser proxies to indicate that they're not real browsers.
Attachment #8774678 - Flags: review?(dao+bmo) → review-
(Reporter)

Comment 8

a year ago
Created attachment 8774686 [details] [diff] [review]
Patch - NetworkPrioritizer.jsm
Attachment #8774686 - Flags: review?(dao+bmo)
Comment on attachment 8774679 [details] [diff] [review]
Patch - browser-addons.js

same here
Attachment #8774679 - Flags: review?(dao+bmo) → review-
(Reporter)

Comment 10

a year ago
Created attachment 8774687 [details] [diff] [review]
Patch - devtools
Attachment #8774687 - Flags: review?(dao+bmo)
(Reporter)

Comment 11

a year ago
Created attachment 8774688 [details] [diff] [review]
Patch - aboutPerformance.js
Attachment #8774688 - Flags: review?(dao+bmo)
Comment on attachment 8774680 [details] [diff] [review]
Patch - WebContentConverter.js

Ugh, we should probably make getTabForBrowser actually return the tab.
Attachment #8774680 - Flags: review?(dao+bmo) → review-
(Reporter)

Comment 13

a year ago
(In reply to Dão Gottwald [:dao] from comment #7)
> Comment on attachment 8774678 [details] [diff] [review]
> Patch - browser.js
> 
> Rather than changing all gBrowser.browsers loops to gBrowser.tabs loops, I
> think we should add a flag on the browser proxies to indicate that they're
> not real browsers.

Oops, didn't see your comment before uploading all of these :-/
(Reporter)

Comment 14

a year ago
(In reply to Dão Gottwald [:dao] from comment #7)
> Comment on attachment 8774678 [details] [diff] [review]
> Patch - browser.js
> 
> Rather than changing all gBrowser.browsers loops to gBrowser.tabs loops, I
> think we should add a flag on the browser proxies to indicate that they're
> not real browsers.

What about using the return value from getTabForBrowser as a flag instead, like I did in WebContentConverter.js? (and of course returning the tab like you said)
How could getTabForBrowser be used as a flag when it returns the tab as it should? And how would that be better than an explicit flag on the proxy?
(Reporter)

Comment 16

a year ago
(In reply to Dão Gottwald [:dao] from comment #15)
> How could getTabForBrowser be used as a flag when it returns the tab as it
> should? And how would that be better than an explicit flag on the proxy?

getTabForBrowser will return `undefined` if the browser is not real.

Putting a flag on the proxy is tricky, because any property on the browser will force instantiation.  But I suppose we could use a trap in the `linkedBrowser` proxy to ignore it?
(In reply to Allasso Travesser from comment #16)
> (In reply to Dão Gottwald [:dao] from comment #15)
> > How could getTabForBrowser be used as a flag when it returns the tab as it
> > should? And how would that be better than an explicit flag on the proxy?
> 
> getTabForBrowser will return `undefined` if the browser is not real.

It shouldn't. It should return the tab.
(Reporter)

Comment 18

a year ago
(In reply to Dão Gottwald [:dao] from comment #17)
> > getTabForBrowser will return `undefined` if the browser is not real.
> 
> It shouldn't. It should return the tab.

getTabForBrowser returns tabbrowser._tabForBrowser.get(aBrowser).  If a tab is lazy, _tabForBrowser never gets set, because there is no browser to set it with.
(Reporter)

Comment 19

a year ago
I suppose we could move _tabForBrowser.set to addTab and set it with the linkedBrowser proxy?
(Reporter)

Comment 20

a year ago
(In reply to Allasso Travesser from comment #19)
> I suppose we could move _tabForBrowser.set to addTab and set it with the
> linkedBrowser proxy?

Hmmm... not sure if that would work.
(Reporter)

Comment 21

a year ago
Actually I think it will work.

We could move _tabForBrowser.set to addTab and set it with the linkedBrowser proxy.  Then when we iterate over `browsers` we could use getTabForBrowser to retrieve the tab and test for tab.hasBrowser.  Then we wouldn't need to introduce another flag in the `browsers` proxy and deal with the complications of that.

What do you think?
Getting the tab from a browser in order to figure out whether the browser is real is just cumbersome. We should have a better API for that.
(Reporter)

Comment 23

a year ago
Well actually, it wouldn't be that hard I guess.  I don't think we need to do anything in the `browsers` proxy.  We just need a `isRealBrowser` property for the browser, and in the linkedBrowser proxy just return false without instantiating the browser.  When the real browser is created, we set `isRealBrowser` on the browser.

Does that sound okay?  Have a suggestion what to call the flag?
My suggestion would be isProxy.
Attachment #8774681 - Flags: review?(dao+bmo)
Attachment #8774683 - Flags: review?(dao+bmo)
Attachment #8774688 - Flags: review?(dao+bmo)
Attachment #8774685 - Flags: review?(dao+bmo)
Attachment #8774686 - Flags: review?(dao+bmo)
Attachment #8774687 - Flags: review?(dao+bmo)
(Reporter)

Comment 25

9 months ago
The strategy of lazy-browsers has changed significantly since this bug was started.  I am closing this bug in favor of bug 1345098.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1345098
You need to log in before you can comment on or make changes to this bug.