WK2 already has the infrastructure for this, but WKGTK+ API is missing. I am working on it. Meanwhile, I need the WK1 API for Ephy 3.6, since we will use wk1.
Created attachment 154377 [details] 2012-07-25 Claudio Saavedra <csaavedra@igalia.com> [Gtk] Add WK1 API for snapshot retrieval https://bugs.webkit.org/show_bug.cgi?id=92261 Reviewed by NOBODY (OOPS!). Added a method to ChromeClientGtk that paints the webview contents into a cairo_surface and add the public API to WebKitWebView for this. * WebCoreSupport/ChromeClientGtk.cpp: (WebKit::ChromeClient::paintWebViewInSurface): Paint the ChromeClient in a given cairo_surface. * WebCoreSupport/ChromeClientGtk.h: (ChromeClient): Ditto. * webkit/webkitwebview.cpp: (webkit_web_view_get_snapshot): New method to paint a webview snapshot. * webkit/webkitwebview.h: Ditto.
Comment on attachment 154377 [details] 2012-07-25 Claudio Saavedra <csaavedra@igalia.com> View in context: https://bugs.webkit.org/attachment.cgi?id=154377&action=review Thanks for the patch, looks good in general, I have just a few comments. > Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:531 > + Frame* frame = core(m_webView)->mainFrame(); I think frame can be null here, check it and return early or add an assert if you are sure it can't be null. > Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:532 > + RefPtr<cairo_t> cr = cairo_create(surface); You should adopt the ref and then you can remove the cairo_destroy. RefPtr<cairo_t> cr = adoptRef(cairo_create(surface)); > Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.h:167 > + void paintWebViewInSurface(cairo_surface_t*, IntRect&); Why do you need this method in the chrome client? It simply uses the frame, so I guess you can use it directly in the web view. > Source/WebKit/gtk/webkit/webkitwebview.cpp:5263 > +cairo_surface_t* > +webkit_web_view_get_snapshot(WebKitWebView *webView) Please add gtk-doc comments. Remember to add Since: 1.10 tag and add the new symbol to the sections file. > Source/WebKit/gtk/webkit/webkitwebview.cpp:5267 > + GtkAllocation allocation; > + IntRect rect; > + cairo_surface_t* surface; Declare these variables when you need them > Source/WebKit/gtk/webkit/webkitwebview.cpp:5274 > + surface = cairo_image_surface_create(CAIRO_FORMAT_RGB24, > + allocation.width, > + allocation.height); This should be one line. > Source/WebKit/gtk/webkit/webkitwebview.cpp:5275 > + rect = IntRect(allocation.x, allocation.y, allocation.width, allocation.height); I think you can do IntRect rect = allocation; or even directly call chromeClient->paintWebViewInSurface(surface, IntRect(allocation));
Created attachment 154935 [details] [Gtk] Add WK1 API for snapshot retrieval https://bugs.webkit.org/show_bug.cgi?id=92261 Reviewed by NOBODY (OOPS!). Add API to WebKitWebView to retrieve a snapshot of its visible contents as a cairo_surface_t. * webkit/webkitwebview.cpp: (webkit_web_view_get_snapshot): New method to paint a webview snapshot. * webkit/webkitwebview.h: Ditto.
Comment on attachment 154935 [details] [Gtk] Add WK1 API for snapshot retrieval https://bugs.webkit.org/show_bug.cgi?id=92261 Attachment 154935 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/13373391
Created attachment 154938 [details] [Gtk] Add WK1 API for snapshot retrieval https://bugs.webkit.org/show_bug.cgi?id=92261 Reviewed by NOBODY (OOPS!). Add API to WebKitWebView to retrieve a snapshot of its visible contents as a cairo_surface_t. * docs/webkitgtk-sections.txt: Add new symbols. * webkit/webkitwebview.cpp: (webkit_web_view_get_snapshot): New method to paint a webview snapshot. * webkit/webkitwebview.h: Ditto.
Comment on attachment 154938 [details] [Gtk] Add WK1 API for snapshot retrieval View in context: https://bugs.webkit.org/attachment.cgi?id=154938&action=review API looks good to me, I just wonder whether it would be useful to pass the clip rect. In any case we need the approval from another reviewer for this. > Source/WebKit/gtk/webkit/webkitwebview.cpp:5264 > + * webkit_web_view_get_snapshot: > + * @web_view: a #WebKitWebView I wonder if we could pass a rectangle to the method, or even having two methods and this one could call the one taking a rectangle with the view allocation. > Source/WebKit/gtk/webkit/webkitwebview.cpp:5270 > + * Since: 1.9.5 Use stable version numbers in Since tags, and 1.9.5 was already released :-) Use 1.10 in this case, please. > Source/WebKit/gtk/webkit/webkitwebview.cpp:5275 > + GtkAllocation allocation; Move this right before gtk_widget_get_allocation() > Source/WebKit/gtk/webkit/webkitwebview.cpp:5277 > + g_return_val_if_fail(WEBKIT_IS_WEB_VIEW(webView), NULL); Use 0 instead of NULL. > Source/WebKit/gtk/webkit/webkitwebview.cpp:5281 > + return NULL; Ditto.
Comment on attachment 154938 [details] [Gtk] Add WK1 API for snapshot retrieval View in context: https://bugs.webkit.org/attachment.cgi?id=154938&action=review Looks good to me apart from two minor changes! > Source/WebKit/gtk/webkit/webkitwebview.cpp:5273 > +webkit_web_view_get_snapshot(WebKitWebView *webView) The asterisk should be up against the type here. > Source/WebKit/gtk/webkit/webkitwebview.cpp:5284 > + cairo_surface_t* surface = cairo_image_surface_create(CAIRO_FORMAT_RGB24, allocation.width, allocation.height); Changing this to CAIRO_FORMAT_ARGB32 and adding a clearRect (see the call in ChromeClient) below would ensure that transparent backgrounds work.
Comment on attachment 154938 [details] [Gtk] Add WK1 API for snapshot retrieval Oops. Looks like this is missing a frame->view()->updateLayoutAndStyleIfNeededRecursive().
Created attachment 155128 [details] [Gtk] Add WK1 API for snapshot retrieval https://bugs.webkit.org/show_bug.cgi?id=92261 Reviewed by NOBODY (OOPS!). Add API to WebKitWebView to retrieve a snapshot of its visible contents as a cairo_surface_t. * docs/webkitgtk-sections.txt: Add new symbols. * webkit/webkitwebview.cpp: (webkit_web_view_get_snapshot): New method to paint a webview snapshot. * webkit/webkitwebview.h: Ditto.
Comment on attachment 155128 [details] [Gtk] Add WK1 API for snapshot retrieval Why not use gtk_widget_draw()? Is it because of the need to apply the device scale factor?
Comment on attachment 154938 [details] [Gtk] Add WK1 API for snapshot retrieval Cleared Martin Robinson's review+ from obsolete attachment 154938 [details] so that this bug does not appear in http://webkit.org/pending-commit.
(In reply to comment #10) > (From update of attachment 155128 [details]) > Why not use gtk_widget_draw()? Is it because of the need to apply the device scale factor? Because it doesn't always work (the widget needs to be mapped and visible, and this won't always hold true).
Comment on attachment 155128 [details] [Gtk] Add WK1 API for snapshot retrieval Thanks!
Comment on attachment 155128 [details] [Gtk] Add WK1 API for snapshot retrieval Rejecting attachment 155128 [details] from commit-queue. csaavedra@igalia.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
Comment on attachment 155128 [details] [Gtk] Add WK1 API for snapshot retrieval Clearing flags on attachment: 155128 Committed r124023: <http://trac.webkit.org/changeset/124023>
All reviewed patches have been landed. Closing bug.
> Because it doesn't always work (the widget needs to be mapped and visible, and this won't always hold true). Aha, thanks!