[IntentHandling] Show intent picker icon for open in new tab.

Currently if we open the link in a new tab (e.g. ctrl + click, context
menu, from another app), the intent picker icon will not show up even
there are apps that can handle that URL. This is because:
1. We check the browser for the web contents in navigation throttle
before the adding the web contents to tab strip model for new tab case.
2. When open new tab from new-tab page, the starting URL is empty.
This CL fixes this issue by moving these checks to only check for
automatically pop up the bubble or automatically launch the app, but
still show the icon if there are app found.

BUG=1141049, b/171260488

Change-Id: Idcf4b07c34304bf6664f874142128ceddb6d6569
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2500974Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Maggie Cai <mxcai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821656}

chrome/browser/apps/intent_helper/apps_navigation_throttle.cc

@@ -104,6 +104,11 @@ GURL GetStartingGURL(content::NavigationHandle* navigation_handle) {
   return initiator_origin.has_value() ? initiator_origin->GetURL() : GURL();
 }
 
+bool InAppBrowser(content::WebContents* web_contents) {
+  Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
+  return !browser || browser->deprecated_is_app();
+}
+
 }  // namespace
 
 namespace apps {
@@ -259,11 +264,13 @@ bool AppsNavigationThrottle::CanCreate(content::WebContents* web_contents) {
     return false;
   }
 
-  // Do not create the throttle if there is no browser for the WebContents or we
-  // are already in an app browser. The former can happen if an initial
-  // navigation is reparented into a new app browser instance.
+  // Do not create the throttle if we are already in an app browser.
+  // It is possible that the web contents is not inserted to tab strip
+  // model at this stage (e.g. open url in new tab). So if we cannot
+  // find a browser at this moment, skip the check and this will be handled
+  // in later stage.
   Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
-  if (!browser || browser->deprecated_is_app())
+  if (browser && browser->deprecated_is_app())
     return false;
 
   return true;
@@ -442,6 +449,7 @@ void AppsNavigationThrottle::ShowIntentPickerForApps(
     ui_displayed_ = false;
     return;
   }
+  IntentPickerTabHelper::SetShouldShowIcon(web_contents, true);
   Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
   if (!browser)
     return;
@@ -450,7 +458,6 @@ void AppsNavigationThrottle::ShowIntentPickerForApps(
   switch (picker_show_state) {
     case PickerShowState::kOmnibox:
       ui_displayed_ = false;
-      IntentPickerTabHelper::SetShouldShowIcon(web_contents, true);
       break;
     case PickerShowState::kPopOut: {
       bool show_persistence_options = ShouldShowPersistenceOptions(apps);
@@ -486,6 +493,23 @@ bool AppsNavigationThrottle::navigate_from_link() const {
   return navigate_from_link_;
 }
 
+bool AppsNavigationThrottle::ShouldAutoDisplayUi(
+    const std::vector<apps::IntentPickerAppInfo>& apps_for_picker,
+    content::WebContents* web_contents,
+    const GURL& url) {
+  if (apps_for_picker.empty())
+    return false;
+
+  if (InAppBrowser(web_contents))
+    return false;
+
+  if (!ShouldOverrideUrlLoading(starting_url_, url))
+    return false;
+
+  DCHECK(ui_auto_display_service_);
+  return ui_auto_display_service_->ShouldAutoDisplayUi(url);
+}
+
 ThrottleCheckResult AppsNavigationThrottle::HandleRequest() {
   content::NavigationHandle* handle = navigation_handle();
   // If the navigation won't update the current document, don't check intent for
@@ -518,30 +542,33 @@ ThrottleCheckResult AppsNavigationThrottle::HandleRequest() {
 
   MaybeRemoveComingFromArcFlag(web_contents, starting_url_, url);
 
-  if (!ShouldOverrideUrlLoading(starting_url_, url))
-    return content::NavigationThrottle::PROCEED;
-
   base::Optional<ThrottleCheckResult> tab_strip_capture =
       CaptureExperimentalTabStripWebAppScopeNavigations(web_contents, handle);
   if (tab_strip_capture.has_value())
     return tab_strip_capture.value();
 
-  // Handles apps that are automatically launched and the navigation needs to be
-  // cancelled. This only applies on the new intent picker system, because we
-  // don't need to defer the navigation to find out preferred app anymore.
-  if (ShouldCancelNavigation(handle)) {
-    return content::NavigationThrottle::CANCEL_AND_IGNORE;
-  }
+  // Do not pop up the intent picker bubble or automatically launch the app if
+  // we shouldn't override url loading, or if we don't have a browser, or we are
+  // already in an app browser.
+  if (ShouldOverrideUrlLoading(starting_url_, url) &&
+      !InAppBrowser(web_contents)) {
+    // Handles apps that are automatically launched and the navigation needs to
+    // be cancelled. This only applies on the new intent picker system, because
+    // we don't need to defer the navigation to find out preferred app anymore.
+    if (ShouldCancelNavigation(handle)) {
+      return content::NavigationThrottle::CANCEL_AND_IGNORE;
+    }
 
-  if (ShouldDeferNavigation(handle)) {
-    // Handling is now deferred to ArcIntentPickerAppFetcher, which
-    // asynchronously queries ARC for apps, and runs
-    // OnDeferredNavigationProcessed() with an action based on whether an
-    // acceptable app was found and user consent to open received. We assume the
-    // UI is shown or a preferred app was found; reset to false if we resume the
-    // navigation.
-    ui_displayed_ = true;
-    return content::NavigationThrottle::DEFER;
+    if (ShouldDeferNavigation(handle)) {
+      // Handling is now deferred to ArcIntentPickerAppFetcher, which
+      // asynchronously queries ARC for apps, and runs
+      // OnDeferredNavigationProcessed() with an action based on whether an
+      // acceptable app was found and user consent to open received. We assume
+      // the UI is shown or a preferred app was found; reset to false if we
+      // resume the navigation.
+      ui_displayed_ = true;
+      return content::NavigationThrottle::DEFER;
+    }
   }
 
   // We didn't query ARC, so proceed with the navigation and query if we have an

chrome/browser/apps/intent_helper/apps_navigation_throttle.h

@@ -221,6 +221,11 @@ class AppsNavigationThrottle : public content::NavigationThrottle {
 
   bool navigate_from_link() const;
 
+  bool ShouldAutoDisplayUi(
+      const std::vector<apps::IntentPickerAppInfo>& apps_for_picker,
+      content::WebContents* web_contents,
+      const GURL& url);
+
   // Keeps track of whether we already shown the UI or preferred app. Since
   // AppsNavigationThrottle cannot wait for the user (due to the non-blocking
   // nature of the feature) the best we can do is check if we launched a

chrome/browser/chromeos/apps/intent_helper/chromeos_apps_navigation_throttle.cc

@@ -270,9 +270,6 @@ bool ChromeOsAppsNavigationThrottle::ShouldAutoDisplayUi(
     const std::vector<apps::IntentPickerAppInfo>& apps_for_picker,
     content::WebContents* web_contents,
     const GURL& url) {
-  if (apps_for_picker.empty())
-    return false;
-
   // If we only have PWAs in the app list, do not show the intent picker.
   // Instead just show the omnibox icon. This is to reduce annoyance to users
   // until "Remember my choice" is available for desktop PWAs.
@@ -281,8 +278,8 @@ bool ChromeOsAppsNavigationThrottle::ShouldAutoDisplayUi(
   if (ContainsOnlyPwasAndMacApps(apps_for_picker))
     return false;
 
-  DCHECK(ui_auto_display_service_);
-  return ui_auto_display_service_->ShouldAutoDisplayUi(url);
+  return AppsNavigationThrottle::ShouldAutoDisplayUi(apps_for_picker,
+                                                     web_contents, url);
 }
 
 bool ChromeOsAppsNavigationThrottle::ShouldLaunchPreferredApp(const GURL& url) {

chrome/browser/chromeos/apps/intent_helper/common_apps_navigation_throttle.cc

@@ -64,6 +64,12 @@ CommonAppsNavigationThrottle::MaybeCreate(content::NavigationHandle* handle) {
 
   content::WebContents* web_contents = handle->GetWebContents();
 
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+
+  if (!AppServiceProxyFactory::IsAppServiceAvailableForProfile(profile))
+    return nullptr;
+
   if (!apps::AppsNavigationThrottle::CanCreate(web_contents))
     return nullptr;
 
@@ -318,9 +324,6 @@ bool CommonAppsNavigationThrottle::ShouldAutoDisplayUi(
     const std::vector<apps::IntentPickerAppInfo>& apps_for_picker,
     content::WebContents* web_contents,
     const GURL& url) {
-  if (apps_for_picker.empty())
-    return false;
-
   // On devices with tablet form factor we should not pop out the intent
   // picker if Chrome has been chosen by the user as the platform for this URL.
   if (chromeos::switches::IsTabletFormFactor()) {
@@ -360,8 +363,8 @@ bool CommonAppsNavigationThrottle::ShouldAutoDisplayUi(
     }
   }
 
-  DCHECK(ui_auto_display_service_);
-  return ui_auto_display_service_->ShouldAutoDisplayUi(url);
+  return AppsNavigationThrottle::ShouldAutoDisplayUi(apps_for_picker,
+                                                     web_contents, url);
 }
 
 }  // namespace apps