The default bug view has changed. See this FAQ.

[Page Thumbnails] Capture thumbnails only for the selected tab

RESOLVED FIXED in Firefox 13

Status

()

Firefox
Tabbed Browser
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ttaubert, Assigned: ttaubert)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 14
Points:
---

Firefox Tracking Flags

(firefox13 verified)

Details

(Whiteboard: [Snappy:P1][qa+:virgil])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
To minimize drawWindow's performance impact for thumbnails being captured in the background we should do it only for selected tabs.
(Assignee)

Comment 1

5 years ago
Created attachment 613833 [details] [diff] [review]
patch v1

This patch captures thumbnails only if the given browser is the currently selected browser. Additionally, I decreased the capture timeout from 2s to 1s to not wait too long before taking a screenshot. This should increase chances when quickly navigating, switching tabs and scrolling.
Attachment #613833 - Flags: review?(dietrich)
Comment on attachment 613833 [details] [diff] [review]
patch v1

>   _shouldCapture: function Thumbnails_shouldCapture(aBrowser) {
>     let doc = aBrowser.contentDocument;
> 
>+    // Capture only if it's the currently selected tab.
>+    if (aBrowser != gBrowser.selectedBrowser)
>+      return false;

This should be above let doc = ...;
(Assignee)

Comment 3

5 years ago
(In reply to Dão Gottwald [:dao] from comment #2)
> This should be above let doc = ...;

Um, yeah of course. Will fix, thanks.
(Assignee)

Comment 4

5 years ago
Created attachment 613914 [details] [diff] [review]
patch v2
Attachment #613833 - Attachment is obsolete: true
Attachment #613833 - Flags: review?(dietrich)
Attachment #613914 - Flags: review?(dietrich)

Updated

5 years ago
Whiteboard: [Snappy] → [Snappy:P1]
Attachment #613914 - Flags: review?(dietrich) → review+
(Assignee)

Comment 5

5 years ago
https://hg.mozilla.org/integration/fx-team/rev/fc693ba6ce24
Whiteboard: [Snappy:P1] → [Snappy:P1][fixed-in-fx-team]
Target Milestone: --- → Firefox 14
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/mozilla-central/rev/fc693ba6ce24
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [Snappy:P1][fixed-in-fx-team] → [Snappy:P1]
(Assignee)

Comment 7

5 years ago
Comment on attachment 613914 [details] [diff] [review]
patch v2

[Approval Request Comment]
User impact if declined: thumbnail service might be too much of a performance impact
Risk to taking this patch (and alternatives if risky): very low risk, tiny patch
String changes made by this patch: none
Attachment #613914 - Flags: approval-mozilla-aurora?

Comment 8

5 years ago
I'd like to see us take this as part of the remaining few clean-up bugs before Net Tab gets uplifted to Beta.
Comment on attachment 613914 [details] [diff] [review]
patch v2

[Triage Comment]
Approving for landing in support of Asa's request for Net Tab feature support for upcoming beta.
Attachment #613914 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 10

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/360971f3fcc3
status-firefox13: --- → fixed
This bug caused performance regressions in a11y 2 MozAfterPaint on Linux and tscroll 2 on Mac, and possibly other benchmarks like Sunspider 2 MozAfterPaint.  (The regression cause wasn't identified on inbound/central because of a problem with the graph server, but it is clear in the Aurora graphs.)
Whiteboard: [Snappy:P1] → [Snappy:P1][qa+]
Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20100101 Firefox/13.0

Verified on 13 beta 3 with Ubuntu 12.04, Mac OS 10.6 and Windows 7. Thumbnails are only saved for focused tabs.

Are there any reasons why this shouldn't be set to verified yet due to comment 11?
(In reply to Virgil Dicu [:virgil] [QA] from comment #12)
> Are there any reasons why this shouldn't be set to verified yet due to
> comment 11?

Given this is a Snappy:P1 and the patch has resulted in a performance regression, I'm wondering that too.
Marking this one verified fixed for Firefox 13. As per Virgil's testing in comment 12, the functionality is working as expected. Please comment if there is something further you want QA to test.
status-firefox13: fixed → verified
Whiteboard: [Snappy:P1][qa+] → [Snappy:P1][qa+:virgil]
You need to log in before you can comment on or make changes to this bug.