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

Status: RESOLVED DUPLICATE of bug 1345098

(Firefox :: Tabbed Browser, enhancement, P2)

Description

[u462496]

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.

Comment 1

[u462496]

Attachment: Patch - browser.js

Comment 2

[u462496]

Attachment: Patch - browser-addons.js

Comment 3

[u462496]

Attachment: Patch - WebContentConverter.js

Comment 4

[u462496]

Attachment: Patch - search.xml

Comment 5

[u462496]

Attachment: Patch - TabStateFlusher.jsm

Comment 6

[u462496]

Attachment: Patch - ContentCrashHandlers.jsm

Comment 7

[Dão Gottwald [:dao]]

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.

Comment 8

[u462496]

Attachment: Patch - NetworkPrioritizer.jsm

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 8774679 [details] [diff] [review]
Patch - browser-addons.js

same here

Comment 10

[u462496]

Attachment: Patch - devtools

Comment 11

[u462496]

Attachment: Patch - aboutPerformance.js

Comment 12

[Dão Gottwald [:dao]]

Comment on attachment 8774680 [details] [diff] [review]
Patch - WebContentConverter.js

Ugh, we should probably make getTabForBrowser actually return the tab.

Comment 13

[u462496]

(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 :-/

Comment 14

[u462496]

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

Comment 15

[Dão Gottwald [:dao]]

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?

Comment 16

[u462496]

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

Comment 17

[Dão Gottwald [:dao]]

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

Comment 18

[u462496]

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

Comment 19

[u462496]

I suppose we could move _tabForBrowser.set to addTab and set it with the linkedBrowser proxy?

Comment 20

[u462496]

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

Comment 21

[u462496]

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?

Comment 22

[Dão Gottwald [:dao]]

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.

Comment 23

[u462496]

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?

Comment 24

[Dão Gottwald [:dao]]

My suggestion would be isProxy.

Comment 25

[u462496]

The strategy of lazy-browsers has changed significantly since this bug was started.  I am closing this bug in favor of bug 1345098.