From fcaeb2bbb99f4ef72791079d0b6d868d2bbdcc69 Mon Sep 17 00:00:00 2001 From: AlexeyBarabash Date: Tue, 28 Jan 2025 13:02:20 +0200 Subject: [PATCH] [cr134] [Android] Changes related to InterceptNavigationDelegate.ShouldIgnoreNavigation: - new args at InterceptNavigationDelegate.ShouldIgnoreNavigation: should_run_async, result_callback; - return type changed from bool to void Related Chromium change: https://source.chromium.org/chromium/chromium/src/+/8c2b10e85ff860959aaf6bf86f48bd03fc998ccb Reland "Fix Async Navigation Intercept using the wrong URL for fast redirects" Original change's description: Fix Async Navigation Intercept using the wrong URL for fast redirects Prior to this change, if we received a redirect before the async shouldIgnoreNavigation call was run (which does happen in practice) we would still run the pending shouldIgnoreNavigation call but the NavigationHandle would already be updated to the redirect URL and so the result would potentially be wrong. It's also complicted to reason about multiple checks in flight at once. This change ensures that before processing the redirect, we finish processing the previous step in the navigation (either the initial navigation or a previous redirect). Also, I moved the async task to Java to make use of the cached ExternalNavigationParams as the NavigationHandle is mutable and could be modified between when the task is posted and when it's run. We'll still get the vast majority of the benefit from the async path as it's extremely rare that we get a redirect before finishing the check, so this should be performance neutral. No new Kill Switch is needed for this change, as we can re-use the kAsyncCheck Kill Switch to disable async nav altogether if we really need to. Bug: 381535042 Change-Id: I674aca65a4f2e5ca31670cf52929e8f87daf0528 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6194176 --- .../intercept_navigation_delegate_impl.cc | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/chromium_src/components/external_intents/android/intercept_navigation_delegate_impl.cc b/chromium_src/components/external_intents/android/intercept_navigation_delegate_impl.cc index 63953029a15..1d65c7a9b2f 100644 --- a/chromium_src/components/external_intents/android/intercept_navigation_delegate_impl.cc +++ b/chromium_src/components/external_intents/android/intercept_navigation_delegate_impl.cc @@ -10,6 +10,7 @@ #include "brave/components/constants/pref_names.h" #include "components/external_intents/android/jni_headers/InterceptNavigationDelegateImpl_jni.h" #include "components/navigation_interception/intercept_navigation_delegate.h" +#include "components/navigation_interception/intercept_navigation_throttle.h" #include "components/prefs/pref_service.h" #include "components/user_prefs/user_prefs.h" #include "content/public/browser/browser_context.h" @@ -30,6 +31,7 @@ namespace external_intents { namespace { using navigation_interception::InterceptNavigationDelegate; +using navigation_interception::InterceptNavigationThrottle; class BraveInterceptNavigationDelegate : public InterceptNavigationDelegate { public: @@ -40,14 +42,18 @@ class BraveInterceptNavigationDelegate : public InterceptNavigationDelegate { pref_service_ = pref_service; } - bool ShouldIgnoreNavigation( - content::NavigationHandle* navigation_handle) override { + void ShouldIgnoreNavigation( + content::NavigationHandle* navigation_handle, + bool should_run_async, + InterceptNavigationThrottle::ResultCallback result_callback) override { if (ShouldPlayVideoInBrowser(GURL(base::EscapeExternalHandlerValue( - navigation_handle->GetURL().spec())))) - return false; + navigation_handle->GetURL().spec())))) { + std::move(result_callback).Run(false); + return; + } return InterceptNavigationDelegate::ShouldIgnoreNavigation( - navigation_handle); + navigation_handle, should_run_async, std::move(result_callback)); } private: