[Android WebView] AwContentsClient.shouldCreate window callback part 2.

Part 1: https://chromiumcodereview.appspot.com/11362183/

The second part of this change implements the "true" path,
i.e. the user provides us with a new AwContents to host
the popup window. This arrives asynchronously.

While we are waiting for the reply from the user, we defer
loading of the popup contents using a resource throttle.

When we receive the callback we replace the ContentViewCore that
is contained in the new AwContents with one that wraps the WebContents
for the popup. This triggers an update to the AwIoThreadClient for
that AwContents, which resumes the deferred load for the popup.

Android only change and android bots green
NOTRY=TRUE

Review URL: https://chromiumcodereview.appspot.com/11348075

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@170721 0039d316-1c4b-4281-b951-d872f2087c98

android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc

@@ -4,12 +4,13 @@
 
 #include "android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.h"
 
-#include "android_webview/browser/aw_login_delegate.h"
 #include "android_webview/browser/aw_contents_io_thread_client.h"
+#include "android_webview/browser/aw_login_delegate.h"
 #include "android_webview/common/url_constants.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "content/components/navigation_interception/intercept_navigation_delegate.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/resource_controller.h"
 #include "content/public/browser/resource_dispatcher_host.h"
 #include "content/public/browser/resource_dispatcher_host_login_delegate.h"
@@ -18,12 +19,12 @@
 #include "net/base/load_flags.h"
 #include "net/url_request/url_request.h"
 
+using android_webview::AwContentsIoThreadClient;
+using content::BrowserThread;
 using content::InterceptNavigationDelegate;
 
 namespace {
 
-using android_webview::AwContentsIoThreadClient;
-
 base::LazyInstance<android_webview::AwResourceDispatcherHostDelegate>
     g_webview_resource_dispatcher_host_delegate = LAZY_INSTANCE_INITIALIZER;
 
@@ -40,65 +41,119 @@ void SetCacheControlFlag(
   request->set_load_flags(load_flags);
 }
 
+}  // namespace
+
+namespace android_webview {
+
 // Calls through the IoThreadClient to check the embedders settings to determine
-// if the request should be cancelled.
+// if the request should be cancelled. There may not always be an IoThreadClient
+// available for the |child_id|, |route_id| pair (in the case of newly created
+// pop up windows, for example) and in that case the request and the client
+// callbacks will be deferred the request until a client is ready.
 class IoThreadClientThrottle : public content::ResourceThrottle {
  public:
   IoThreadClientThrottle(int child_id,
                          int route_id,
-                         net::URLRequest* request)
-      : child_id_(child_id),
-        route_id_(route_id),
-        request_(request) { }
+                         net::URLRequest* request);
+  virtual ~IoThreadClientThrottle();
+
+  // From content::ResourceThrottle
   virtual void WillStartRequest(bool* defer) OVERRIDE;
+  virtual void WillRedirectRequest(const GURL& new_url, bool* defer) OVERRIDE;
 
-  scoped_ptr<AwContentsIoThreadClient> GetIoThreadClient() {
-    return AwContentsIoThreadClient::FromID(child_id_, route_id_);
-  }
+  bool MaybeDeferRequest(bool* defer);
+  void OnIoThreadClientReady(int new_child_id, int new_route_id);
+  bool MaybeBlockRequest();
+  bool ShouldBlockRequest();
+  scoped_ptr<AwContentsIoThreadClient> GetIoThreadClient();
+  int get_child_id() const { return child_id_; }
+  int get_route_id() const { return route_id_; }
 
- private:
+private:
   int child_id_;
   int route_id_;
   net::URLRequest* request_;
 };
 
+IoThreadClientThrottle::IoThreadClientThrottle(int child_id,
+                                               int route_id,
+                                               net::URLRequest* request)
+    : child_id_(child_id),
+      route_id_(route_id),
+      request_(request) { }
+
+IoThreadClientThrottle::~IoThreadClientThrottle() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  g_webview_resource_dispatcher_host_delegate.Get().
+      RemovePendingThrottleOnIoThread(this);
+}
+
 void IoThreadClientThrottle::WillStartRequest(bool* defer) {
-  // If there is no IO thread client set at this point, use a
-  // restrictive policy. This can happen for blocked popup
-  // windows for example.
-  // TODO(benm): Revert this to a DCHECK when the we support
-  // pop up windows being created in the WebView, as at that
-  // time we should always have an IoThreadClient at this
-  // point (i.e., the one associated with the new popup).
-  if (!GetIoThreadClient()) {
+  if (!MaybeDeferRequest(defer)) {
+    MaybeBlockRequest();
+  }
+}
+
+void IoThreadClientThrottle::WillRedirectRequest(const GURL& new_url,
+                                                 bool* defer) {
+  WillStartRequest(defer);
+}
+
+bool IoThreadClientThrottle::MaybeDeferRequest(bool* defer) {
+  scoped_ptr<AwContentsIoThreadClient> io_client =
+      AwContentsIoThreadClient::FromID(child_id_, route_id_);
+  *defer = false;
+  if (!io_client.get()) {
+    *defer = true;
+    AwResourceDispatcherHostDelegate::AddPendingThrottle(
+        child_id_, route_id_, this);
+  }
+  return *defer;
+}
+
+void IoThreadClientThrottle::OnIoThreadClientReady(int new_child_id,
+                                                   int new_route_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  if (!MaybeBlockRequest()) {
+    controller()->Resume();
+  }
+}
+
+bool IoThreadClientThrottle::MaybeBlockRequest() {
+  if (ShouldBlockRequest()) {
     controller()->CancelWithError(net::ERR_ACCESS_DENIED);
-    return;
+    return true;
   }
+  return false;
+}
+
+bool IoThreadClientThrottle::ShouldBlockRequest() {
+  scoped_ptr<AwContentsIoThreadClient> io_client =
+      AwContentsIoThreadClient::FromID(child_id_, route_id_);
+  DCHECK(io_client.get());
 
   // Part of implementation of WebSettings.allowContentAccess.
   if (request_->url().SchemeIs(android_webview::kContentScheme) &&
-      GetIoThreadClient()->ShouldBlockContentUrls()) {
-    controller()->CancelWithError(net::ERR_ACCESS_DENIED);
-    return;
+      io_client->ShouldBlockContentUrls()) {
+    return true;
   }
 
   // Part of implementation of WebSettings.allowFileAccess.
   if (request_->url().SchemeIsFile() &&
-      GetIoThreadClient()->ShouldBlockFileUrls()) {
+      io_client->ShouldBlockFileUrls()) {
     const GURL& url = request_->url();
     if (!url.has_path() ||
         // Application's assets and resources are always available.
         (url.path().find(android_webview::kAndroidResourcePath) != 0 &&
          url.path().find(android_webview::kAndroidAssetPath) != 0)) {
-      controller()->CancelWithError(net::ERR_ACCESS_DENIED);
-      return;
+      return true;
     }
   }
 
-  if (GetIoThreadClient()->ShouldBlockNetworkLoads()) {
+  if (io_client->ShouldBlockNetworkLoads()) {
     if (request_->url().SchemeIs(chrome::kFtpScheme)) {
-      controller()->CancelWithError(net::ERR_ACCESS_DENIED);
-      return;
+      return true;
     }
     SetCacheControlFlag(request_, net::LOAD_ONLY_FROM_CACHE);
   } else {
@@ -118,11 +173,13 @@ void IoThreadClientThrottle::WillStartRequest(bool* defer) {
         break;
     }
   }
+  return false;
 }
 
-}  // namespace
-
-namespace android_webview {
+scoped_ptr<AwContentsIoThreadClient>
+    IoThreadClientThrottle::GetIoThreadClient() {
+  return AwContentsIoThreadClient::FromID(child_id_, route_id_);
+}
 
 // static
 void AwResourceDispatcherHostDelegate::ResourceDispatcherHostCreated() {
@@ -193,4 +250,63 @@ bool AwResourceDispatcherHostDelegate::HandleExternalProtocol(const GURL& url,
   return false;
 }
 
+void AwResourceDispatcherHostDelegate::RemovePendingThrottleOnIoThread(
+    IoThreadClientThrottle* throttle) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  PendingThrottleMap::iterator it = pending_throttles_.find(
+      ChildRouteIDPair(throttle->get_child_id(), throttle->get_route_id()));
+  if (it != pending_throttles_.end()) {
+    pending_throttles_.erase(it);
+  }
+}
+
+// static
+void AwResourceDispatcherHostDelegate::OnIoThreadClientReady(
+    int new_child_id,
+    int new_route_id) {
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+      base::Bind(
+          &AwResourceDispatcherHostDelegate::OnIoThreadClientReadyInternal,
+          base::Unretained(
+              g_webview_resource_dispatcher_host_delegate.Pointer()),
+          new_child_id, new_route_id));
+}
+
+// static
+void AwResourceDispatcherHostDelegate::AddPendingThrottle(
+    int child_id,
+    int route_id,
+    IoThreadClientThrottle* pending_throttle) {
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+      base::Bind(
+          &AwResourceDispatcherHostDelegate::AddPendingThrottleOnIoThread,
+          base::Unretained(
+              g_webview_resource_dispatcher_host_delegate.Pointer()),
+          child_id, route_id, pending_throttle));
 }
+
+void AwResourceDispatcherHostDelegate::AddPendingThrottleOnIoThread(
+    int child_id,
+    int route_id,
+    IoThreadClientThrottle* pending_throttle) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  pending_throttles_.insert(
+      std::pair<ChildRouteIDPair, IoThreadClientThrottle*>(
+          ChildRouteIDPair(child_id, route_id), pending_throttle));
+}
+
+void AwResourceDispatcherHostDelegate::OnIoThreadClientReadyInternal(
+    int new_child_id,
+    int new_route_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  PendingThrottleMap::iterator it = pending_throttles_.find(
+      ChildRouteIDPair(new_child_id, new_route_id));
+
+  if (it != pending_throttles_.end()) {
+    IoThreadClientThrottle* throttle = it->second;
+    throttle->OnIoThreadClientReady(new_child_id, new_route_id);
+    pending_throttles_.erase(it);
+  }
+}
+
+}  // namespace android_webview

android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.h

@@ -5,9 +5,10 @@
 #ifndef ANDROID_WEBVIEW_LIB_RENDERER_HOST_AW_RESOURCE_DISPATCHER_HOST_DELEGATE_H_
 #define ANDROID_WEBVIEW_LIB_RENDERER_HOST_AW_RESOURCE_DISPATCHER_HOST_DELEGATE_H_
 
-#include "content/public/browser/resource_dispatcher_host_delegate.h"
+#include <map>
 
 #include "base/lazy_instance.h"
+#include "content/public/browser/resource_dispatcher_host_delegate.h"
 
 namespace content {
 class ResourceDispatcherHostLoginDelegate;
@@ -15,6 +16,8 @@ class ResourceDispatcherHostLoginDelegate;
 
 namespace android_webview {
 
+class IoThreadClientThrottle;
+
 class AwResourceDispatcherHostDelegate
     : public content::ResourceDispatcherHostDelegate {
  public:
@@ -42,12 +45,32 @@ class AwResourceDispatcherHostDelegate
                                       int child_id,
                                       int route_id) OVERRIDE;
 
+  void RemovePendingThrottleOnIoThread(IoThreadClientThrottle* throttle);
+
+  static void OnIoThreadClientReady(int new_child_id, int new_route_id);
+  static void AddPendingThrottle(int child_id,
+                                 int route_id,
+                                 IoThreadClientThrottle* pending_throttle);
+
  private:
   friend struct base::DefaultLazyInstanceTraits<
       AwResourceDispatcherHostDelegate>;
   AwResourceDispatcherHostDelegate();
   virtual ~AwResourceDispatcherHostDelegate();
 
+  // These methods must be called on IO thread.
+  void OnIoThreadClientReadyInternal(int child_id, int route_id);
+  void AddPendingThrottleOnIoThread(int child_id,
+                                    int route_id,
+                                    IoThreadClientThrottle* pending_throttle);
+
+  typedef std::pair<int, int> ChildRouteIDPair;
+  typedef std::map<ChildRouteIDPair, IoThreadClientThrottle*>
+      PendingThrottleMap;
+
+  // Only accessed on the IO thread.
+  PendingThrottleMap pending_throttles_;
+
   DISALLOW_COPY_AND_ASSIGN(AwResourceDispatcherHostDelegate);
 };
 

android_webview/java/src/org/chromium/android_webview/AwContents.java

@@ -25,6 +25,7 @@ import android.webkit.ValueCallback;
 import org.chromium.base.CalledByNative;
 import org.chromium.base.JNINamespace;
 import org.chromium.base.ThreadUtils;
+import org.chromium.content.browser.ContentSettings;
 import org.chromium.content.browser.ContentViewCore;
 import org.chromium.content.browser.LoadUrlParams;
 import org.chromium.content.browser.NavigationHistory;
@@ -78,6 +79,7 @@ public class AwContents {
     private AwContentsClient mContentsClient;
     private AwContentsIoThreadClient mIoThreadClient;
     private InterceptNavigationDelegateImpl mInterceptNavigationDelegate;
+    private ContentViewCore.InternalAccessDelegate mInternalAccessAdapter;
     // This can be accessed on any thread after construction. See AwContentsIoThreadClient.
     private final AwSettings mSettings;
     private final ClientCallbackHandler mClientCallbackHandler;
@@ -216,6 +218,7 @@ public class AwContents {
      * @param contentsClient will receive API callbacks from this WebView Contents
      * @param privateBrowsing whether this is a private browsing instance of WebView.
      * @param isAccessFromFileURLsGrantedByDefault passed to ContentViewCore.initialize.
+     * TODO(benm): Remove the nativeWindow parameter.
      */
     public AwContents(ViewGroup containerView,
             ContentViewCore.InternalAccessDelegate internalAccessAdapter,
@@ -223,6 +226,7 @@ public class AwContents {
             NativeWindow nativeWindow, boolean privateBrowsing,
             boolean isAccessFromFileURLsGrantedByDefault) {
         mContainerView = containerView;
+        mInternalAccessAdapter = internalAccessAdapter;
         // Note that ContentViewCore must be set up before AwContents, as ContentViewCore
         // setup performs process initialisation work needed by AwContents.
         mContentViewCore = new ContentViewCore(containerView.getContext(),
@@ -233,9 +237,10 @@ public class AwContents {
         mClientCallbackHandler = new ClientCallbackHandler();
 
         mContentViewCore.initialize(containerView, internalAccessAdapter,
-                nativeGetWebContents(mNativeAwContents), nativeWindow,
+                nativeGetWebContents(mNativeAwContents),
+                new AwNativeWindow(mContainerView.getContext()),
                 isAccessFromFileURLsGrantedByDefault);
-        mContentViewCore.setContentViewClient(contentsClient);
+        mContentViewCore.setContentViewClient(mContentsClient);
         mContentsClient.installWebContentsObserver(mContentViewCore);
 
         mSettings = new AwSettings(mContentViewCore.getContext());
@@ -356,6 +361,56 @@ public class AwContents {
         }
     }
 
+    /**
+     * Called on the "source" AwContents that is opening the popup window to
+     * provide the AwContents to host the pop up content.
+     */
+    public void supplyContentsForPopup(AwContents newContents) {
+        int popupWebContents = nativeReleasePopupWebContents(mNativeAwContents);
+        assert popupWebContents != 0;
+        newContents.setNewWebContents(popupWebContents);
+    }
+
+    private void setNewWebContents(int newWebContentsPtr) {
+        // When setting a new WebContents, we new up a ContentViewCore that will
+        // wrap it and then swap it.
+        ContentViewCore newCore = new ContentViewCore(mContainerView.getContext(),
+                ContentViewCore.PERSONALITY_VIEW);
+        // Note we pass false for isAccessFromFileURLsGrantedByDefault as we'll
+        // set it correctly when when we copy the settings from the old ContentViewCore
+        // into the new one.
+        newCore.initialize(mContainerView, mInternalAccessAdapter,
+                newWebContentsPtr, new AwNativeWindow(mContainerView.getContext()),
+                false);
+        newCore.setContentViewClient(mContentsClient);
+        mContentsClient.installWebContentsObserver(newCore);
+
+        ContentSettings oldSettings = mContentViewCore.getContentSettings();
+        newCore.getContentSettings().initFrom(oldSettings);
+
+        // Now swap the Java side reference.
+        mContentViewCore.destroy();
+        mContentViewCore = newCore;
+
+        // Now rewire native side to use the new WebContents.
+        nativeSetWebContents(mNativeAwContents, newWebContentsPtr);
+        nativeSetIoThreadClient(mNativeAwContents, mIoThreadClient);
+        nativeSetInterceptNavigationDelegate(mNativeAwContents, mInterceptNavigationDelegate);
+
+        // Finally poke the new ContentViewCore with the size of the container view and show it.
+        if (mContainerView.getWidth() != 0 || mContainerView.getHeight() != 0) {
+            mContentViewCore.onSizeChanged(
+                    mContainerView.getWidth(), mContainerView.getHeight(), 0, 0);
+        }
+        nativeDidInitializeContentViewCore(mNativeAwContents,
+                mContentViewCore.getNativeContentViewCore());
+        if (mContainerView.getVisibility() == View.VISIBLE) {
+            // The popup window was hidden when we prompted the embedder to display
+            // it, so show it again now we have a container.
+            mContentViewCore.onShow();
+        }
+    }
+
     //--------------------------------------------------------------------------------------------
     //  WebView[Provider] method implementations (where not provided by ContentViewCore)
     //--------------------------------------------------------------------------------------------
@@ -629,9 +684,15 @@ public class AwContents {
 
     private void updateVisiblityState() {
         if (mNativeAwContents == 0 || mIsPaused) return;
-        nativeSetWindowViewVisibility(mNativeAwContents,
-                mContainerView.getWindowVisibility() == View.VISIBLE,
-                mContainerView.getVisibility() == View.VISIBLE);
+        boolean windowVisible = mContainerView.getWindowVisibility() == View.VISIBLE;
+        boolean viewVisible = mContainerView.getVisibility() == View.VISIBLE;
+        nativeSetWindowViewVisibility(mNativeAwContents, windowVisible, viewVisible);
+
+        if (viewVisible) {
+          mContentViewCore.onShow();
+        } else {
+          mContentViewCore.onHide();
+        }
     }
 
 
@@ -834,4 +895,7 @@ public class AwContents {
 
     // Returns false if restore state fails.
     private native boolean nativeRestoreFromOpaqueState(int nativeAwContents, byte[] state);
+
+    private native int nativeReleasePopupWebContents(int nativeAwContents);
+    private native void  nativeSetWebContents(int nativeAwContents, int nativeNewWebContents);
 }

android_webview/java/src/org/chromium/android_webview/AwContentsClient.java

@@ -172,7 +172,9 @@ public abstract class AwContentsClient extends ContentViewClient {
     }
 
     void installWebContentsObserver(ContentViewCore contentViewCore) {
-        assert mWebContentsObserver == null;
+        if (mWebContentsObserver != null) {
+            mWebContentsObserver.detachFromWebContents();
+        }
         mWebContentsObserver = new AwWebContentsObserver(contentViewCore);
     }
 

android_webview/native/aw_contents.cc

@@ -7,6 +7,7 @@
 #include "android_webview/browser/aw_browser_main_parts.h"
 #include "android_webview/browser/net_disk_cache_remover.h"
 #include "android_webview/browser/renderer_host/aw_render_view_host_ext.h"
+#include "android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.h"
 #include "android_webview/common/aw_hit_test_data.h"
 #include "android_webview/native/aw_browser_dependency_factory.h"
 #include "android_webview/native/aw_contents_io_thread_client_impl.h"
@@ -17,6 +18,7 @@
 #include "base/android/jni_string.h"
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/message_loop.h"
 #include "base/pickle.h"
 #include "base/supports_user_data.h"
 #include "content/components/navigation_interception/intercept_navigation_delegate.h"
@@ -24,6 +26,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/cert_store.h"
 #include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/ssl_status.h"
 #include "jni/AwContents_jni.h"
@@ -126,9 +129,14 @@ AwContents::AwContents(JNIEnv* env,
   android_webview::AwBrowserDependencyFactory* dependency_factory =
       android_webview::AwBrowserDependencyFactory::GetInstance();
 
-  web_contents_.reset(dependency_factory->CreateWebContents(private_browsing));
+  // TODO(joth): rather than create and set the WebContents here, expose the
+  // factory method to java side and have that orchestrate the construction
+  // order.
+  SetWebContents(dependency_factory->CreateWebContents(private_browsing));
+}
 
-  DCHECK(!AwContents::FromWebContents(web_contents_.get()));
+void AwContents::SetWebContents(content::WebContents* web_contents) {
+  web_contents_.reset(web_contents);
   web_contents_->SetUserData(kAwContentsUserDataKey,
                              new AwContentsUserData(this));
 
@@ -142,6 +150,10 @@ AwContents::AwContents(JNIEnv* env,
   }
 }
 
+void AwContents::SetWebContents(JNIEnv* env, jobject obj, jint new_wc) {
+  SetWebContents(reinterpret_cast<content::WebContents*>(new_wc));
+}
+
 AwContents::~AwContents() {
   DCHECK(AwContents::FromWebContents(web_contents_.get()) == this);
   web_contents_->RemoveUserData(kAwContentsUserDataKey);
@@ -302,6 +314,9 @@ void AwContents::SetIoThreadClient(JNIEnv* env, jobject obj, jobject client) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   AwContentsIoThreadClientImpl::Associate(
       web_contents_.get(), ScopedJavaLocalRef<jobject>(env, client));
+  int child_id = web_contents_->GetRenderProcessHost()->GetID();
+  int route_id = web_contents_->GetRoutingID();
+  AwResourceDispatcherHostDelegate::OnIoThreadClientReady(child_id, route_id);
 }
 
 void AwContents::SetInterceptNavigationDelegate(JNIEnv* env,
@@ -512,4 +527,20 @@ jboolean AwContents::RestoreFromOpaqueState(
   return RestoreFromPickle(&iterator, web_contents_.get());
 }
 
+void AwContents::SetPendingWebContentsForPopup(
+    scoped_ptr<content::WebContents> pending) {
+  if (pending_contents_.get()) {
+    // TODO(benm): Support holding multiple pop up window requests.
+    LOG(WARNING) << "Blocking popup window creation as an outstanding "
+                 << "popup window is still pending.";
+    MessageLoop::current()->DeleteSoon(FROM_HERE, pending.release());
+    return;
+  }
+  pending_contents_ = pending.Pass();
+}
+
+jint AwContents::ReleasePopupWebContents(JNIEnv* env, jobject obj) {
+  return reinterpret_cast<jint>(pending_contents_.release());
+}
+
 }  // namespace android_webview

android_webview/native/aw_contents.h

@@ -67,6 +67,8 @@ class AwContents : public FindHelper::Listener,
 
   // Methods called from Java.
   jint GetWebContents(JNIEnv* env, jobject obj);
+  void SetWebContents(JNIEnv* env, jobject obj, jint web_contents);
+
   void DidInitializeContentViewCore(JNIEnv* env, jobject obj,
                                     jint content_view_core);
   void Destroy(JNIEnv* env, jobject obj);
@@ -107,8 +109,12 @@ class AwContents : public FindHelper::Listener,
   virtual void ScheduleComposite() OVERRIDE;
   virtual void OnSwapBuffersCompleted() OVERRIDE;
 
+  void SetPendingWebContentsForPopup(scoped_ptr<content::WebContents> pending);
+  jint ReleasePopupWebContents(JNIEnv* env, jobject obj);
+
  private:
   void Invalidate();
+  void SetWebContents(content::WebContents* web_contents);
 
   JavaObjectWeakGlobalRef java_ref_;
   scoped_ptr<content::WebContents> web_contents_;
@@ -120,6 +126,7 @@ class AwContents : public FindHelper::Listener,
   bool view_visible_;
   bool compositor_visible_;
   bool is_composite_pending_;
+  scoped_ptr<content::WebContents> pending_contents_;
 
   DISALLOW_COPY_AND_ASSIGN(AwContents);
 };

android_webview/native/aw_web_contents_delegate.cc

@@ -4,10 +4,11 @@
 
 #include "android_webview/native/aw_web_contents_delegate.h"
 
-#include "base/lazy_instance.h"
 #include "android_webview/browser/find_helper.h"
 #include "android_webview/native/aw_contents.h"
 #include "android_webview/native/aw_javascript_dialog_creator.h"
+#include "base/lazy_instance.h"
+#include "base/message_loop.h"
 #include "content/public/browser/android/download_controller_android.h"
 #include "content/public/browser/web_contents.h"
 #include "jni/AwWebContentsDelegate_jni.h"
@@ -79,13 +80,22 @@ void AwWebContentsDelegate::AddNewContents(content::WebContents* source,
       GetJavaDelegate(env).obj(), is_dialog, user_gesture);
 
   if (create_popup) {
-    // TODO(benm): Implement, taking ownership of new_contents.
-    NOTIMPLEMENTED() << "Popup windows are currently not supported for "
-                     << "the chromium powered Android WebView.";
+    // The embedder would like to display the popup and we will receive
+    // a callback from them later with an AwContents to use to display
+    // it. The source AwContents takes ownership of the new WebContents
+    // until then, and when the callback is made we will swap the WebContents
+    // out into the new AwContents.
+    AwContents::FromWebContents(source)->SetPendingWebContentsForPopup(
+        make_scoped_ptr(new_contents));
+    // Hide the WebContents for the pop up now, we will show it again
+    // when the user calls us back with an AwContents to use to show it.
+    new_contents->WasHidden();
   } else {
     // The embedder has forgone their chance to display this popup
-    // window, so we're done with the WebContents now.
-    delete new_contents;
+    // window, so we're done with the WebContents now. We use
+    // DeleteSoon as WebContentsImpl may call methods on |new_contents|
+    // after this method returns.
+    MessageLoop::current()->DeleteSoon(FROM_HERE, new_contents);
   }
 
   if (was_blocked) {