Improve intent: URI handling

chrome/browser/chromeos/arc/arc_external_protocol_dialog.cc

Index: chrome/browser/chromeos/arc/arc_external_protocol_dialog.cc
diff --git a/chrome/browser/chromeos/arc/arc_external_protocol_dialog.cc b/chrome/browser/chromeos/arc/arc_external_protocol_dialog.cc
index 35157fd48b9e967d486ac1c389132da9e8239b80..50c2cfa2ad05515fcea9b5ec543ca8696b54f4d1 100644
--- a/chrome/browser/chromeos/arc/arc_external_protocol_dialog.cc
+++ b/chrome/browser/chromeos/arc/arc_external_protocol_dialog.cc
@@ -5,7 +5,6 @@
 #include "chrome/browser/chromeos/arc/arc_external_protocol_dialog.h"
 
 #include <memory>
-#include <string>
 #include <utility>
 #include <vector>
 
@@ -23,7 +22,11 @@
 #include "components/arc/intent_helper/page_transition_util.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/page_navigator.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/referrer.h"
+#include "ui/base/page_transition_types.h"
+#include "ui/base/window_open_disposition.h"
 #include "ui/gfx/image/image.h"
 #include "url/gurl.h"
 
@@ -65,6 +68,161 @@ void CloseTabIfNeeded(int render_process_host_id, int routing_id) {
     web_contents->Close();
 }
 
+// Shows |url| in the current tab.
+void OpenUrlInChrome(int render_process_host_id,
+                     int routing_id,
+                     const GURL& url) {
+  WebContents* web_contents =
+      tab_util::GetWebContentsByID(render_process_host_id, routing_id);
+  if (!web_contents)
+    return;
+
+  // Use the PAGE_TRANSITION_FROM_API qualifier so that this nativation won't
+  // end up showing the disambig dialog.
+  const ui::PageTransition page_transition_type = ui::PageTransitionFromInt(
+      ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_FROM_API);
+  constexpr bool kIsRendererInitiated = false;
+  const content::OpenURLParams params(
+      url, content::Referrer(), WindowOpenDisposition::CURRENT_TAB,

[djacobo_, 2016/10/24 18:17:36]

qq: does using content::Referrer() in here means we are discarding part of the
navigation story within this |web_contents|, are we OK with this?

[Yusuke Sato, 2016/10/24 19:30:11]

Let me make this a TODO. Sending a non-empty referrer following the privacy
policy requires a fair amount of work, I think.

[djacobo_, 2016/10/24 20:23:47]

Sounds fine to me, thanks.

+      page_transition_type, kIsRendererInitiated);
+  web_contents->OpenURL(params);
+}
+
+// Sends |url| to ARC.
+void HandleUrlInArc(int render_process_host_id,
+                    int routing_id,
+                    const std::pair<GURL, std::string>& url_and_package) {
+  auto* instance = ArcIntentHelperBridge::GetIntentHelperInstance(
+      "HandleUrl", kMinVersionForHandleUrl);
+  if (!instance)
+    return;
+
+  instance->HandleUrl(url_and_package.first.spec(), url_and_package.second);
+  CloseTabIfNeeded(render_process_host_id, routing_id);
+}
+
+// Finds |selected_app_package| from the |handlers| array and returns the index.
+// If the app is not found, returns |handlers.size()|.
+size_t GetAppIndex(const mojo::Array<mojom::IntentHandlerInfoPtr>& handlers,
+                   const std::string& selected_app_package) {
+  // If the user selected an app to continue the navigation, confirm that the
+  // |package_name| matches a valid option and return the index.

[djacobo_, 2016/10/24 18:17:36]

nit: kind of duplicated comment? I would keep the one at lines [104 -105] :) 

[Yusuke Sato, 2016/10/24 19:30:11]

Thanks, this is a copy-and-paste mistake. Moved the comment back to
OnIntentPickerClosed.

+  for (size_t i = 0; i < handlers.size(); ++i) {
+    if (handlers[i]->package_name == selected_app_package)
+      return i;
+  }
+  return handlers.size();
+}
+
+// A helper function called by GetAction().
+GetActionResult GetActionInternal(
+    const GURL& original_url,
+    const mojom::IntentHandlerInfoPtr& handler,
+    std::pair<GURL, std::string>* out_url_and_package) {
+  if (!handler->fallback_url.is_null()) {
+    *out_url_and_package = std::make_pair(GURL(handler->fallback_url.get()),
+                                          handler->package_name.get());
+    if (ArcIntentHelperBridge::IsIntentHelperPackage(handler->package_name)) {
+      // Since |package_name| is "Chrome", and |fallback_url| is not null, the
+      // URL must be either http or https. Check it just in case, and if not,
+      // fallback to HANDLE_URL_IN_ARC;
+      if (out_url_and_package->first.SchemeIsHTTPOrHTTPS())
+        return GetActionResult::OPEN_URL_IN_CHROME;
+
+      LOG(WARNING) << "Failed to handle " << out_url_and_package->first
+                   << " in Chrome. Falling back to ARC...";
+    }
+    // |fallback_url| which Chrome doesn't support is passed (e.g. market:).
+    return GetActionResult::HANDLE_URL_IN_ARC;
+  }
+
+  // Unlike |handler->fallback_url|, the |original_url| should always be handled
+  // in ARC since it's external to Chrome.
+  *out_url_and_package =
+      std::make_pair(original_url, handler->package_name.get());
+  return GetActionResult::HANDLE_URL_IN_ARC;
+}
+
+// Gets an action that should be done when ARC has the |handlers| for the
+// |original_url| and the user selects |selected_app_index|. When the user
+// hasn't selected any app, |selected_app_index| must be set to
+// |handlers.size()|.
+//
+// When the returned action is either OPEN_URL_IN_CHROME or HANDLE_URL_IN_ARC,
+// |out_url_and_package| is filled accordingly.
+GetActionResult GetAction(
+    const GURL& original_url,
+    const mojo::Array<mojom::IntentHandlerInfoPtr>& handlers,
+    size_t selected_app_index,
+    std::pair<GURL, std::string>* out_url_and_package) {
+  DCHECK(out_url_and_package);
+  if (!handlers.size())
+    return GetActionResult::SHOW_CHROME_OS_DIALOG;  // no apps found.
+
+  if (selected_app_index == handlers.size()) {
+    // The user hasn't made the selection yet.
+
+    // If |handlers| has only one element and its package is "Chrome", open
+    // the fallback URL in the current tab without showing the dialog.
+    if (handlers.size() == 1) {
+      if (GetActionInternal(original_url, handlers[0], out_url_and_package) ==
+          GetActionResult::OPEN_URL_IN_CHROME) {
+        return GetActionResult::OPEN_URL_IN_CHROME;
+      }
+    }
+
+    // If one of the apps is marked as preferred, use it right away without
+    // showing the UI. |is_preferred| will never be true unless the user
+    // explicitly makes it the default with the "always" button.
+    for (size_t i = 0; i < handlers.size(); ++i) {
+      const mojom::IntentHandlerInfoPtr& handler = handlers[i];
+      if (!handler->is_preferred)
+        continue;
+      // A preferred activity is found. Decide how to open it, either in Chrome
+      // or ARC.
+      return GetActionInternal(original_url, handler, out_url_and_package);
+    }
+    // Ask the user to pick one.
+    return GetActionResult::ASK_USER;
+  }
+
+  // The user has already made the selection. Decide how to open it, either in
+  // Chrome or ARC.
+  return GetActionInternal(original_url, handlers[selected_app_index],
+                           out_url_and_package);
+}
+
+// Handles |url| if possible. Returns true if it is actually handled.
+bool HandleUrl(int render_process_host_id,
+               int routing_id,
+               const GURL& url,
+               const mojo::Array<mojom::IntentHandlerInfoPtr>& handlers,
+               size_t selected_app_index,
+               GetActionResult* out_result) {
+  std::pair<GURL, std::string> url_and_package;
+  const GetActionResult result =
+      GetAction(url, handlers, selected_app_index, &url_and_package);
+  if (out_result)
+    *out_result = result;
+
+  switch (result) {
+    case GetActionResult::SHOW_CHROME_OS_DIALOG:
+      ShowFallbackExternalProtocolDialog(render_process_host_id, routing_id,
+                                         url);
+      return true;
+    case GetActionResult::OPEN_URL_IN_CHROME:
+      OpenUrlInChrome(render_process_host_id, routing_id,
+                      url_and_package.first);
+      return true;
+    case GetActionResult::HANDLE_URL_IN_ARC:
+      HandleUrlInArc(render_process_host_id, routing_id, url_and_package);
+      return true;
+    case GetActionResult::ASK_USER:
+      break;
+  }
+  return false;
+}
+
 // Called when the dialog is closed.
 void OnIntentPickerClosed(int render_process_host_id,
                           int routing_id,
@@ -74,7 +232,7 @@ void OnIntentPickerClosed(int render_process_host_id,
                           ArcNavigationThrottle::CloseReason close_reason) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  size_t selected_app_index = handlers.size();
+  const size_t selected_app_index = GetAppIndex(handlers, selected_app_package);
   // Make sure that the instance at least supports HandleUrl.
   auto* instance = ArcIntentHelperBridge::GetIntentHelperInstance(
       "HandleUrl", kMinVersionForHandleUrl);
@@ -84,15 +242,6 @@ void OnIntentPickerClosed(int render_process_host_id,
                  ArcNavigationThrottle::CloseReason::JUST_ONCE_PRESSED ||
              close_reason ==
                  ArcNavigationThrottle::CloseReason::ALWAYS_PRESSED) {
-    // If the user selected an app to continue the navigation, confirm that the
-    // |package_name| matches a valid option and return the index.
-    for (size_t i = 0; i < handlers.size(); ++i) {
-      if (handlers[i]->package_name == selected_app_package) {
-        selected_app_index = i;
-        break;
-      }
-    }
-
     if (selected_app_index == handlers.size())
       close_reason = ArcNavigationThrottle::CloseReason::ERROR;
   }
@@ -108,9 +257,8 @@ void OnIntentPickerClosed(int render_process_host_id,
     }
     case ArcNavigationThrottle::CloseReason::JUST_ONCE_PRESSED: {
       // Launch the selected app.
-      instance->HandleUrl(url.spec(),
-                          handlers[selected_app_index]->package_name);
-      CloseTabIfNeeded(render_process_host_id, routing_id);
+      HandleUrl(render_process_host_id, routing_id, url, handlers,
+                selected_app_index, nullptr);
       break;
     }
     case ArcNavigationThrottle::CloseReason::PREFERRED_ACTIVITY_FOUND:
@@ -175,22 +323,19 @@ void OnUrlHandlerList(int render_process_host_id,
       "HandleUrl", kMinVersionForHandleUrl);
   scoped_refptr<ActivityIconLoader> icon_loader = GetIconLoader();
 
-  if (!instance || !icon_loader || !handlers.size()) {
-    // No handler is available on ARC side. Show the Chrome OS dialog.
+  if (!instance || !icon_loader) {
+    // ARC is not running anymore. Show the Chrome OS dialog.
     ShowFallbackExternalProtocolDialog(render_process_host_id, routing_id, url);
     return;
   }
 
-  // If one of the apps is marked as preferred, use it right away without
-  // showing the UI. |is_preferred| will never be true unless the user
-  // explicitly makes it the default with the "always" button.
-  for (size_t i = 0; i < handlers.size(); ++i) {
-    if (!handlers[i]->is_preferred)
-      continue;
-    instance->HandleUrl(url.spec(), handlers[i]->package_name);
-    CloseTabIfNeeded(render_process_host_id, routing_id);
-    RecordUma(ArcNavigationThrottle::CloseReason::PREFERRED_ACTIVITY_FOUND);
-    return;
+  // Check if the |url| should be handled right away without showing the UI.
+  GetActionResult result;
+  if (HandleUrl(render_process_host_id, routing_id, url, handlers,
+                handlers.size(), &result)) {
+    if (result == GetActionResult::HANDLE_URL_IN_ARC)
+      RecordUma(ArcNavigationThrottle::CloseReason::PREFERRED_ACTIVITY_FOUND);
+    return;  // the |url| has been handled.
   }
 
   // Otherwise, retrieve icons of the activities.
@@ -210,6 +355,9 @@ bool RunArcExternalProtocolDialog(const GURL& url,
                                   int routing_id,
                                   ui::PageTransition page_transition,
                                   bool has_user_gesture) {
+  // This function is for external protocols that Chrome cannot handle.
+  DCHECK(!url.SchemeIsHTTPOrHTTPS()) << url;
+
   // Try to forward <form> submissions to ARC when possible.
   constexpr bool kAllowFormSubmit = true;
   if (ShouldIgnoreNavigation(page_transition, kAllowFormSubmit))
@@ -235,4 +383,19 @@ bool RunArcExternalProtocolDialog(const GURL& url,
   return true;
 }
 
+size_t GetAppIndexForTesting(
+    const mojo::Array<mojom::IntentHandlerInfoPtr>& handlers,
+    const std::string& selected_app_package) {
+  return GetAppIndex(handlers, selected_app_package);
+}
+
+GetActionResult GetActionForTesting(
+    const GURL& original_url,
+    const mojo::Array<mojom::IntentHandlerInfoPtr>& handlers,
+    size_t selected_app_index,
+    std::pair<GURL, std::string>* out_url_and_package) {
+  return GetAction(original_url, handlers, selected_app_index,
+                   out_url_and_package);
+}
+
 }  // namespace arc