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

android_webview/native/aw_contents.cc

 // 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.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/common/aw_hit_test_data.h"
 #include "android_webview/native/aw_browser_dependency_factory.h"
@@ skipping 82 matching lines @@
   web_contents_.reset(dependency_factory->CreateWebContents(private_browsing));
 
   DCHECK(!AwContents::FromWebContents(web_contents_.get()));
   web_contents_->SetUserData(kAwContentsUserDataKey,
                              new AwContentsUserData(this));
 
   web_contents_->SetDelegate(web_contents_delegate_.get());
   render_view_host_ext_.reset(new AwRenderViewHostExt(web_contents_.get()));
 }
 
+void AwContents::ReInit(JNIEnv* env, jobject obj, jint new_wc) {

[joth, 2012/11/16 06:24:31]

InstallWebContents or SetWebContents ?

[benm (inactive), 2012/11/16 12:29:09]

Done.

+  WebContents* wc = reinterpret_cast<WebContents*>(new_wc);
+  web_contents_.reset(wc);

[joth, 2012/11/16 06:24:31]

|wc| seems unneeded here? just do the cast in the reset()

[benm (inactive), 2012/11/16 12:29:09]

sg. Left over from when I was doing other things in this function I think.

+  web_contents_->SetUserData(kAwContentsUserDataKey,
+                             new AwContentsUserData(this));
+
+  web_contents_->SetDelegate(web_contents_delegate_.get());
+  render_view_host_ext_.reset(new AwRenderViewHostExt(web_contents_.get()));

[joth, 2012/11/16 06:24:31]

factor out a method for all this and share with constructor?

or.... just delete it from the c'tor and have the java side call both methods:
mNativeAwContents = nativeInit();
nativeInstallWebContents(mNativeAwContents, nativeWebContents)

[benm (inactive), 2012/11/16 12:29:09]

Done.

+  web_contents_->WasShown();

[joth, 2012/11/16 06:24:31]

seems odd to tuck a visibility state change into an 'init' method.

[benm (inactive), 2012/11/16 12:29:09]

mm, yeah. Kind of made sense while this is a "init the popup window" method, but
agree bit dodgier when factored out into a general setWebContents method. I
think we can probably trigger it with a ContentViewCore call in AwContents.java.

+}
+
 AwContents::~AwContents() {
   DCHECK(AwContents::FromWebContents(web_contents_.get()) == this);
   web_contents_->RemoveUserData(kAwContentsUserDataKey);
   if (find_helper_.get())
     find_helper_->SetListener(NULL);
 }
 
 void AwContents::DrawGL(AwDrawGLInfo* draw_info) {
   // TODO(joth): Do some drawing.
 }
@@ skipping 259 matching lines @@
                                int w, int h, int ow, int oh) {
   // TODO(joth): Set Compositor size.
 }
 
 void AwContents::SetWindowViewVisibility(JNIEnv* env, jobject obj,
                                          bool window_visible,
                                          bool view_visible) {
   // TODO(joth): Set the Compositor visibility.
 }
 
+void AwContents::SetPendingWebContentsForPopup(content::WebContents* pending) {

[joth, 2012/11/16 06:24:31]

consider passing: scoped_ptr<WebContents> pending_content

[benm (inactive), 2012/11/16 12:29:09]

Done.

+  // TODO(benm): Support holding multiple pop up window requests.
+  DCHECK(!pending_contents_);

[joth, 2012/11/16 06:24:31]

I'd be tempted to do:
if (pending_contents_) {
 LOG(WARNING) << "blocking window popup as previous popup still pending";
 delete pending;
}

[benm (inactive), 2012/11/16 12:29:09]

Done.

+  pending_contents_.reset(pending);
+}
+
 }  // namespace android_webview