Closed
Bug 1289370
Opened 8 years ago
Closed 7 years ago
Lazy-browser-tabs: modify tabbrowser call sites to handle lazy-browser tabs
Categories
(Firefox :: Tabbed Browser, enhancement, P2)
Firefox
Tabbed Browser
Tracking
()
RESOLVED
DUPLICATE
of bug 1345098
People
(Reporter: u462496, Unassigned)
References
Details
Attachments
(9 files)
1.32 KB,
patch
|
dao
:
review-
|
Details | Diff | Splinter Review |
952 bytes,
patch
|
dao
:
review-
|
Details | Diff | Splinter Review |
1.25 KB,
patch
|
dao
:
review-
|
Details | Diff | Splinter Review |
2.20 KB,
patch
|
Details | Diff | Splinter Review | |
923 bytes,
patch
|
Details | Diff | Splinter Review | |
969 bytes,
patch
|
Details | Diff | Splinter Review | |
1.27 KB,
patch
|
Details | Diff | Splinter Review | |
1.67 KB,
patch
|
Details | Diff | Splinter Review | |
981 bytes,
patch
|
Details | Diff | Splinter Review |
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.
Attachment #8774678 -
Flags: review?(dao+bmo)
Attachment #8774679 -
Flags: review?(dao+bmo)
Attachment #8774680 -
Flags: review?(dao+bmo)
Attachment #8774681 -
Flags: review?(dao+bmo)
Attachment #8774683 -
Flags: review?(dao+bmo)
Attachment #8774685 -
Flags: review?(dao+bmo)
Comment 7•8 years ago
|
||
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-
Attachment #8774686 -
Flags: review?(dao+bmo)
Comment 9•8 years ago
|
||
Comment on attachment 8774679 [details] [diff] [review] Patch - browser-addons.js same here
Attachment #8774679 -
Flags: review?(dao+bmo) → review-
Reporter | ||
Comment 10•8 years ago
|
||
Attachment #8774687 -
Flags: review?(dao+bmo)
Reporter | ||
Comment 11•8 years ago
|
||
Attachment #8774688 -
Flags: review?(dao+bmo)
Comment 12•8 years ago
|
||
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•8 years 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•8 years 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)
Comment 15•8 years ago
|
||
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•8 years 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?
Comment 17•8 years ago
|
||
(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•8 years 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•8 years ago
|
||
I suppose we could move _tabForBrowser.set to addTab and set it with the linkedBrowser proxy?
Reporter | ||
Comment 20•8 years 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•8 years 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?
Comment 22•8 years ago
|
||
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•8 years 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?
Comment 24•8 years ago
|
||
My suggestion would be isProxy.
Updated•8 years ago
|
Attachment #8774681 -
Flags: review?(dao+bmo)
Updated•8 years ago
|
Attachment #8774683 -
Flags: review?(dao+bmo)
Updated•8 years ago
|
Attachment #8774688 -
Flags: review?(dao+bmo)
Updated•8 years ago
|
Attachment #8774685 -
Flags: review?(dao+bmo)
Updated•8 years ago
|
Attachment #8774686 -
Flags: review?(dao+bmo)
Updated•8 years ago
|
Attachment #8774687 -
Flags: review?(dao+bmo)
Reporter | ||
Comment 25•7 years 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
Closed: 7 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•