[ios] Snapshot for WebState

ios/web/web_state/web_state_impl.mm

Index: ios/web/web_state/web_state_impl.mm
diff --git a/ios/web/web_state/web_state_impl.mm b/ios/web/web_state/web_state_impl.mm
index de5e4b97b79629f782af4f2a69367a7974b32070..fa10ad12af83d7311598d05ff60cb1b823289bf8 100644
--- a/ios/web/web_state/web_state_impl.mm
+++ b/ios/web/web_state/web_state_impl.mm
@@ -37,6 +37,7 @@
 #include "ios/web/webui/web_ui_ios_controller_factory_registry.h"
 #include "ios/web/webui/web_ui_ios_impl.h"
 #include "net/http/http_response_headers.h"
+#include "ui/gfx/image/image.h"
 
 namespace web {
 
@@ -670,6 +671,20 @@ bool WebStateImpl::HasOpener() const {
   return created_with_opener_;
 }
 
+void WebStateImpl::TakeSnapshot(
+    const base::Callback<void(const gfx::Image& snapshot)>& callback) const {

[marq (ping after 24h), 2017/06/21 11:13:23]

The CL description describes this method as asynchronous, but this
implementation is entirely synchronous -- a caller will have |callback| execute
before |TakeSnapshot()| returns. 

[rohitrao (ping after 24h), 2017/06/21 11:38:15]

Should we post callback to the current TaskRunner, to help avoid writing code
that accidentally requires a synchronous callback?

[edchin, 2017/06/21 16:57:24]

I'll implement Rohit's suggestion for now. The ios 11 snapshotting method is
actually asynchronous,
and this synchronous code will be dropped in the future with ios 10. 

[Eugene But (OOO till 7-30), 2017/06/21 22:11:29]

I believe the API should be either always synchronous or asynchronous. Having
different API contact depending on iOS version, is quite inconvenient for the
clients.

[edchin, 2017/06/22 07:01:07]

Made pre-ios-11 also asynchronous using SequencedTaskRunner.

+  UIView* view = [web_controller_ view];
+  if (!view || CGRectIsEmpty(view.bounds)) {
+    callback.Run(gfx::Image());
+    return;
+  }
+  UIGraphicsBeginImageContextWithOptions(view.bounds.size, YES, 0);

[marq (ping after 24h), 2017/06/21 11:13:23]

Is this preferable to UIView's -snapshotViewAfterScreenUpdates: method?

[rohitrao (ping after 24h), 2017/06/21 11:38:15]

I don't think that call actually works consistently enough =)

[edchin, 2017/06/21 16:57:24]

Ditto.

+  [view drawViewHierarchyInRect:view.bounds afterScreenUpdates:NO];
+  UIImage* snapshot = UIGraphicsGetImageFromCurrentImageContext();
+  UIGraphicsEndImageContext();
+  callback.Run(gfx::Image(snapshot));

[marq (ping after 24h), 2017/06/21 11:13:23]

What's the cost of converting to/from a gfx::Image? What benefit do we get from
this?

[rohitrao (ping after 24h), 2017/06/21 11:38:15]

gfx::Images are pretty cheap, as long as you don't do anything that requires
converting the underlying image to a new format.

I'm not sure what the arguments for using it here are, other than that we don't
have many raw UIKit types in WebState methods.

[edchin, 2017/06/21 16:57:24]

There was some discussion on this on the design doc. One reason is that web
layer uses Chrome/C++ primitives where possible (GURL instead of NSURL,
std::string instead of NSString, Callback instead of blocks). 

[marq (ping after 24h), 2017/06/22 11:55:31]

I agree that this approach is consistent with that; whether that's how web
should work is a separate conversation, then.

+}
+
 void WebStateImpl::OnNavigationStarted(web::NavigationContext* context) {
   for (auto& observer : observers_)
     observer.DidStartNavigation(context);