Add more tests to the AppControllerService.

This second batch of tests cover the LaunchHomeUrl method.

One small change was made to the AppControllerService to make the tests
simple: We no longer read the URL prefix during initialization, instead
we do it on demand when trying to launch home URLs. This makes it easier
to override the prefix when testing the same service instance.

Bug: 927809
Change-Id: I5460b055f6d423cd196b3c5b4b06d723e6abfd67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1558411Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Commit-Queue: Lucas Tenório <ltenorio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#649300}

chrome/browser/chromeos/kiosk_next_home/app_controller_service.cc

@@ -43,9 +43,7 @@ AppControllerService* AppControllerService::Get(
 
 AppControllerService::AppControllerService(Profile* profile)
     : profile_(profile),
-      app_service_proxy_(apps::AppServiceProxy::Get(profile)),
-      url_prefix_(base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-          chromeos::switches::kKioskNextHomeUrlPrefix)) {
+      app_service_proxy_(apps::AppServiceProxy::Get(profile)) {
   DCHECK(profile);
   app_service_proxy_->AppRegistryCache().AddObserver(this);
 
@@ -109,12 +107,16 @@ void AppControllerService::GetArcAndroidId(
 
 void AppControllerService::LaunchHomeUrl(const std::string& suffix,
                                          LaunchHomeUrlCallback callback) {
-  if (url_prefix_.empty()) {
+  std::string url_prefix =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          chromeos::switches::kKioskNextHomeUrlPrefix);
+
+  if (url_prefix.empty()) {
     std::move(callback).Run(false, "No URL prefix.");
     return;
   }
 
-  GURL url(url_prefix_ + suffix);
+  GURL url(url_prefix + suffix);
   if (!url.is_valid()) {
     std::move(callback).Run(false, "Invalid URL.");
     return;

chrome/browser/chromeos/kiosk_next_home/app_controller_service.h

@@ -86,9 +86,6 @@ class AppControllerService : public mojom::AppController,
   // is necessary.
   std::map<std::string, std::string> android_package_map_;
 
-  // URL prefix that can be launched by Kiosk Next Home.
-  const std::string url_prefix_;
-
   DISALLOW_COPY_AND_ASSIGN(AppControllerService);
 };
 

chrome/browser/chromeos/kiosk_next_home/app_controller_service_unittest.cc

@@ -10,6 +10,8 @@
 #include <utility>
 #include <vector>
 
+#include "base/command_line.h"
+#include "base/optional.h"
 #include "base/test/bind_test_util.h"
 #include "chrome/browser/apps/app_service/app_service_proxy.h"
 #include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
@@ -19,6 +21,7 @@
 #include "chrome/services/app_service/public/cpp/app_registry_cache.h"
 #include "chrome/services/app_service/public/cpp/app_update.h"
 #include "chrome/test/base/testing_profile.h"
+#include "chromeos/constants/chromeos_switches.h"
 #include "components/arc/test/fake_app_instance.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -53,6 +56,8 @@ class AppControllerServiceTest : public testing::Test {
 
   Profile* profile() { return profile_.get(); }
 
+  AppControllerService* service() { return app_controller_service_; }
+
   std::string GetAppIdFromAndroidPackage(const std::string& package) {
     return ArcAppListPrefs::GetAppId(package, kFakeActivity);
   }
@@ -76,6 +81,11 @@ class AppControllerServiceTest : public testing::Test {
 
   void StopArc() { arc_test_.StopArcInstance(); }
 
+  void SetHomeUrlPrefix(const std::string& url_prefix) {
+    base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+        chromeos::switches::kKioskNextHomeUrlPrefix, url_prefix);
+  }
+
   void AddAppDeltaToAppService(apps::mojom::AppPtr delta) {
     std::vector<apps::mojom::AppPtr> deltas;
     deltas.push_back(std::move(delta));
@@ -143,6 +153,50 @@ class AppControllerServiceTest : public testing::Test {
     EXPECT_EQ(returned_android_id, android_id);
   }
 
+  void ExpectLaunchHomeUrlResponse(
+      const std::string& url_to_launch,
+      bool success,
+      const base::Optional<std::string>& error_message) {
+    bool returned_success;
+    base::Optional<std::string> returned_error_message;
+
+    service()->LaunchHomeUrl(
+        url_to_launch,
+        base::BindLambdaForTesting(
+            [&returned_success, &returned_error_message](
+                bool success,
+                const base::Optional<std::string>& error_message) {
+              returned_success = success;
+              returned_error_message = error_message;
+            }));
+
+    EXPECT_EQ(returned_success, success);
+
+    // We first check if the optionals have the same value state.
+    // Then we check their values. This is necessary to get more readable
+    // failure messages when the returned values are different, if we simply
+    // compare two different optionals we get:
+    // Expected equality of these values:
+    // returned_error_message
+    //   Which is: 32-byte object <01-00 ... 00-00>
+    // error_message
+    //   Which is: 32-byte object <01-2E ... 00-00>
+    ASSERT_EQ(returned_error_message.has_value(), error_message.has_value());
+    if (returned_error_message.has_value())
+      EXPECT_EQ(returned_error_message.value(), error_message.value());
+  }
+
+  void ExpectNoLaunchedHomeUrls() {
+    EXPECT_EQ(0U, arc_test_.app_instance()->launch_intents().size())
+        << "At least one ARC intent was lauched, we expected none.";
+  }
+
+  void ExpectHomeUrlLaunched(const std::string& launched_url) {
+    ASSERT_EQ(arc_test_.app_instance()->launch_intents().size(), 1U)
+        << "We expect exactly one ARC intent to be launched.";
+    EXPECT_EQ(arc_test_.app_instance()->launch_intents()[0], launched_url);
+  }
+
  private:
   content::TestBrowserThreadBundle test_browser_thread_bundle_;
   std::unique_ptr<TestingProfile> profile_;
@@ -364,5 +418,33 @@ TEST_F(AppControllerServiceTest, GetArcAndroidIdFailureIsPropagated) {
   ExpectArcAndroidIdResponse(false, "0");
 }
 
+TEST_F(AppControllerServiceTest, LaunchHomeUrlFailsWhenWeDontHaveUrlPrefix) {
+  base::Optional<std::string> error_message("No URL prefix.");
+  ExpectLaunchHomeUrlResponse("http://example.com", false, error_message);
+  ExpectNoLaunchedHomeUrls();
+}
+
+TEST_F(AppControllerServiceTest, LaunchHomeUrlFailsWhenArcIsDisabled) {
+  SetHomeUrlPrefix("https://example.com/?q=");
+  StopArc();
+  base::Optional<std::string> error_message("ARC bridge not available.");
+  ExpectLaunchHomeUrlResponse("example_query", false, error_message);
+  ExpectNoLaunchedHomeUrls();
+}
+
+TEST_F(AppControllerServiceTest, LaunchHomeUrlFailsWhenUrlIsInvalid) {
+  SetHomeUrlPrefix("invalid_url_prefix_example");
+  base::Optional<std::string> error_message("Invalid URL.");
+  ExpectLaunchHomeUrlResponse("invalid_query", false, error_message);
+  ExpectNoLaunchedHomeUrls();
+}
+
+TEST_F(AppControllerServiceTest, LaunchHomeUrlLaunchesWhenWeHaveAValidPrefix) {
+  SetHomeUrlPrefix("https://example.com/?q=");
+  ExpectLaunchHomeUrlResponse("example_query", true, base::nullopt);
+
+  ExpectHomeUrlLaunched("https://example.com/?q=example_query");
+}
+
 }  // namespace kiosk_next_home
 }  // namespace chromeos