Add ContentViewDelegate

android_webview/native/aw_contents_io_thread_client.cc

Index: android_webview/native/aw_contents_io_thread_client.cc
diff --git a/android_webview/native/aw_contents_io_thread_client.cc b/android_webview/native/aw_contents_io_thread_client.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d43f478e485bfd582cfcfea50b6b903b047bf310
--- /dev/null
+++ b/android_webview/native/aw_contents_io_thread_client.cc
@@ -0,0 +1,183 @@
+// 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/native/aw_contents_io_thread_client.h"
+
+#include <map>
+#include <utility>
+
+#include "android_webview/native/intercepted_request_data.h"
+#include "base/android/jni_helper.h"
+#include "base/android/jni_string.h"
+#include "base/lazy_instance.h"
+#include "base/memory/linked_ptr.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/synchronization/lock.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_observer.h"
+#include "net/url_request/url_request.h"
+
+#include "jni/AwContentsIoThreadClient_jni.h"
+
+using base::android::AttachCurrentThread;
+using base::android::ConvertUTF8ToJavaString;
+using base::android::JavaRef;
+using base::android::ScopedJavaLocalRef;
+using base::LazyInstance;
+using content::BrowserThread;
+using content::RenderViewHost;
+using content::WebContents;
+using std::map;
+using std::pair;
+
+namespace android_webview {
+
+namespace {
+
+typedef map<pair<int, int>, JavaObjectWeakGlobalRef>
+    RenderViewHostToWeakDelegateMapType;
+
+base::Lock g_map_lock;

[joth, 2012/09/13 20:09:12]

you still have a global in here!

Suggest wrapping the lock and the map into a single struct {} and have that as a
lazy instance. Maybe nice to even put methods that use the lock onto that class,
then you can keep the lock itself private.

[mkosiba (inactive), 2012/09/17 17:48:22]

Done.

+LazyInstance<RenderViewHostToWeakDelegateMapType> g_rvh_to_weak_delegate_map =
+    LAZY_INSTANCE_INITIALIZER;
+
+static pair<int, int> GetRenderViewHostIdPair(RenderViewHost* rvh) {
+  return pair<int, int>(rvh->GetProcess()->GetID(), rvh->GetRoutingID());
+}
+
+// JavaClientMapMaintainer ----------------------------------------------------

[joth, 2012/09/13 20:09:12]

nit: FooMaintainer sounds like something I'd expect there to be one of per Foo
instance. (So here I expect one per Map, i.e. one)

How about ClientMapEntryUpdater (making it clear it's per Entry, not per Map)

[mkosiba (inactive), 2012/09/17 17:48:22]

Done.

+
+class JavaClientMapMaintainer : public content::WebContentsObserver {
+ public:
+  JavaClientMapMaintainer(JNIEnv* env, WebContents* web_contents,
+                          jobject jdelegate);
+
+  virtual void RenderViewCreated(RenderViewHost* render_view_host) OVERRIDE;
+  virtual void RenderViewForInterstitialPageCreated(
+      RenderViewHost* render_view_host) OVERRIDE;
+  virtual void RenderViewDeleted(RenderViewHost* render_view_host) OVERRIDE;
+  virtual void WebContentsDestroyed(WebContents* web_contents);
+
+ private:
+  JavaObjectWeakGlobalRef jdelegate_;
+};
+
+JavaClientMapMaintainer::JavaClientMapMaintainer(
+    JNIEnv* env, WebContents* web_contents, jobject jdelegate)

[joth, 2012/09/13 20:09:12]

one param per line. (and in the class declaration)

[mkosiba (inactive), 2012/09/17 17:48:22]

Done.

+    : content::WebContentsObserver(web_contents),
+      jdelegate_(env, jdelegate) {
+  DCHECK(web_contents);
+  DCHECK(jdelegate);
+
+  if (web_contents->GetRenderViewHost())
+    RenderViewCreated(web_contents->GetRenderViewHost());
+}
+
+void JavaClientMapMaintainer::RenderViewCreated(RenderViewHost* rvh) {
+  pair<int, int> rvh_id = GetRenderViewHostIdPair(rvh);
+  base::AutoLock lock(g_map_lock);
+  g_rvh_to_weak_delegate_map.Get()[rvh_id] = jdelegate_;
+}
+
+void JavaClientMapMaintainer::RenderViewForInterstitialPageCreated(
+    RenderViewHost* rvh) {
+  RenderViewCreated(rvh);
+}
+
+void JavaClientMapMaintainer::RenderViewDeleted(RenderViewHost* rvh) {
+  pair<int, int> rvh_id = GetRenderViewHostIdPair(rvh);
+  base::AutoLock lock(g_map_lock);
+  g_rvh_to_weak_delegate_map.Get().erase(rvh_id);
+}
+
+void JavaClientMapMaintainer::WebContentsDestroyed(
+    WebContents* web_contents) {
+
+  if (web_contents->GetRenderViewHost())
+    RenderViewDeleted(web_contents->GetRenderViewHost());
+
+  delete this;
+}
+
+} // namespace
+
+// AwContentsIoThreadClient ----------------------------------------------
+
+// JNI Methods ----------------------------------------------------------------

[joth, 2012/09/13 20:09:12]

nit: If you're splitting the file by class name I don't think it needs ascii art
for the method types within it too.

(static methods we tend to do "// static" prefix per method)


nit: make the ascii art ---- line up on right margin throughout the file.

[mkosiba (inactive), 2012/09/17 17:48:22]

Done.

+
+bool AwContentsIoThreadClient::RegisterAwContentsIoThreadClient(
+    JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
+// Static methods -------------------------------------------------------------
+
+void AwContentsIoThreadClient::Associate(
+    WebContents* web_contents, const JavaRef<jobject>& jclient) {
+  JNIEnv* env = AttachCurrentThread();
+  // The JavaClientMapMaintainer lifespan is tied to the WebContents.
+  new JavaClientMapMaintainer(env, web_contents, jclient.obj());
+}
+
+AwContentsIoThreadClient
+AwContentsIoThreadClient::FromID(int render_process_id, int render_view_id) {
+  pair<int, int> rvh_id(render_process_id, render_view_id);
+  base::AutoLock lock(g_map_lock);
+  RenderViewHostToWeakDelegateMapType::iterator weak_delegate_iterator =
+      g_rvh_to_weak_delegate_map.Get().find(rvh_id);
+  if (weak_delegate_iterator == g_rvh_to_weak_delegate_map.Get().end())
+    return AwContentsIoThreadClient();
+
+  JNIEnv* env = AttachCurrentThread();
+  ScopedJavaLocalRef<jobject> java_delegate =
+      weak_delegate_iterator->second.get(env);
+  if (java_delegate.is_null())
+    return AwContentsIoThreadClient();
+
+  return AwContentsIoThreadClient(java_delegate);
+}
+
+// regular methods ------------------------------------------------------------
+AwContentsIoThreadClient::AwContentsIoThreadClient() {
+}
+
+AwContentsIoThreadClient::AwContentsIoThreadClient(const JavaRef<jobject>& obj)
+    : java_object_(obj) {
+}
+
+AwContentsIoThreadClient::AwContentsIoThreadClient(
+    const AwContentsIoThreadClient& orig)
+    : java_object_(orig.java_object_) {
+}
+
+void AwContentsIoThreadClient::operator=(const AwContentsIoThreadClient& rhs) {
+  java_object_.Reset(rhs.java_object_);
+}
+
+AwContentsIoThreadClient::~AwContentsIoThreadClient() {
+}
+
+scoped_ptr<InterceptedRequestData>
+AwContentsIoThreadClient::ShouldInterceptRequest(
+    const net::URLRequest* request) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (java_object_.is_null())
+    return scoped_ptr<InterceptedRequestData>();
+
+  JNIEnv* env = AttachCurrentThread();
+  ScopedJavaLocalRef<jstring> jstring_url =
+      ConvertUTF8ToJavaString(env, request->url().spec());
+  ScopedJavaLocalRef<jobject> ret =
+      Java_AwContentsIoThreadClient_shouldInterceptRequest(
+          env, java_object_.obj(), jstring_url.obj());
+  if (ret.is_null())
+    return scoped_ptr<InterceptedRequestData>();
+  return scoped_ptr<InterceptedRequestData>(
+      new InterceptedRequestData(ret));
+}
+
+} // namespace android_webview