616755 - make Tab.getThumbnailURL() be Tab.getThumbnail() and update docs from Tab.thumbnail days

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

The Tab.thumbnail property doesn't provide the flexibility that addon authors want from such an interface, as it doesn't allow them to specify the size of the thumbnail generated.

And it's particularly problematic from an e10s perspective, since it requires an asynchronous call across the process boundary (unless we intend for the addon process to periodically poll the content process for a new thumbnail, which seems like a bad idea).

So I think we should pull it and later implement a getThumbnail(options, callback) method that asynchronously retrieves and returns a thumbnail conforming to options specified by the method call.

Comment 1

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to pull request

Comment 2

[Dietrich Ayala (:dietrich)]

Can we not remove this API? Instead can we implement the suggested solution, with no supported options, and have the default be what we return now?

Comment 3

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to pull request

Comment 4

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 495492 [details]
Pointer to pull request

After discussion with Dietrich I've implemented another option as well.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

Actually, it turns out that I was mistaken, as this call across the process boundary can block, since the work being done in the chrome/content process is synchronous.

So the only issue is that the docs are incorrect (they refer to a thumbnail property).  But let's also make the getThumbnailURL -> getThumbnail change in the process.

Comment 6

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to pull request

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 495988 [details]
Pointer to pull request

Looks good! r=myk

Comment 8

[Myk Melez [:myk] [@mykmelez]]

Irakli landed this in merge commit:

https://github.com/mozilla/addon-sdk/commit/d78a463da3bbd5f25eefc4e536e54b2ebd0ae582

Change commit:

https://github.com/mozilla/addon-sdk/commit/c47ed8d7c6b65f7e09f0104355b1bf7ee5e20126

(plus a couple other merges)

I cherry-picked it in commit:

https://github.com/mozilla/addon-sdk/commit/a2a1a62c07f5dc4890fec95006c8e31353f280bf