Add an extra test for IntentHelperHost::OnOpenWebApp.

This CL adds an additional test for the non-installed URL case, as well as
some extra documentation for profile fetching.

BUG=893927

Change-Id: Icb6d98121f9c4347561a31c9ea97af15b10ebdfb
Reviewed-on: https://chromium-review.googlesource.com/c/1292659Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: David Jacobo <djacobo@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601768}

chrome/browser/ui/ash/chrome_new_window_client.cc

@@ -289,10 +289,16 @@ void ChromeNewWindowClient::OpenUrlFromArc(const GURL& url) {
 void ChromeNewWindowClient::OpenWebAppFromArc(const GURL& url) {
   DCHECK(url.is_valid() && url.SchemeIs(url::kHttpsScheme));
 
+  // Fetch the profile associated with ARC. This method should only be called
+  // for a |url| which was installed via ARC, and so we want the web app that is
+  // opened through here to be installed in the profile associated with ARC.
+  // |user| may be null if sign-in hasn't happened yet
   const auto* user = user_manager::UserManager::Get()->GetPrimaryUser();
   if (!user)
     return;
 
+  // |profile| may be null if sign-in has happened but the profile isn't loaded
+  // yet.
   Profile* profile = chromeos::ProfileHelper::Get()->GetProfileByUser(user);
   if (!profile)
     return;

chrome/browser/ui/ash/chrome_new_window_client_browsertest.cc

@@ -130,16 +130,34 @@ IN_PROC_BROWSER_TEST_F(ChromeNewWindowClientWebAppBrowserTest, OpenWebApp) {
   const char* key =
       arc::ArcWebContentsData::ArcWebContentsData::kArcTransitionFlag;
 
-  // Calling OpenWebAppFromArc for an installed web app URL should open in an
-  // app window.
-  auto observer = GetTestNavigationObserver(app_url);
-  ChromeNewWindowClient::Get()->OpenWebAppFromArc(app_url);
-  observer->WaitForNavigationFinished();
-
-  EXPECT_EQ(2u, chrome::GetTotalBrowserCount());
-  EXPECT_TRUE(GetLastActiveBrowser()->is_app());
-  content::WebContents* contents =
-      GetLastActiveBrowser()->tab_strip_model()->GetActiveWebContents();
-  EXPECT_EQ(app_url, contents->GetLastCommittedURL());
-  EXPECT_NE(nullptr, contents->GetUserData(key));
+  {
+    // Calling OpenWebAppFromArc for a not installed HTTPS URL should open in
+    // an ordinary browser tab.
+    const GURL url("https://www.google.com");
+    auto observer = GetTestNavigationObserver(url);
+    ChromeNewWindowClient::Get()->OpenWebAppFromArc(url);
+    observer->WaitForNavigationFinished();
+
+    EXPECT_EQ(1u, chrome::GetTotalBrowserCount());
+    EXPECT_FALSE(GetLastActiveBrowser()->is_app());
+    content::WebContents* contents =
+        GetLastActiveBrowser()->tab_strip_model()->GetActiveWebContents();
+    EXPECT_EQ(url, contents->GetLastCommittedURL());
+    EXPECT_NE(nullptr, contents->GetUserData(key));
+  }
+
+  {
+    // Calling OpenWebAppFromArc for an installed web app URL should open in an
+    // app window.
+    auto observer = GetTestNavigationObserver(app_url);
+    ChromeNewWindowClient::Get()->OpenWebAppFromArc(app_url);
+    observer->WaitForNavigationFinished();
+
+    EXPECT_EQ(2u, chrome::GetTotalBrowserCount());
+    EXPECT_TRUE(GetLastActiveBrowser()->is_app());
+    content::WebContents* contents =
+        GetLastActiveBrowser()->tab_strip_model()->GetActiveWebContents();
+    EXPECT_EQ(app_url, contents->GetLastCommittedURL());
+    EXPECT_NE(nullptr, contents->GetUserData(key));
+  }
 }

components/arc/intent_helper/open_url_delegate.h

@@ -16,7 +16,8 @@ class OpenUrlDelegate {
   // Opens the given URL in the Chrome browser.
   virtual void OpenUrlFromArc(const GURL& url) = 0;
 
-  // Opens the given URL as a web app in the Chrome browser.
+  // Opens the given URL as a web app in the Chrome browser, falling back to
+  // opening as a tab if no installed web app is found.
   virtual void OpenWebAppFromArc(const GURL& url) = 0;
 };