From 62b2b5a80f0d0c3f23fe21bd7924a5b660984f61 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Thu, 8 Aug 2024 16:34:44 -0400 Subject: [PATCH] Route Download bubble file open to OnOpenURLFromTab (fixes #3750) Some downloaded file types [1] default to opening in a Browser. Open requests for these file types originating from the Download bubble UI should route to the source Browser (call OnOpenURLFromTab). If OnOpenURLFromTab is unhandled proceed with the default Chrome behavior of opening the URL in a new default Browser. [1] PDF, html, etc. For the complete list of file types see ChromeDownloadManagerDelegate::IsOpenInBrowserPreferredForFile. --- libcef/browser/chrome/browser_delegate.h | 7 ++-- .../browser/chrome/chrome_browser_delegate.cc | 28 +++++++++---- .../browser/chrome/chrome_browser_delegate.h | 9 ++-- patch/patch.cfg | 4 ++ patch/patches/chrome_browser_browser.patch | 42 +++++++++---------- patch/patches/chrome_browser_download.patch | 32 +++++++++++--- 6 files changed, 80 insertions(+), 42 deletions(-) diff --git a/libcef/browser/chrome/browser_delegate.h b/libcef/browser/chrome/browser_delegate.h index 032b8b886..feb2ca6d2 100644 --- a/libcef/browser/chrome/browser_delegate.h +++ b/libcef/browser/chrome/browser_delegate.h @@ -164,13 +164,14 @@ class BrowserDelegate : public content::WebContentsDelegate { virtual bool HasViewsHostedOpener() const { return false; } // Same as OpenURLFromTab but only taking |navigation_handle_callback| - // if the return value is non-nullptr. - virtual content::WebContents* OpenURLFromTabEx( + // if the return value is false. Return false to cancel the navigation + // or true to proceed with default chrome handling. + virtual bool OpenURLFromTabEx( content::WebContents* source, const content::OpenURLParams& params, base::OnceCallback& navigation_handle_callback) { - return nullptr; + return true; } }; diff --git a/libcef/browser/chrome/chrome_browser_delegate.cc b/libcef/browser/chrome/chrome_browser_delegate.cc index 48a490520..afbdf27e9 100644 --- a/libcef/browser/chrome/chrome_browser_delegate.cc +++ b/libcef/browser/chrome/chrome_browser_delegate.cc @@ -527,7 +527,7 @@ void ChromeBrowserDelegate::WebContentsCreated( /*is_devtools_popup=*/false, opener); } -content::WebContents* ChromeBrowserDelegate::OpenURLFromTabEx( +bool ChromeBrowserDelegate::OpenURLFromTabEx( content::WebContents* source, const content::OpenURLParams& params, base::OnceCallback& @@ -536,17 +536,31 @@ content::WebContents* ChromeBrowserDelegate::OpenURLFromTabEx( // Reading List sidebar. In that case we default to using the Browser's // currently active WebContents. if (!source) { + // GetActiveWebContents() may return nullptr if we're in a new Browser + // created using ScopedTabbedBrowserDisplayer. This new Browser does + // not have a WebContents yet. source = browser_->tab_strip_model()->GetActiveWebContents(); - DCHECK(source); + } + if (!source) { + LOG(WARNING) << "Failed to identify target browser for " + << params.url.spec(); + // Proceed with default chrome handling. + return true; } - // Return nullptr to cancel the navigation. Otherwise, proceed with default - // chrome handling. if (auto delegate = GetDelegateForWebContents(source)) { - return delegate->OpenURLFromTabEx(source, params, - navigation_handle_callback); + // Returns nullptr to cancel the navigation. + const bool cancel = + delegate->OpenURLFromTabEx(source, params, + navigation_handle_callback) == nullptr; + if (cancel) { + // Cancel the navigation. + return false; + } } - return nullptr; + + // Proceed with default chrome handling. + return true; } void ChromeBrowserDelegate::LoadingStateChanged(content::WebContents* source, diff --git a/libcef/browser/chrome/chrome_browser_delegate.h b/libcef/browser/chrome/chrome_browser_delegate.h index bddc2498c..c582e8922 100644 --- a/libcef/browser/chrome/chrome_browser_delegate.h +++ b/libcef/browser/chrome/chrome_browser_delegate.h @@ -90,11 +90,10 @@ class ChromeBrowserDelegate : public cef::BrowserDelegate { const std::optional GetDraggableRegion() const override; void WindowFullscreenStateChanged() override; bool HasViewsHostedOpener() const override; - content::WebContents* OpenURLFromTabEx( - content::WebContents* source, - const content::OpenURLParams& params, - base::OnceCallback& - navigation_handle_callback) override; + bool OpenURLFromTabEx(content::WebContents* source, + const content::OpenURLParams& params, + base::OnceCallback& + navigation_handle_callback) override; // WebContentsDelegate methods: void WebContentsCreated(content::WebContents* source_contents, diff --git a/patch/patch.cfg b/patch/patch.cfg index 97886d9ae..f24271a8b 100644 --- a/patch/patch.cfg +++ b/patch/patch.cfg @@ -289,6 +289,10 @@ patches = [ # alloy: Support override of DownloadPrefs::FromBrowserContext # chrome: Support custom DownloadManagerDelegate handling. # https://github.com/chromiumembedded/cef/issues/3681 + # + # chrome: Allow routing of Download bubble file open to non-Tabbed + # source browser. + # https://github.com/chromiumembedded/cef/issues/3750 'name': 'chrome_browser_download', }, { diff --git a/patch/patches/chrome_browser_browser.patch b/patch/patches/chrome_browser_browser.patch index 8b4773fb3..066b4b854 100644 --- a/patch/patches/chrome_browser_browser.patch +++ b/patch/patches/chrome_browser_browser.patch @@ -154,7 +154,7 @@ index 49e6bdcbd55a6..0425eb49c4296 100644 ] } diff --git chrome/browser/ui/browser.cc chrome/browser/ui/browser.cc -index a7a3a0722bcbe..53051e86f03c1 100644 +index a7a3a0722bcbe..4a9ce76019fc3 100644 --- chrome/browser/ui/browser.cc +++ chrome/browser/ui/browser.cc @@ -271,6 +271,25 @@ @@ -252,24 +252,22 @@ index a7a3a0722bcbe..53051e86f03c1 100644 } bool Browser::TabsNeedBeforeUnloadFired() const { -@@ -1798,6 +1847,16 @@ WebContents* Browser::OpenURLFromTab( +@@ -1798,6 +1847,14 @@ WebContents* Browser::OpenURLFromTab( } #endif // BUILDFLAG(IS_CHROMEOS_ASH) +#if BUILDFLAG(ENABLE_CEF) -+ if (cef_browser_delegate_) { -+ auto web_contents = -+ cef_browser_delegate_->OpenURLFromTabEx(source, params, -+ navigation_handle_callback); -+ if (!web_contents) -+ return nullptr; ++ if (cef_browser_delegate_ && ++ !cef_browser_delegate_->OpenURLFromTabEx(source, params, ++ navigation_handle_callback)) { ++ return nullptr; + } +#endif + NavigateParams nav_params(this, params.url, params.transition); nav_params.FillNavigateParamsFromOpenURLParams(params); nav_params.source_contents = source; -@@ -1960,6 +2019,8 @@ void Browser::LoadingStateChanged(WebContents* source, +@@ -1960,6 +2017,8 @@ void Browser::LoadingStateChanged(WebContents* source, bool should_show_loading_ui) { ScheduleUIUpdate(source, content::INVALIDATE_TYPE_LOAD); UpdateWindowForLoadingStateChanged(source, should_show_loading_ui); @@ -278,7 +276,7 @@ index a7a3a0722bcbe..53051e86f03c1 100644 } void Browser::CloseContents(WebContents* source) { -@@ -1988,6 +2049,8 @@ void Browser::SetContentsBounds(WebContents* source, const gfx::Rect& bounds) { +@@ -1988,6 +2047,8 @@ void Browser::SetContentsBounds(WebContents* source, const gfx::Rect& bounds) { } void Browser::UpdateTargetURL(WebContents* source, const GURL& url) { @@ -287,7 +285,7 @@ index a7a3a0722bcbe..53051e86f03c1 100644 if (!GetStatusBubble()) return; -@@ -1995,6 +2058,17 @@ void Browser::UpdateTargetURL(WebContents* source, const GURL& url) { +@@ -1995,6 +2056,17 @@ void Browser::UpdateTargetURL(WebContents* source, const GURL& url) { GetStatusBubble()->SetURL(url); } @@ -305,7 +303,7 @@ index a7a3a0722bcbe..53051e86f03c1 100644 void Browser::ContentsMouseEvent(WebContents* source, const ui::Event& event) { const ui::EventType type = event.type(); const bool exited = type == ui::ET_MOUSE_EXITED; -@@ -2022,6 +2096,19 @@ bool Browser::TakeFocus(content::WebContents* source, bool reverse) { +@@ -2022,6 +2094,19 @@ bool Browser::TakeFocus(content::WebContents* source, bool reverse) { return false; } @@ -325,7 +323,7 @@ index a7a3a0722bcbe..53051e86f03c1 100644 void Browser::BeforeUnloadFired(WebContents* web_contents, bool proceed, bool* proceed_to_fire_unload) { -@@ -2121,12 +2208,24 @@ void Browser::WebContentsCreated(WebContents* source_contents, +@@ -2121,12 +2206,24 @@ void Browser::WebContentsCreated(WebContents* source_contents, // Make the tab show up in the task manager. task_manager::WebContentsTags::CreateForTabContents(new_contents); @@ -350,7 +348,7 @@ index a7a3a0722bcbe..53051e86f03c1 100644 // Don't show the page hung dialog when a HTML popup hangs because // the dialog will take the focus and immediately close the popup. RenderWidgetHostView* view = render_widget_host->GetView(); -@@ -2139,6 +2238,13 @@ void Browser::RendererUnresponsive( +@@ -2139,6 +2236,13 @@ void Browser::RendererUnresponsive( void Browser::RendererResponsive( WebContents* source, content::RenderWidgetHost* render_widget_host) { @@ -364,7 +362,7 @@ index a7a3a0722bcbe..53051e86f03c1 100644 RenderWidgetHostView* view = render_widget_host->GetView(); if (view && !render_widget_host->GetView()->IsHTMLFormPopup()) { TabDialogs::FromWebContents(source)->HideHungRendererDialog( -@@ -2148,6 +2254,15 @@ void Browser::RendererResponsive( +@@ -2148,6 +2252,15 @@ void Browser::RendererResponsive( content::JavaScriptDialogManager* Browser::GetJavaScriptDialogManager( WebContents* source) { @@ -380,7 +378,7 @@ index a7a3a0722bcbe..53051e86f03c1 100644 return javascript_dialogs::TabModalDialogManager::FromWebContents(source); } -@@ -2183,6 +2298,11 @@ void Browser::DraggableRegionsChanged( +@@ -2183,6 +2296,11 @@ void Browser::DraggableRegionsChanged( if (app_controller_) { app_controller_->DraggableRegionsChanged(regions, contents); } @@ -392,7 +390,7 @@ index a7a3a0722bcbe..53051e86f03c1 100644 } void Browser::DidFinishNavigation( -@@ -2263,11 +2383,15 @@ void Browser::EnterFullscreenModeForTab( +@@ -2263,11 +2381,15 @@ void Browser::EnterFullscreenModeForTab( const blink::mojom::FullscreenOptions& options) { exclusive_access_manager_->fullscreen_controller()->EnterFullscreenModeForTab( requesting_frame, options.display_id); @@ -408,7 +406,7 @@ index a7a3a0722bcbe..53051e86f03c1 100644 } bool Browser::IsFullscreenForTabOrPending(const WebContents* web_contents) { -@@ -2467,6 +2591,15 @@ void Browser::RequestMediaAccessPermission( +@@ -2467,6 +2589,15 @@ void Browser::RequestMediaAccessPermission( content::WebContents* web_contents, const content::MediaStreamRequest& request, content::MediaResponseCallback callback) { @@ -424,7 +422,7 @@ index a7a3a0722bcbe..53051e86f03c1 100644 const extensions::Extension* extension = GetExtensionForOrigin(profile_, request.security_origin); MediaCaptureDevicesDispatcher::GetInstance()->ProcessMediaAccessRequest( -@@ -3011,9 +3144,11 @@ void Browser::RemoveScheduledUpdatesFor(WebContents* contents) { +@@ -3011,9 +3142,11 @@ void Browser::RemoveScheduledUpdatesFor(WebContents* contents) { // Browser, Getters for UI (private): StatusBubble* Browser::GetStatusBubble() { @@ -437,7 +435,7 @@ index a7a3a0722bcbe..53051e86f03c1 100644 } // We hide the status bar for web apps windows as this matches native -@@ -3021,6 +3156,12 @@ StatusBubble* Browser::GetStatusBubble() { +@@ -3021,6 +3154,12 @@ StatusBubble* Browser::GetStatusBubble() { // mode, as the minimal browser UI includes the status bar. if (web_app::AppBrowserController::IsWebApp(this) && !app_controller()->HasMinimalUiButtons()) { @@ -450,7 +448,7 @@ index a7a3a0722bcbe..53051e86f03c1 100644 return nullptr; } -@@ -3170,6 +3311,8 @@ void Browser::SetAsDelegate(WebContents* web_contents, bool set_delegate) { +@@ -3170,6 +3309,8 @@ void Browser::SetAsDelegate(WebContents* web_contents, bool set_delegate) { BookmarkTabHelper::FromWebContents(web_contents)->RemoveObserver(this); web_contents_collection_.StopObserving(web_contents); } @@ -459,7 +457,7 @@ index a7a3a0722bcbe..53051e86f03c1 100644 } void Browser::TabDetachedAtImpl(content::WebContents* contents, -@@ -3324,6 +3467,14 @@ bool Browser::PictureInPictureBrowserSupportsWindowFeature( +@@ -3324,6 +3465,14 @@ bool Browser::PictureInPictureBrowserSupportsWindowFeature( bool Browser::SupportsWindowFeatureImpl(WindowFeature feature, bool check_can_support) const { diff --git a/patch/patches/chrome_browser_download.patch b/patch/patches/chrome_browser_download.patch index 807794588..7b7287cde 100644 --- a/patch/patches/chrome_browser_download.patch +++ b/patch/patches/chrome_browser_download.patch @@ -1,8 +1,16 @@ diff --git chrome/browser/download/chrome_download_manager_delegate.cc chrome/browser/download/chrome_download_manager_delegate.cc -index bd2c41e30f01e..d87b8baf3de2b 100644 +index bd2c41e30f01e..e57592e75dec7 100644 --- chrome/browser/download/chrome_download_manager_delegate.cc +++ chrome/browser/download/chrome_download_manager_delegate.cc -@@ -150,6 +150,10 @@ +@@ -30,6 +30,7 @@ + #include "base/time/time.h" + #include "build/build_config.h" + #include "build/chromeos_buildflags.h" ++#include "cef/libcef/features/features.h" + #include "chrome/browser/browser_process.h" + #include "chrome/browser/download/bubble/download_bubble_prefs.h" + #include "chrome/browser/download/download_core_service.h" +@@ -150,6 +151,10 @@ #include "chrome/browser/safe_browsing/download_protection/download_protection_util.h" #endif @@ -13,7 +21,7 @@ index bd2c41e30f01e..d87b8baf3de2b 100644 using content::BrowserThread; using content::DownloadManager; using download::DownloadItem; -@@ -500,6 +504,11 @@ ChromeDownloadManagerDelegate::ChromeDownloadManagerDelegate(Profile* profile) +@@ -500,6 +505,11 @@ ChromeDownloadManagerDelegate::ChromeDownloadManagerDelegate(Profile* profile) download_dialog_bridge_ = std::make_unique(); download_message_bridge_ = std::make_unique(); #endif @@ -25,7 +33,7 @@ index bd2c41e30f01e..d87b8baf3de2b 100644 } ChromeDownloadManagerDelegate::~ChromeDownloadManagerDelegate() { -@@ -559,6 +568,9 @@ void ChromeDownloadManagerDelegate::Shutdown() { +@@ -559,6 +569,9 @@ void ChromeDownloadManagerDelegate::Shutdown() { download_manager_->RemoveObserver(this); download_manager_ = nullptr; } @@ -35,7 +43,7 @@ index bd2c41e30f01e..d87b8baf3de2b 100644 } void ChromeDownloadManagerDelegate::OnDownloadCanceledAtShutdown( -@@ -627,6 +639,12 @@ bool ChromeDownloadManagerDelegate::DetermineDownloadTarget( +@@ -627,6 +640,12 @@ bool ChromeDownloadManagerDelegate::DetermineDownloadTarget( ReportPDFLoadStatus(PDFLoadStatus::kTriggeredNoGestureDriveByDownload); } @@ -48,6 +56,20 @@ index bd2c41e30f01e..d87b8baf3de2b 100644 DownloadTargetDeterminer::CompletionCallback target_determined_callback = base::BindOnce(&ChromeDownloadManagerDelegate::OnDownloadTargetDetermined, weak_ptr_factory_.GetWeakPtr(), download->GetId(), +@@ -1015,8 +1034,11 @@ void ChromeDownloadManagerDelegate::OpenDownload(DownloadItem* download) { + Browser* browser = + web_contents ? chrome::FindBrowserWithTab(web_contents) : nullptr; + std::unique_ptr browser_displayer; +- if (!browser || +- !browser->CanSupportWindowFeature(Browser::FEATURE_TABSTRIP)) { ++ if (!browser ++#if !BUILDFLAG(ENABLE_CEF) ++ || !browser->CanSupportWindowFeature(Browser::FEATURE_TABSTRIP) ++#endif ++ ) { + browser_displayer = + std::make_unique(profile_); + browser = browser_displayer->browser(); diff --git chrome/browser/download/chrome_download_manager_delegate.h chrome/browser/download/chrome_download_manager_delegate.h index af7f1d5fac12b..1cb1d01464c11 100644 --- chrome/browser/download/chrome_download_manager_delegate.h