From ebcd9cb97dec283986d1810d6c8868bfc98567d9 Mon Sep 17 00:00:00 2001 From: Sergio Villar Senin Date: Wed, 22 Nov 2023 18:23:11 +0100 Subject: [PATCH] Notify clients when the web engine wants to create a new Window Certain navigations (like target=_blank links) require the web engine to create a new Window. In those cases Chromium creates a new WebContents object and notifies the client (Wolvic in this case) about that. However the current architecture of Wolvic does not allow us to properly use that new WebContents object to create a new tab. To circumvent that, we can use the fact that Chromium cancels the creation of a new window if the ownership of the WebContents object is not taken when AddNewContents() is called on the delegate. We precisely do that (we let the new WebContents die) but we use the target_url (the URL of the new window) to notify the client (Wolvic) via a new method called onCreateNewWindow() that was added to a new object called WolvicWebContentsDelegate which directly inherits from WebContentsDelegateAndroid (the one we were using previously). The client just needs to override that onCreateWindow() method and trigger the creation of a new window there using the passed in URL. --- wolvic/BUILD.gn | 8 ++- wolvic/browser/tab_jni.cc | 6 +-- wolvic/browser/wolvic_contents.cc | 3 +- wolvic/browser/wolvic_contents.h | 7 +-- .../browser/wolvic_web_contents_delegate.cc | 50 +++++++++++++++++++ wolvic/browser/wolvic_web_contents_delegate.h | 34 +++++++++++++ wolvic/java/org/chromium/wolvic/Tab.java | 6 +-- .../wolvic/WolvicWebContentsDelegate.java | 18 +++++++ 8 files changed, 119 insertions(+), 13 deletions(-) create mode 100644 wolvic/browser/wolvic_web_contents_delegate.cc create mode 100644 wolvic/browser/wolvic_web_contents_delegate.h create mode 100644 wolvic/java/org/chromium/wolvic/WolvicWebContentsDelegate.java diff --git a/wolvic/BUILD.gn b/wolvic/BUILD.gn index a64bbc25bf62c3..c2409a5ec2e1d3 100644 --- a/wolvic/BUILD.gn +++ b/wolvic/BUILD.gn @@ -36,8 +36,6 @@ shared_library("libcontent_native") { "browser/session_settings.cc", "browser/session_settings.h", "browser/session_settings_jni.cc", - "browser/wolvic_contents.cc", - "browser/wolvic_contents.h", "browser/tab_jni.cc", "browser/vr/moz_external_vr.h", "browser/vr/wolvic_xr_integration_client.cc", @@ -54,6 +52,10 @@ shared_library("libcontent_native") { "browser/vr/wvr_manager.h", "browser/vr/wvr_thread.cc", "browser/vr/wvr_thread.h", + "browser/wolvic_contents.cc", + "browser/wolvic_contents.h", + "browser/wolvic_web_contents_delegate.cc", + "browser/wolvic_web_contents_delegate.h", "renderer/browser_exposed_renderer_interfaces.cc", "renderer/browser_exposed_renderer_interfaces.h", "renderer/wolvic_content_renderer_client.cc", @@ -214,6 +216,7 @@ generate_jni("jni_headers") { "java/org/chromium/wolvic/Tab.java", "java/org/chromium/wolvic/VRManager.java", "java/org/chromium/wolvic/WVRSurfaceTexture.java", + "java/org/chromium/wolvic/WolvicWebContentsDelegate.java", ] } @@ -225,6 +228,7 @@ android_library("wolvic_java") { "java/org/chromium/wolvic/TabCompositorView.java", "java/org/chromium/wolvic/VRManager.java", "java/org/chromium/wolvic/WVRSurfaceTexture.java", + "java/org/chromium/wolvic/WolvicWebContentsDelegate.java", ] annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ] deps = [ diff --git a/wolvic/browser/tab_jni.cc b/wolvic/browser/tab_jni.cc index 4885f94a5e3620..278bc203588c71 100644 --- a/wolvic/browser/tab_jni.cc +++ b/wolvic/browser/tab_jni.cc @@ -4,12 +4,12 @@ #include "wolvic/jni_headers/Tab_jni.h" -#include "components/embedder_support/android/delegate/web_contents_delegate_android.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/navigation_controller.h" #include "content/public/browser/web_contents.h" #include "url/gurl.h" #include "wolvic/browser/wolvic_contents.h" +#include "wolvic/browser/wolvic_web_contents_delegate.h" #include "wolvic/wolvic_browser_context.h" #include "wolvic/wolvic_content_browser_client.h" @@ -47,10 +47,8 @@ void JNI_Tab_SetWebContentsDelegate( DCHECK(web_contents); auto* wolvic_contents = WolvicContents::FromWebContents(web_contents); - DCHECK(wolvic_contents); - auto web_contents_delegate = - std::make_unique(env, jweb_contents_delegate); + std::make_unique(env, jweb_contents_delegate); wolvic_contents->SetDelegate(std::move(web_contents_delegate)); } diff --git a/wolvic/browser/wolvic_contents.cc b/wolvic/browser/wolvic_contents.cc index 61d1a90d099fe4..9c506c01e6b072 100644 --- a/wolvic/browser/wolvic_contents.cc +++ b/wolvic/browser/wolvic_contents.cc @@ -10,6 +10,7 @@ #include "wolvic/wolvic_browser_context.h" #include "wolvic/wolvic_content_browser_client.h" +#include "wolvic/browser/wolvic_web_contents_delegate.h" using content::WebContents; @@ -75,7 +76,7 @@ WolvicContents::DidFinishNavigation(content::NavigationHandle* navigation_handle } void -WolvicContents::SetDelegate(std::unique_ptr delegate) { +WolvicContents::SetDelegate(std::unique_ptr delegate) { web_contents_delegate_ = std::move(delegate); web_contents_->SetDelegate(web_contents_delegate_.get()); } diff --git a/wolvic/browser/wolvic_contents.h b/wolvic/browser/wolvic_contents.h index 0baf63451ac2b6..c9017fa205c65f 100644 --- a/wolvic/browser/wolvic_contents.h +++ b/wolvic/browser/wolvic_contents.h @@ -5,11 +5,12 @@ #ifndef WOLVIC_BROWSER_WOLVIC_CONTENTS_H_ #define WOLVIC_BROWSER_WOLVIC_CONTENTS_H_ -#include "components/embedder_support/android/delegate/web_contents_delegate_android.h" #include "content/public/browser/web_contents_observer.h" namespace wolvic { +class WolvicWebContentsDelegate; + class WolvicContents : public content::WebContentsObserver { public: static WolvicContents* FromWebContents(content::WebContents* web_contents); @@ -24,11 +25,11 @@ class WolvicContents : public content::WebContentsObserver { void DidFinishNavigation(content::NavigationHandle*) override; - void SetDelegate(std::unique_ptr delegate); + void SetDelegate(std::unique_ptr); private: std::unique_ptr web_contents_; - std::unique_ptr web_contents_delegate_; + std::unique_ptr web_contents_delegate_; }; } // namespace content diff --git a/wolvic/browser/wolvic_web_contents_delegate.cc b/wolvic/browser/wolvic_web_contents_delegate.cc new file mode 100644 index 00000000000000..0d8615afcf334b --- /dev/null +++ b/wolvic/browser/wolvic_web_contents_delegate.cc @@ -0,0 +1,50 @@ +// Copyright 2023 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "wolvic/browser/wolvic_web_contents_delegate.h" + +#include "base/android/jni_android.h" +#include "base/android/scoped_java_ref.h" +#include "content/public/browser/web_contents.h" +#include "wolvic/browser/wolvic_contents.h" +#include "wolvic/jni_headers/WolvicWebContentsDelegate_jni.h" +#include "url/android/gurl_android.h" + +namespace wolvic { + +WolvicWebContentsDelegate::WolvicWebContentsDelegate(JNIEnv* env, jobject obj) + : WebContentsDelegateAndroid(env, obj) {} + +WolvicWebContentsDelegate::~WolvicWebContentsDelegate() = default; + +using base::android::ScopedJavaLocalRef; + +// Called by web_contents_impl.cc whenever a navigation +// requires the creation of a new window (for example +// a link with target=_blank +void WolvicWebContentsDelegate::AddNewContents( + content::WebContents* source, + std::unique_ptr new_contents, + const GURL& target_url, + WindowOpenDisposition disposition, + const blink::mojom::WindowFeatures& window_features, + bool user_gesture, + bool* was_blocked) { + + JNIEnv* env = base::android::AttachCurrentThread(); + ScopedJavaLocalRef java_delegate = GetJavaDelegate(env); + DCHECK(java_delegate.obj()); + + // We let new_contents die on purpouse (by not assigning the + // unique_ptr to any local variable/attribute because that way + // we are telling the web engine that we do not want a new + // window to be created. We'll in any case use the target_urlformatter + // to notify the client (Wolvic) so that it can create the window + // for the new URL on its own. + ScopedJavaLocalRef java_gurl = + url::GURLAndroid::FromNativeGURL(env, target_url); + Java_WolvicWebContentsDelegate_onCreateNewWindow(env, java_delegate, java_gurl); +} + +} // namespace wolvic diff --git a/wolvic/browser/wolvic_web_contents_delegate.h b/wolvic/browser/wolvic_web_contents_delegate.h new file mode 100644 index 00000000000000..6f8281d7cfa62c --- /dev/null +++ b/wolvic/browser/wolvic_web_contents_delegate.h @@ -0,0 +1,34 @@ +// Copyright 2023 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef WOLVIC_BROWSER_WOLVIC_WEB_CONTENTS_DELEGATE_H_ +#define WOLVIC_BROWSER_WOLVIC_WEB_CONTENTS_DELEGATE_H_ + +#include "components/embedder_support/android/delegate/web_contents_delegate_android.h" + +namespace wolvic { + +class WolvicWebContentsDelegate + : public web_contents_delegate_android::WebContentsDelegateAndroid { + public: + WolvicWebContentsDelegate(JNIEnv* env, jobject obj); + ~WolvicWebContentsDelegate() override; + + // See //android_webview/docs/how-does-on-create-window-work.md for more + // details. + void AddNewContents(content::WebContents* source, + std::unique_ptr new_contents, + const GURL& target_url, + WindowOpenDisposition disposition, + const blink::mojom::WindowFeatures& window_features, + bool user_gesture, + bool* was_blocked) final; + + private: + std::unique_ptr new_contents_; +}; + +}// namespace wolvic + +#endif // WOLVIC_BROWSER_WOLVIC_WEB_CONTENTS_DELEGATE_H_ diff --git a/wolvic/java/org/chromium/wolvic/Tab.java b/wolvic/java/org/chromium/wolvic/Tab.java index 58388bfcd129f8..40602b5d163559 100644 --- a/wolvic/java/org/chromium/wolvic/Tab.java +++ b/wolvic/java/org/chromium/wolvic/Tab.java @@ -11,7 +11,6 @@ import org.chromium.base.ContextUtils; import org.chromium.base.annotations.JNINamespace; import org.chromium.base.annotations.NativeMethods; -import org.chromium.components.embedder_support.delegate.WebContentsDelegateAndroid; import org.chromium.components.embedder_support.view.ContentView; import org.chromium.components.url_formatter.UrlFormatter; import org.chromium.content_public.browser.ImeAdapter; @@ -21,6 +20,7 @@ import org.chromium.ui.base.ActivityWindowAndroid; import org.chromium.ui.base.IntentRequestTracker; import org.chromium.ui.base.ViewAndroidDelegate; +import org.chromium.wolvic.WolvicWebContentsDelegate; @JNINamespace("wolvic") public class Tab { @@ -35,7 +35,7 @@ public WebContents createWebContents(boolean is_off_the_record) { } public void setWebContentsDelegate( - WebContents webContents, WebContentsDelegateAndroid delegate) { + WebContents webContents, WolvicWebContentsDelegate delegate) { TabJni.get().setWebContentsDelegate(webContents, delegate); } @@ -96,6 +96,6 @@ public ImeAdapter getImeAdapter() { @NativeMethods public interface Natives { WebContents createWebContents(boolean is_off_the_record); - void setWebContentsDelegate(WebContents webContents, WebContentsDelegateAndroid delegate); + void setWebContentsDelegate(WebContents webContents, WolvicWebContentsDelegate delegate); } } diff --git a/wolvic/java/org/chromium/wolvic/WolvicWebContentsDelegate.java b/wolvic/java/org/chromium/wolvic/WolvicWebContentsDelegate.java new file mode 100644 index 00000000000000..4e15760da400a9 --- /dev/null +++ b/wolvic/java/org/chromium/wolvic/WolvicWebContentsDelegate.java @@ -0,0 +1,18 @@ +// Copyright 2023 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.wolvic; + +import androidx.annotation.VisibleForTesting; + +import org.chromium.url.GURL; +import org.chromium.base.annotations.CalledByNative; +import org.chromium.base.annotations.JNINamespace; +import org.chromium.components.embedder_support.delegate.WebContentsDelegateAndroid; + +@JNINamespace("wolvic") +public abstract class WolvicWebContentsDelegate extends WebContentsDelegateAndroid { + @CalledByNative + public abstract void onCreateNewWindow(GURL url); +}