Summary: | Web Automation: Add Automation.takeScreenshot | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||
Component: | Web Inspector | Assignee: | Joseph Pecoraro <joepeck> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bburg, graouts, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Joseph Pecoraro
2016-03-31 11:16:25 PDT
Created attachment 275307 [details]
[PATCH] Proposed Fix
Comment on attachment 275307 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=275307&action=review > Source/WebKit2/UIProcess/Automation/Automation.json:313 > + "name": "screenshot", I think takeScreenshot would be better. See below. > Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp:777 > +void WebAutomationSession::screenshot(ErrorString& errorString, const String& handle, Ref<ScreenshotCallback>&& callback) WebAutomationSession::takeScreenshot would be good, we have been naming these the same as the WebAutomationSessionPoxy methods. > Source/WebKit2/WebProcess/Automation/WebAutomationSessionProxy.cpp:476 > + RefPtr<WebImage> image = page->scaledSnapshotWithOptions(snapshotRect, 1, SnapshotOptionsShareable); This forces 1x? I wonder if we should make it work with 2x too. (In reply to comment #3) > Comment on attachment 275307 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=275307&action=review > > > Source/WebKit2/UIProcess/Automation/Automation.json:313 > > + "name": "screenshot", > > I think takeScreenshot would be better. See below. > > > Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp:777 > > +void WebAutomationSession::screenshot(ErrorString& errorString, const String& handle, Ref<ScreenshotCallback>&& callback) > > WebAutomationSession::takeScreenshot would be good, we have been naming > these the same as the WebAutomationSessionPoxy methods. I'll rename. > > Source/WebKit2/WebProcess/Automation/WebAutomationSessionProxy.cpp:476 > > + RefPtr<WebImage> image = page->scaledSnapshotWithOptions(snapshotRect, 1, SnapshotOptionsShareable); > > This forces 1x? I wonder if we should make it work with 2x too. Nope, this is an "additional scale". it is fine to be 1. Comment on attachment 275307 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=275307&action=review r=me! > Source/WebKit2/ChangeLog:35 > + Use null string where possible for efficiency. Efficiency! :) > Source/WebKit2/UIProcess/Automation/WebAutomationSession.h:109 > + void screenshot(Inspector::ErrorString&, const String& handle, Ref<Inspector::AutomationBackendDispatcherHandler::ScreenshotCallback>&&) override; I would name it takeScreenshot to match the callback. > Source/WebKit2/WebProcess/Automation/WebAutomationSessionProxy.cpp:455 > + ShareableBitmap::Handle handle; Nit: move this closer to the use-site. > > Source/WebKit2/WebProcess/Automation/WebAutomationSessionProxy.cpp:455
> > + ShareableBitmap::Handle handle;
>
> Nit: move this closer to the use-site.
This is used, even when empty, in all the DidTakeScreenshot responses, so I put it at the top.
|