Implement DocuementHasImages

android_webview/browser/renderer_host/aw_render_view_host_ext.cc

Index: android_webview/browser/renderer_host/aw_render_view_host_ext.cc
diff --git a/android_webview/browser/renderer_host/aw_render_view_host_ext.cc b/android_webview/browser/renderer_host/aw_render_view_host_ext.cc
new file mode 100644
index 0000000000000000000000000000000000000000..1051427d697fea5bb63b10d53eba2ef2bc53e26f
--- /dev/null
+++ b/android_webview/browser/renderer_host/aw_render_view_host_ext.cc
@@ -0,0 +1,65 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "android_webview/browser/renderer_host/aw_render_view_host_ext.h"
+
+#include "android_webview/common/render_view_messages.h"
+#include "base/android/scoped_java_ref.h"
+#include "base/callback.h"
+#include "content/public/browser/user_metrics.h"
+#include "content/public/browser/web_contents.h"
+
+namespace android_webview {
+
+AwRenderViewHostExt::AwRenderViewHostExt(content::WebContents* contents)
+    : content::WebContentsObserver(contents) {
+}
+
+AwRenderViewHostExt::~AwRenderViewHostExt() {}
+
+void AwRenderViewHostExt::DocumentHasImages(DocumentHasImagesResult result) {
+  if (!web_contents()->GetRenderViewHost()) {
+    result.Run(false);
+    return;
+  }
+  static int next_id = 1;

[mkosiba (inactive), 2012/08/30 09:43:13]

ugh... please make the function explicitly non-threadsafe with a DCHECK (more to
document the behavior than anything). 

[joth, 2012/08/30 17:26:53]

Yeah the hidden static int isn't cutest thing. (I blame the code I was copying
http://codereview.chromium.org/10553014/ :-) but it is the prevailing pattern
for this I believe.


of course the map<> is also totally non-thread safe too. So yes, added
NonThreadSafe base class and sprinkled some DCHECKs around.

+  int this_id = next_id++;
+  pending_document_has_images_requests_[this_id] = result;
+  Send(new AwViewMsg_DocumentHasImages(web_contents()->GetRoutingID(),
+                                       this_id));
+}
+
+void AwRenderViewHostExt::RenderViewGone(base::TerminationStatus status) {
+  for (std::map<int, DocumentHasImagesResult>::iterator pending_req =
+           pending_document_has_images_requests_.begin();
+       pending_req != pending_document_has_images_requests_.end();
+      ++pending_req) {
+    pending_req->second.Run(false);
+  }
+}
+
+bool AwRenderViewHostExt::OnMessageReceived(const IPC::Message& message) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(AwRenderViewHostExt, message)
+    IPC_MESSAGE_HANDLER(AwViewHostMsg_DocumentHasImagesResponse,
+                        OnDocumentHasImagesResponse)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+
+  if (!handled) handled = WebContentsObserver::OnMessageReceived(message);

[mkosiba (inactive), 2012/08/30 09:43:13]

uber-nit; maybe a newline after ) ?

[joth, 2012/08/30 17:26:53]

You're right. 
Actually I can tertiary it..
  return handled ? true : WebContentsObserver::OnMessageReceived(message);

+
+  return handled;
+}
+
+void AwRenderViewHostExt::OnDocumentHasImagesResponse(int msg_id,
+                                                      bool has_images) {
+  std::map<int, DocumentHasImagesResult>::iterator pending_req =
+      pending_document_has_images_requests_.find(msg_id);
+  if (pending_req != pending_document_has_images_requests_.end()) {
+    pending_req->second.Run(has_images);
+    pending_document_has_images_requests_.erase(pending_req);
+  }

[mkosiba (inactive), 2012/08/30 09:43:13]

I'd say we should DCHECK that a match is always found, unless you've found a
valid scenario where this is not the case?

[joth, 2012/08/30 17:26:53]

A good question.
in general chrome code treats anything coming from the render process as slighty
suspect, and by far the most common pattern is to just disregard unexpected
message (Postel's law). 

Adding DCHECK sounds a good balance, but quickly starts to go right against
http://www.chromium.org/developers/coding-style#TOC-CHECK-vs-DCHECK-and-NOTRE...
(i.e. it's creating making a non-testable code path in debug builds)

So that suggests correctly handling it with a call to
web_content()->GetRenderProcessHost()->ReceivedBadMessage() -- but it seems
*extremely* rare for anyone to call that. (and, it's just a round about way to
commit suicide in single process mode).


DLOG(WARNING) ?

+}
+
+}  // namespace android_webview