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

android_webview/native/aw_contents.cc

Index: android_webview/native/aw_contents.cc
diff --git a/android_webview/native/aw_contents.cc b/android_webview/native/aw_contents.cc
index 0c8b5bd4f03f0765fb88432887b7f2696d94afa8..ba02d18e7f9ab05e507aa4d509f381ca4833fecf 100644
--- a/android_webview/native/aw_contents.cc
+++ b/android_webview/native/aw_contents.cc
@@ -100,6 +100,17 @@ AwContents::AwContents(JNIEnv* env,
   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);
@@ -379,4 +390,10 @@ void AwContents::SetWindowViewVisibility(JNIEnv* env, jobject obj,
   // 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