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.
chromiumembedded · Aug 9, 2024 · 701c947 · 701c947
1 parent 889cfcf
commit 701c947
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 42 deletions.
diff --git 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<void(content::NavigationHandle&)>&
           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
@@ -526,7 +526,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<void(content::NavigationHandle&)>&
@@ -535,17 +535,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
@@ -90,11 +90,10 @@ class ChromeBrowserDelegate : public cef::BrowserDelegate {
   const std::optional<SkRegion> GetDraggableRegion() const override;
   void WindowFullscreenStateChanged() override;
   bool HasViewsHostedOpener() const override;
-  content::WebContents* OpenURLFromTabEx(
-      content::WebContents* source,
-      const content::OpenURLParams& params,
-      base::OnceCallback<void(content::NavigationHandle&)>&
-          navigation_handle_callback) override;
+  bool OpenURLFromTabEx(content::WebContents* source,
+                        const content::OpenURLParams& params,
+                        base::OnceCallback<void(content::NavigationHandle&)>&
+                            navigation_handle_callback) override;
 
   // WebContentsDelegate methods:
   void WebContentsCreated(content::WebContents* source_contents,

diff --git a/patch/patch.cfg b/patch/patch.cfg
@@ -278,6 +278,10 @@ patches = [
   {
     # 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
@@ -173,7 +173,7 @@ index c54ec37e56ad7..fda899ec278ef 100644
      ]
    }
 diff --git chrome/browser/ui/browser.cc chrome/browser/ui/browser.cc
-index 681fe10f28260..89da7d87b306b 100644
+index 681fe10f28260..19a47e4a5bb22 100644
 --- chrome/browser/ui/browser.cc
 +++ chrome/browser/ui/browser.cc
 @@ -271,6 +271,25 @@
@@ -271,24 +271,22 @@ index 681fe10f28260..89da7d87b306b 100644
  }
 
  bool Browser::TabsNeedBeforeUnloadFired() const {
-@@ -1785,6 +1834,16 @@ WebContents* Browser::OpenURLFromTab(
+@@ -1785,6 +1834,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;
-@@ -1947,6 +2006,8 @@ void Browser::LoadingStateChanged(WebContents* source,
+@@ -1947,6 +2004,8 @@ void Browser::LoadingStateChanged(WebContents* source,
                                    bool should_show_loading_ui) {
    ScheduleUIUpdate(source, content::INVALIDATE_TYPE_LOAD);
    UpdateWindowForLoadingStateChanged(source, should_show_loading_ui);
@@ -297,7 +295,7 @@ index 681fe10f28260..89da7d87b306b 100644
  }
 
  void Browser::CloseContents(WebContents* source) {
-@@ -1975,6 +2036,8 @@ void Browser::SetContentsBounds(WebContents* source, const gfx::Rect& bounds) {
+@@ -1975,6 +2034,8 @@ void Browser::SetContentsBounds(WebContents* source, const gfx::Rect& bounds) {
  }
 
  void Browser::UpdateTargetURL(WebContents* source, const GURL& url) {
@@ -306,7 +304,7 @@ index 681fe10f28260..89da7d87b306b 100644
    if (!GetStatusBubble())
      return;
 
-@@ -1982,6 +2045,17 @@ void Browser::UpdateTargetURL(WebContents* source, const GURL& url) {
+@@ -1982,6 +2043,17 @@ void Browser::UpdateTargetURL(WebContents* source, const GURL& url) {
      GetStatusBubble()->SetURL(url);
  }
 
@@ -324,7 +322,7 @@ index 681fe10f28260..89da7d87b306b 100644
  void Browser::ContentsMouseEvent(WebContents* source, const ui::Event& event) {
    const ui::EventType type = event.type();
    const bool exited = type == ui::EventType::kMouseExited;
-@@ -2010,6 +2084,19 @@ bool Browser::TakeFocus(content::WebContents* source, bool reverse) {
+@@ -2010,6 +2082,19 @@ bool Browser::TakeFocus(content::WebContents* source, bool reverse) {
    return false;
  }
 
@@ -344,7 +342,7 @@ index 681fe10f28260..89da7d87b306b 100644
  void Browser::BeforeUnloadFired(WebContents* web_contents,
                                  bool proceed,
                                  bool* proceed_to_fire_unload) {
-@@ -2109,12 +2196,24 @@ void Browser::WebContentsCreated(WebContents* source_contents,
+@@ -2109,12 +2194,24 @@ void Browser::WebContentsCreated(WebContents* source_contents,
 
    // Make the tab show up in the task manager.
    task_manager::WebContentsTags::CreateForTabContents(new_contents);
@@ -369,7 +367,7 @@ index 681fe10f28260..89da7d87b306b 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();
-@@ -2127,6 +2226,13 @@ void Browser::RendererUnresponsive(
+@@ -2127,6 +2224,13 @@ void Browser::RendererUnresponsive(
  void Browser::RendererResponsive(
      WebContents* source,
      content::RenderWidgetHost* render_widget_host) {
@@ -383,7 +381,7 @@ index 681fe10f28260..89da7d87b306b 100644
    RenderWidgetHostView* view = render_widget_host->GetView();
    if (view && !render_widget_host->GetView()->IsHTMLFormPopup()) {
      TabDialogs::FromWebContents(source)->HideHungRendererDialog(
-@@ -2136,6 +2242,15 @@ void Browser::RendererResponsive(
+@@ -2136,6 +2240,15 @@ void Browser::RendererResponsive(
 
  content::JavaScriptDialogManager* Browser::GetJavaScriptDialogManager(
      WebContents* source) {
@@ -399,7 +397,7 @@ index 681fe10f28260..89da7d87b306b 100644
    return javascript_dialogs::TabModalDialogManager::FromWebContents(source);
  }
 
-@@ -2171,6 +2286,11 @@ void Browser::DraggableRegionsChanged(
+@@ -2171,6 +2284,11 @@ void Browser::DraggableRegionsChanged(
    if (app_controller_) {
      app_controller_->DraggableRegionsChanged(regions, contents);
    }
@@ -411,7 +409,7 @@ index 681fe10f28260..89da7d87b306b 100644
  }
 
  void Browser::DidFinishNavigation(
-@@ -2251,11 +2371,15 @@ void Browser::EnterFullscreenModeForTab(
+@@ -2251,11 +2369,15 @@ void Browser::EnterFullscreenModeForTab(
      const blink::mojom::FullscreenOptions& options) {
    exclusive_access_manager_->fullscreen_controller()->EnterFullscreenModeForTab(
        requesting_frame, options.display_id);
@@ -427,7 +425,7 @@ index 681fe10f28260..89da7d87b306b 100644
  }
 
  bool Browser::IsFullscreenForTabOrPending(const WebContents* web_contents) {
-@@ -2456,6 +2580,15 @@ void Browser::RequestMediaAccessPermission(
+@@ -2456,6 +2578,15 @@ void Browser::RequestMediaAccessPermission(
      content::WebContents* web_contents,
      const content::MediaStreamRequest& request,
      content::MediaResponseCallback callback) {
@@ -443,7 +441,7 @@ index 681fe10f28260..89da7d87b306b 100644
    const extensions::Extension* extension =
        GetExtensionForOrigin(profile_, request.security_origin);
    MediaCaptureDevicesDispatcher::GetInstance()->ProcessMediaAccessRequest(
-@@ -2998,9 +3131,11 @@ void Browser::RemoveScheduledUpdatesFor(WebContents* contents) {
+@@ -2998,9 +3129,11 @@ void Browser::RemoveScheduledUpdatesFor(WebContents* contents) {
  // Browser, Getters for UI (private):
 
  StatusBubble* Browser::GetStatusBubble() {
@@ -456,7 +454,7 @@ index 681fe10f28260..89da7d87b306b 100644
    }
 
    // We hide the status bar for web apps windows as this matches native
-@@ -3008,6 +3143,12 @@ StatusBubble* Browser::GetStatusBubble() {
+@@ -3008,6 +3141,12 @@ StatusBubble* Browser::GetStatusBubble() {
    // mode, as the minimal browser UI includes the status bar.
    if (web_app::AppBrowserController::IsWebApp(this) &&
        !app_controller()->HasMinimalUiButtons()) {
@@ -469,7 +467,7 @@ index 681fe10f28260..89da7d87b306b 100644
      return nullptr;
    }
 
-@@ -3157,6 +3298,8 @@ void Browser::SetAsDelegate(WebContents* web_contents, bool set_delegate) {
+@@ -3157,6 +3296,8 @@ void Browser::SetAsDelegate(WebContents* web_contents, bool set_delegate) {
      BookmarkTabHelper::FromWebContents(web_contents)->RemoveObserver(this);
      web_contents_collection_.StopObserving(web_contents);
    }
@@ -478,7 +476,7 @@ index 681fe10f28260..89da7d87b306b 100644
  }
 
  void Browser::TabDetachedAtImpl(content::WebContents* contents,
-@@ -3311,6 +3454,14 @@ bool Browser::PictureInPictureBrowserSupportsWindowFeature(
+@@ -3311,6 +3452,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
@@ -1,8 +1,16 @@
 diff --git chrome/browser/download/chrome_download_manager_delegate.cc chrome/browser/download/chrome_download_manager_delegate.cc
-index 1becf94d357e9..9cb014e242917 100644
+index 1becf94d357e9..10c169b092a3d 100644
 --- chrome/browser/download/chrome_download_manager_delegate.cc
 +++ chrome/browser/download/chrome_download_manager_delegate.cc
-@@ -157,6 +157,10 @@
+@@ -31,6 +31,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"
+@@ -157,6 +158,10 @@
  #include "chrome/browser/ash/policy/skyvault/skyvault_rename_handler.h"
  #endif
 
@@ -13,7 +21,7 @@ index 1becf94d357e9..9cb014e242917 100644
  using content::BrowserThread;
  using content::DownloadManager;
  using download::DownloadItem;
-@@ -513,6 +517,11 @@ ChromeDownloadManagerDelegate::ChromeDownloadManagerDelegate(Profile* profile)
+@@ -513,6 +518,11 @@ ChromeDownloadManagerDelegate::ChromeDownloadManagerDelegate(Profile* profile)
    download_dialog_bridge_ = std::make_unique<DownloadDialogBridge>();
    download_message_bridge_ = std::make_unique<DownloadMessageBridge>();
  #endif
@@ -25,7 +33,7 @@ index 1becf94d357e9..9cb014e242917 100644
  }
 
  ChromeDownloadManagerDelegate::~ChromeDownloadManagerDelegate() {
-@@ -572,6 +581,9 @@ void ChromeDownloadManagerDelegate::Shutdown() {
+@@ -572,6 +582,9 @@ void ChromeDownloadManagerDelegate::Shutdown() {
      download_manager_->RemoveObserver(this);
      download_manager_ = nullptr;
    }
@@ -35,7 +43,7 @@ index 1becf94d357e9..9cb014e242917 100644
  }
 
  void ChromeDownloadManagerDelegate::OnDownloadCanceledAtShutdown(
-@@ -640,6 +652,12 @@ bool ChromeDownloadManagerDelegate::DetermineDownloadTarget(
+@@ -640,6 +653,12 @@ bool ChromeDownloadManagerDelegate::DetermineDownloadTarget(
      ReportPDFLoadStatus(PDFLoadStatus::kTriggeredNoGestureDriveByDownload);
    }
 
@@ -48,6 +56,20 @@ index 1becf94d357e9..9cb014e242917 100644
    DownloadTargetDeterminer::CompletionCallback target_determined_callback =
        base::BindOnce(&ChromeDownloadManagerDelegate::OnDownloadTargetDetermined,
                       weak_ptr_factory_.GetWeakPtr(), download->GetId(),
+@@ -1027,8 +1046,11 @@ void ChromeDownloadManagerDelegate::OpenDownload(DownloadItem* download) {
+   Browser* browser =
+       web_contents ? chrome::FindBrowserWithTab(web_contents) : nullptr;
+   std::unique_ptr<chrome::ScopedTabbedBrowserDisplayer> 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<chrome::ScopedTabbedBrowserDisplayer>(profile_);
+     browser = browser_displayer->browser();
 diff --git chrome/browser/download/chrome_download_manager_delegate.h chrome/browser/download/chrome_download_manager_delegate.h
 index 2c99baa2c9fa8..b721db7058d8f 100644
 --- chrome/browser/download/chrome_download_manager_delegate.h