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

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

Index: android_webview/java/src/org/chromium/android_webview/AwContents.java
diff --git a/android_webview/java/src/org/chromium/android_webview/AwContents.java b/android_webview/java/src/org/chromium/android_webview/AwContents.java
index cffd535812ff6731ec2521883ef876108733150c..7a45cc12328499b5c30a0c01d324ba181b404c26 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContents.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwContents.java
@@ -24,6 +24,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;
@@ -85,6 +86,11 @@ public class AwContents {
     // Must call nativeUpdateLastHitTestData first to update this before use.
     private final HitTestData mPossiblyStaleHitTestData;
 
+    // Needed for re-creating a new ContentViewCore when this AwContents
+    // is used to host a pop up window.

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

looks like the sort of comment that may go out of date?

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

Fair enough, removed. Especially as we don't need the bool anymore.

+    private ContentViewCore.InternalAccessDelegate mInternalAccessAdapter;
+    private boolean mIsAccessFromFileUrlsGrantedByDefault;
+
     private static final class DestroyRunnable implements Runnable {
         private int mNativeAwContents;
         private DestroyRunnable(int nativeAwContents) {
@@ -212,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,
@@ -219,6 +226,8 @@ public class AwContents {
             NativeWindow nativeWindow, boolean privateBrowsing,
             boolean isAccessFromFileURLsGrantedByDefault) {
         mContainerView = containerView;
+        mInternalAccessAdapter = internalAccessAdapter;
+        mIsAccessFromFileUrlsGrantedByDefault = isAccessFromFileURLsGrantedByDefault;
         // Note that ContentViewCore must be set up before AwContents, as ContentViewCore
         // setup performs process initialisation work needed by AwContents.
         mContentViewCore = new ContentViewCore(containerView.getContext(),
@@ -229,9 +238,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());
@@ -350,6 +360,40 @@ public class AwContents {
         }
     }
 
+    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 .

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

spurious space

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

Done.

+
+        ContentViewCore cvc = new ContentViewCore(mContainerView.getContext(),

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

nit: cvc is odd name. if you want a short name, viewCore or contentCore or core
are good with me :-)

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

Went for newCore :)

+                ContentViewCore.PERSONALITY_VIEW);
+        cvc.initialize(mContainerView, mInternalAccessAdapter,
+                newWebContentsPtr, new AwNativeWindow(mContainerView.getContext()),
+                mIsAccessFromFileUrlsGrantedByDefault);

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

interestingly, we don't need to retain mIsAccessFromFileUrlsGranted across to
the next settings instance, as this will immediately get overritten with the
initFrom(oldSettings) below. So I'd be tempted to pass false here and just
comment it. (and avoid the need for this member)

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

sg.

+        cvc.setContentViewClient(mContentsClient);
+        mContentsClient.installWebContentsObserver(cvc);
+
+        ContentSettings oldSettings = mContentViewCore.getContentSettings();
+        cvc.getContentSettings().initFrom(oldSettings);
+
+        // Now swap the Java side reference.
+        mContentViewCore.destroy();
+        mContentViewCore = cvc;
+
+        // Now rewire native side to use the new WebContents.
+        nativeReInit(mNativeAwContents, newWebContentsPtr);

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

consider is we can factor any common init out from here and c'tor?

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

I don't think there's that much code we can share on this side, as the ordering
is a bit different. In the ctor, we must new a ContentViewCore before running
the AwContents init, and we need to have run AwContents init to get the web
contents. When doing the popup, we don't need the native AwContents init
inbetween the CVC ctor and initialize call.

[joth, 2012/11/16 21:52:56]

But this is just an accidental dependency. We should be explicitly calling
AndroidBrowserProcess.initContentViewProcess() somewhere (e.g. the
AwContentsFactory class I have a TODO to create :-), For now in AwContents c'tor
would be fine.
Then we can order these two methods anyway it makes sense to.

+        nativeSetIoThreadClient(mNativeAwContents, mIoThreadClient);
+        nativeSetInterceptNavigationDelegate(mNativeAwContents, mInterceptNavigationDelegate);
+
+        // Finally poke the new ContentViewCore with the size of the container view.
+        mContentViewCore.onSizeChanged(mContainerView.getWidth(), mContainerView.getHeight(), 0, 0);

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

nit: could skip this if the width ==0 && height == 0

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

Done.

+    }
+
     //--------------------------------------------------------------------------------------------
     //  WebView[Provider] method implementations (where not provided by ContentViewCore)
     //--------------------------------------------------------------------------------------------
@@ -762,4 +806,6 @@ public class AwContents {
     private native void nativeOnSizeChanged(int nativeAwContents, int w, int h, int ow, int oh);
     private native void nativeSetWindowViewVisibility(int nativeAwContents, boolean windowVisible,
             boolean viewVisible);
+    private native int nativeReleasePopupWebContents(int nativeAwContents);
+    private native void  nativeReInit(int nativeAwContents, int nativeNewWebContents);
 }