Let Panorama use the thumbnail service

RESOLVED FIXED in Firefox 14


Firefox Graveyard
5 years ago
a year ago


(Reporter: ttaubert, Assigned: ttaubert)


Firefox 14



(2 attachments, 2 obsolete attachments)

Panorama should use the thumbnail service to display thumbnails for not-yet-restored tabs. We can rip out the whole thumbnail storage and other code parts with this.

Also, this will fix bug 738128.
Created attachment 614948 [details] [diff] [review]
Part 1 - Make Panorama use the thumbnail service
Created attachment 614949 [details] [diff] [review]
Part 2 - Correct Panorama tests
Created attachment 614953 [details] [diff] [review]
Part 1 - Make Panorama use the thumbnail service
Attachment #614948 - Attachment is obsolete: true
Comment on attachment 614949 [details] [diff] [review]
Part 2 - Correct Panorama tests

Explaining some changes:

>--- a/browser/components/tabview/test/Makefile.in
>                  browser_tabview_bug596781.js \
>-                 browser_tabview_bug597248.js \
>                  browser_tabview_bug597360.js \

Removing the test for bug 597248. No need to make sure Panorama's thumbnail cache is solid anymore.

>                  browser_tabview_bug626791.js \
>-                 browser_tabview_bug627288.js \
>                  browser_tabview_bug627736.js \

Removing test for bug 627288 because TabItem.showCachedData() retrieves its data now from TabItem.getTabState() which is never stale. Title and URL (and therefore the thumbnail) are always up-to-date.

>                  browser_tabview_bug673729.js \
>-                 browser_tabview_bug677310.js \
>                  browser_tabview_bug678374.js \

Removing test for bug 677310 because we can't control whether all thumbnails have been captured before transitioning into private browsing mode. This is okay since we shouldn't execute any sync operations and most thumbnails should have been stored anyway before entering PB mode.
Attachment #614949 - Flags: review?(dietrich)
Created attachment 614958 [details] [diff] [review]
Part 1 - Make Panorama use the thumbnail service

And again explaining some changes:

>--- a/browser/components/tabview/content.js

There's a lot of stuff we can remove from the Panorama content script. The majority of the code was there to figure out the storage policy for thumbnails.

>diff --git a/browser/components/tabview/storagePolicy.js b/browser/components/tabview/storagePolicy.js
>diff --git a/browser/components/tabview/thumbnailStorage.js b/browser/components/tabview/thumbnailStorage.js

We don't the thumbnail storage and its storage policy anymore.

>--- a/browser/components/tabview/tabitems.js
>   _reconnect: function TabItem__reconnect(options) {
>     Utils.assertThrow(!this._reconnected, "shouldn't already be reconnected");
>     Utils.assertThrow(this.tab, "should have a xul:tab");
>     let tabData = Storage.getTabData(this.tab);
>     let groupItem;
>     if (tabData && TabItems.storageSanity(tabData)) {
>-      this.loadThumbnail();
>+      // Show the cached data while we're waiting for the tabItem to be updated.
>+      // If the tab isn't restored yet this acts as a placeholder until it is.
>+      this.showCachedData();

We never show stale data, we just create a placeholder while the tab is loading/restoring or waiting to be restored. So we can just unconditionally call showCachedData() because this gets removed when the TabItem is updated.
Attachment #614953 - Attachment is obsolete: true
Attachment #614958 - Flags: review?(dietrich)
Comment on attachment 614958 [details] [diff] [review]
Part 1 - Make Panorama use the thumbnail service

Review of attachment 614958 [details] [diff] [review]:

yay code removal. boo on making the tooltip changes on an unrelated patch :P but they look ok, so r=me.
Attachment #614958 - Flags: review?(dietrich) → review+
Attachment #614949 - Flags: review?(dietrich) → review+
(In reply to Dietrich Ayala (:dietrich) from comment #6)
> yay code removal. boo on making the tooltip changes on an unrelated patch :P
> but they look ok, so r=me.

Sorry, I couldn't resist :)
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 14
Forgot to remove the now empty TabItem.loadThumbnail() method:

Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.