Functionality has been requested in the Extension API for Javascript to...

I have taken over this patch, original authored by Adam Hunter (adamhunter).

The original issue is: http://codereview.chromium.org/144019

--------------

Functionality has been requested in the Extension API for Javascript to
take screenshots of the currently visible tab.  This changelist builds this
function, chrome.tabs.getVisibleScreenCapture.  This function takes a
single callback function and returns to that function a data URL of a JPEG
image of the current screen.  A simple sample extension is provided as a 
use case.

BUG=14760
TEST=There is an extension in chrome\test\data\extensions\samples\screenshot,
load this extension.  It creates a toolstrip button.  Click this button, you
should get a page with a screenshot of the active tab.  The API function
is found at chrome.tabs.getVisibleScreenCapture.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=23259

[rafaelw, 2009-07-28 01:05:26]

I've taken over this patch from Adam. I've reviewed the comment history of the
original patch, and I believe the current state reflects all of the changes
requested except for darin's -- which I think it makes sense to tackle in a
subsequent change.

The original issue lacked lgtms from brettw, darin & aa.

Please review the current state.

[omattos, 2009-07-28 13:06:33]

Have you considered instead of returning a Data: URL, you could return an
ImageData object.  This means there is less duplicated functionality and the
implementation should be simpler, and would me considerably more efficient in
the case where the javascript wants to manipulate the image (eg. resize, crop,
or watermark).

[rafaelw, 2009-07-30 17:45:20]

I'd like to know whether Aaron, Darin & Brett are LGTM on the this patch, or if
it needs to go back to the drawing board for a different approach.

[Aaron Boodman, 2009-07-30 18:18:52]

lgtm, w/ nits

http://codereview.chromium.org/160228/diff/1024/14
File chrome/browser/extensions/extension_tabs_module.cc (right):

http://codereview.chromium.org/160228/diff/1024/14#newcode671
Line 671: NOTIMPLEMENTED();
I think we should return an error to the caller in the case of not being
implemented on other platforms.

http://codereview.chromium.org/160228/diff/1024/14#newcode671
Line 671: NOTIMPLEMENTED();
What is the timeline for implementing this on other platforms? Do we know how to
do it? I don't want to have windows-only APIs.

http://codereview.chromium.org/160228/diff/1024/18
File chrome/common/extensions/api/extension_api.json (right):

http://codereview.chromium.org/160228/diff/1024/18#newcode332
Line 332: name: "getVisibleTabCapture",
Can we change this to captureVisibleTab()?

[darin (slow to review), 2009-07-30 19:14:51]

I defer to Aaron.-Darin


On Thu, Jul 30, 2009 at 10:45 AM, <rafaelw@chromium.org> wrote:

> I'd like to know whether Aaron, Darin & Brett are LGTM on the this
> patch, or if it needs to go back to the drawing board for a different
> approach.
>
>
>
> http://codereview.chromium.org/160228
>

[rafaelw, 2009-08-13 00:35:58]

I had to allow the extension_tabs_module to call GetBackingStore with
force=false. Force=true was causing a crash if the called before the page was
properly setup.

This means that the getVisibleTabCapture() can error if called to early when a
tab opens.

http://codereview.chromium.org/160228/diff/1024/14
File chrome/browser/extensions/extension_tabs_module.cc (right):

http://codereview.chromium.org/160228/diff/1024/14#newcode671
Line 671: NOTIMPLEMENTED();
On 2009/07/30 18:18:53, Aaron Boodman wrote:
> I think we should return an error to the caller in the case of not being
> implemented on other platforms.

Done.

http://codereview.chromium.org/160228/diff/1024/14#newcode671
Line 671: NOTIMPLEMENTED();
On 2009/07/30 18:18:53, Aaron Boodman wrote:
> What is the timeline for implementing this on other platforms? Do we know how
to
> do it? I don't want to have windows-only APIs.

The plan is to implement on linux & mac when time allows. (Hopefully the
platform-specific changes will be small).

http://codereview.chromium.org/160228/diff/1024/18
File chrome/common/extensions/api/extension_api.json (right):

http://codereview.chromium.org/160228/diff/1024/18#newcode332
Line 332: name: "getVisibleTabCapture",
On 2009/07/30 18:18:53, Aaron Boodman wrote:
> Can we change this to captureVisibleTab()?

Done.