Closed
Bug 580954
Opened 14 years ago
Closed 14 years ago
Replace the heartbeat with something smarter
Categories
(Firefox Graveyard :: Panorama, defect, P3)
Firefox Graveyard
Panorama
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: Mardak, Assigned: iangilman)
References
Details
(Whiteboard: [qa-])
In tracking down BrowserTab usage in bug 580937, I noticed .url and .raw were constantly getting called and turns out to be the heartbeat. It is scheduled to run every 100 milliseconds.
This is probably contributing a good portion to the Ts impact.
I'm guessing this is used to make sure the mirrors are kept up-to-date?
Perhaps this could leverage the AllTabs.onChange I'm adding to the tabs module in bug 580952.
Assignee | ||
Comment 1•14 years ago
|
||
Yes, the heartbeat was written to work around the fact that we weren't getting reliable information about when the tabs were changing. If we can get reliable events, we should definitely use them instead.
That said, the other function that the heartbeat performs is to throttle thumbnail updating; if you've got a lot of tabs with dynamic content, you don't want them updating constantly or all at the same time.
So I think the ideal system would be triggered by AllTabs.onChange, but would then be throttled as needed.
Comment 2•14 years ago
|
||
Hmm, Firefox (on Windows only?) has some code to grab tab screenshots in canvas elements. Maybe TabCandy can share the same code with the rest of Firefox?
Comment 3•14 years ago
|
||
This is the code that I'm talking about:
<http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-tabPreviews.js>
Comment 4•14 years ago
|
||
The AeroPeek code obtains full-size canvas representations and incrementally updates them. Relevant bits of the code start here:
http://mxr.mozilla.org/mozilla-central/source/browser/components/wintaskbar/WindowsPreviewPerTab.jsm#197
Assignee | ||
Comment 5•14 years ago
|
||
Cool! I wonder if either of these can be generalized for all platforms?
By the way, we've got another bug, bug 581038, for discussing alternatives to how we currently thumbnail. This bug is more about how/when we trigger the updates of the thumbnails as well as tab titles, favIcons, etc.
Comment 6•14 years ago
|
||
(In reply to comment #5)
> Cool! I wonder if either of these can be generalized for all platforms?
>
> By the way, we've got another bug, bug 581038, for discussing alternatives to
> how we currently thumbnail. This bug is more about how/when we trigger the
> updates of the thumbnails as well as tab titles, favIcons, etc.
The code I linked does all this in a way that is, I believe, cross platform. It doesn't handle zooming, but there's a patch posted in bug 520943 that does.
Assignee | ||
Comment 7•14 years ago
|
||
Rob, thanks!
Mardak, I'll take a whack at refactoring heartbeat once AllTabs has landed.
Assignee | ||
Comment 8•14 years ago
|
||
The first part of this is in now:
http://hg.mozilla.org/users/edward.lee_engineering.uiuc.edu/tabcandy-central/rev/b08bcefd2895
It still needs to be smarter, though:
* We're hiding the cached thumbnails too soon
* We shouldn't update at all when the Tab Candy UI isn't visible (except for the current tab, to facilitate the zoom down)
* If we get a bunch of updates all at the same time, spread them out so we don't get a big stutter
Assignee | ||
Comment 9•14 years ago
|
||
With this change:
http://hg.mozilla.org/users/edward.lee_engineering.uiuc.edu/tabcandy-central/rev/d6d4d80a0dea
... we're now not updating when the UI isn't visible, and we're spreading things out when it is. Remaining:
* Hiding cached thumbnails too soon
* An exception for the current tab
Updated•14 years ago
|
Assignee: nobody → ian
Assignee | ||
Comment 10•14 years ago
|
||
With this change:
http://hg.mozilla.org/users/edward.lee_engineering.uiuc.edu/tabcandy-central/rev/674fb15553b8
... we're now updating the thumbnail for the current tab even if the UI isn't visible (so the zoom down works). Remaining:
* Hiding cached thumbnails too soon
Also, we might want to do some tests to see whether triggering repainting the thumbnail on every onChange is too much.
Comment 11•14 years ago
|
||
Mass moving all Tab Candy bugs from Mozilla Labs to Firefox::Tab Candy. Filter the bugmail spam with "tabcandymassmove".
Product: Mozilla Labs → Firefox
Target Milestone: -- → ---
Updated•14 years ago
|
QA Contact: tabcandy → tabcandy
Comment 12•14 years ago
|
||
"Something smarter" may include using -moz-element... just a thought.
Priority: -- → P3
Comment 13•14 years ago
|
||
Maybe I'm overlooking something, but wouldn't listening for load and MozAfterPaint events help? Let both events set a dirty state. When the heartbeat-controlled drawing part comes across a thumbnail that's dirty it will redraw, otherwise the old one is kept.
Comment 14•14 years ago
|
||
(In reply to comment #13)
> Maybe I'm overlooking something, but wouldn't listening for load and
> MozAfterPaint events help? Let both events set a dirty state. When the
> heartbeat-controlled drawing part comes across a thumbnail that's dirty it will
> redraw, otherwise the old one is kept.
That was actually what we had earlier, but we moved away from it, I believe for performance reasons. I'll let Ian chime in.
Assignee | ||
Comment 15•14 years ago
|
||
(In reply to comment #13)
> Maybe I'm overlooking something, but wouldn't listening for load and
> MozAfterPaint events help? Let both events set a dirty state. When the
> heartbeat-controlled drawing part comes across a thumbnail that's dirty it will
> redraw, otherwise the old one is kept.
Early on we had reliability issues with MozAfterPaint, but it's probably worth revisiting it (along with load) for use instead of TabAttrModified. I've now filed bug 598435 to look into this.
I'm going to close this bug, as all of the remaining items now have their own bugs:
Bug 597258 - Explore using -moz-element instead of the periodic canvas painting for Panorama thumbnails
Bug 581038 - Should we be using an existing thumbnailing system from elsewhere in Firefox?
Bug 597248 - Make sure Panorama's thumbnail cache is solid
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Whiteboard: [qa-]
Updated•9 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•