Add calls to the server to request WebAPK updates.

chrome/browser/android/webapk/webapk_installer_unittest.cc

Index: chrome/browser/android/webapk/webapk_installer_unittest.cc
diff --git a/chrome/browser/android/webapk/webapk_installer_unittest.cc b/chrome/browser/android/webapk/webapk_installer_unittest.cc
index fee301cd2009653f8a663699455201ce05c0ec9c..6052531b50b4c3c9587f43f3717c5b17b4829222 100644
--- a/chrome/browser/android/webapk/webapk_installer_unittest.cc
+++ b/chrome/browser/android/webapk/webapk_installer_unittest.cc
@@ -36,7 +36,10 @@ const base::FilePath::CharType kTestDataDir[] =
     FILE_PATH_LITERAL("chrome/test/data");
 
 // URL of mock WebAPK server.
-const std::string kServerUrl = "/webapkserver";
+const std::string kServerUrl = "/webapkserver/";
+
+// The response format type expected from the WebAPK server.
+const std::string kWebApkServerUrlResponseType = "?alt=proto";
 
 // URL of file to download from the WebAPK server. We use a random file in the
 // test data directory.
@@ -46,14 +49,22 @@ const std::string kDownloadUrl = "/simple.html";
 const std::string kDownloadedWebApkPackageName = "party.unicode";
 
 // WebApkInstaller subclass where
-// WebApkInstaller::StartDownloadedWebApkInstall() is stubbed out.
+// WebApkInstaller::StartInstallingDownloadedWebApk() is stubbed out.

[pkotwicz, 2016/08/11 22:02:45]

Nit: "WebApkInstaller::StartInstallingDownloadedWebApk() is stubbed out.
WebApkInstaller::StartUpdateUsingDownloadedWebApk() is stubbed out." ->
"WebApkInstaller::StartInstallingDownloadedWebApk() and
WebApkInstaller::StartUpdateUsingDownloadedWebApk() are stubbed out."

[Xi Han, 2016/08/15 21:38:45]

Done.

+// WebApkInstaller::StartUpdateUsingDownloadedWebApk() is stubbed out.
 class TestWebApkInstaller : public WebApkInstaller {
  public:
   TestWebApkInstaller(const ShortcutInfo& shortcut_info,
                       const SkBitmap& shortcut_icon)
       : WebApkInstaller(shortcut_info, shortcut_icon) {}
 
-  bool StartDownloadedWebApkInstall(
+  bool StartInstallingDownloadedWebApk(
+      JNIEnv* env,
+      const base::android::ScopedJavaLocalRef<jstring>& file_path,
+      const base::android::ScopedJavaLocalRef<jstring>& package_name) override {
+    return true;
+  }
+
+  bool StartUpdateUsingDownloadedWebApk(
       JNIEnv* env,
       const base::android::ScopedJavaLocalRef<jstring>& file_path,
       const base::android::ScopedJavaLocalRef<jstring>& package_name) override {
@@ -72,18 +83,41 @@ class WebApkInstallerRunner {
             base::ThreadTaskRunnerHandle::Get())) {}
   ~WebApkInstallerRunner() {}
 
-  void Run() {
-    ShortcutInfo info(GURL::EmptyGURL());
+  void RunCreateWebApk() {
+    WebApkInstaller* installer = createWebApkInstaller();
 
-    // WebApkInstaller owns itself.
-    WebApkInstaller* installer = new TestWebApkInstaller(info, SkBitmap());
-
-    installer->SetTimeoutMs(100);
     installer->InstallAsyncWithURLRequestContextGetter(
         url_request_context_getter_.get(),
         base::Bind(&WebApkInstallerRunner::OnCompleted,
                    base::Unretained(this)));
 
+    setupCallback();
+  }
+
+  void RunUpdateWebApk() {

[pkotwicz, 2016/08/11 22:02:44]

Nit: "int webapk_version" -> "const int kWebApkVersion"

[Xi Han, 2016/08/15 21:38:45]

Done.

+    int webapk_version = 1;
+
+    // WebApkInstaller owns itself.
+    WebApkInstaller* installer = createWebApkInstaller();
+
+    installer->UpdateAsyncWithURLRequestContextGetter(
+        url_request_context_getter_.get(),
+        base::Bind(&WebApkInstallerRunner::OnCompleted, base::Unretained(this)),
+        kDownloadedWebApkPackageName, webapk_version);
+
+    setupCallback();
+  }
+
+  WebApkInstaller* createWebApkInstaller() {

[pkotwicz, 2016/08/11 22:02:44]

Functions start with uppercase in C++

[Xi Han, 2016/08/15 21:38:45]

Done.

+    ShortcutInfo info(GURL::EmptyGURL());
+
+    // WebApkInstaller owns itself.
+    WebApkInstaller* installer = new TestWebApkInstaller(info, SkBitmap());
+    installer->SetTimeoutMs(100);
+    return installer;
+  }
+
+  void setupCallback() {

[pkotwicz, 2016/08/11 22:02:45]

I think that Run() would be a more appropriate name for this function. (This
function blocks till the callback is called)

[Xi Han, 2016/08/15 21:38:45]

Done.

     base::RunLoop run_loop;
     on_completed_callback_ = run_loop.QuitClosure();
     run_loop.Run();
@@ -140,9 +174,8 @@ class WebApkInstallerTest : public ::testing::Test {
 
   void SetUp() override {
     test_server_.AddDefaultHandlers(base::FilePath(kTestDataDir));
-    test_server_.RegisterRequestHandler(
-        base::Bind(&WebApkInstallerTest::HandleCreateWebApkRequest,
-                   base::Unretained(this)));
+    test_server_.RegisterRequestHandler(base::Bind(
+        &WebApkInstallerTest::HandleWebApkRequest, base::Unretained(this)));
     ASSERT_TRUE(test_server_.Start());
 
     SetDefaults();
@@ -173,13 +206,23 @@ class WebApkInstallerTest : public ::testing::Test {
         base::Bind(&BuildValidWebApkResponse, download_url));
   }
 
-  std::unique_ptr<net::test_server::HttpResponse> HandleCreateWebApkRequest(
+  std::unique_ptr<net::test_server::HttpResponse> HandleWebApkRequest(
       const net::test_server::HttpRequest& request) {
-    return (request.relative_url == kServerUrl)
+    return (request.relative_url == GetServerUrlForCreateWebApk() ||
+            request.relative_url == GetServerUrlForUpdateWebApk())
                ? webapk_response_builder_.Run()
                : std::unique_ptr<net::test_server::HttpResponse>();
   }
 
+  std::string GetServerUrlForCreateWebApk() {

[pkotwicz, 2016/08/11 22:02:44]

Nit: This should be a const function

[Xi Han, 2016/08/15 21:38:45]

Done.

+    return kServerUrl + kWebApkServerUrlResponseType;
+  }
+
+  std::string GetServerUrlForUpdateWebApk() {

[pkotwicz, 2016/08/11 22:02:44]

Nit: This should be a const function

[Xi Han, 2016/08/15 21:38:45]

Done.

+    return kServerUrl + kDownloadedWebApkPackageName + "/" +
+           kWebApkServerUrlResponseType;
+  }
+
   content::TestBrowserThreadBundle thread_bundle_;
   net::EmbeddedTestServer test_server_;
 
@@ -192,7 +235,7 @@ class WebApkInstallerTest : public ::testing::Test {
 // Test installation succeeding.
 TEST_F(WebApkInstallerTest, Success) {
   WebApkInstallerRunner runner;
-  runner.Run();
+  runner.RunCreateWebApk();
   EXPECT_TRUE(runner.success());
 }
 
@@ -202,7 +245,7 @@ TEST_F(WebApkInstallerTest, CreateWebApkRequestTimesOut) {
   SetWebApkServerUrl(server_url);
 
   WebApkInstallerRunner runner;
-  runner.Run();
+  runner.RunCreateWebApk();
   EXPECT_FALSE(runner.success());
 }
 
@@ -212,7 +255,7 @@ TEST_F(WebApkInstallerTest, WebApkDownloadTimesOut) {
   SetWebApkResponseBuilder(base::Bind(&BuildValidWebApkResponse, download_url));
 
   WebApkInstallerRunner runner;
-  runner.Run();
+  runner.RunCreateWebApk();
   EXPECT_FALSE(runner.success());
 }
 
@@ -222,7 +265,7 @@ TEST_F(WebApkInstallerTest, WebApkDownloadFails) {
   SetWebApkResponseBuilder(base::Bind(&BuildValidWebApkResponse, download_url));
 
   WebApkInstallerRunner runner;
-  runner.Run();
+  runner.RunCreateWebApk();
   EXPECT_FALSE(runner.success());
 }
 
@@ -246,6 +289,13 @@ TEST_F(WebApkInstallerTest, UnparsableCreateWebApkResponse) {
   SetWebApkResponseBuilder(base::Bind(&BuildUnparsableWebApkResponse));
 
   WebApkInstallerRunner runner;
-  runner.Run();
+  runner.RunCreateWebApk();
   EXPECT_FALSE(runner.success());
 }
+
+// Test update succeeding.
+TEST_F(WebApkInstallerTest, UpdateSuccess) {
+  WebApkInstallerRunner runner;
+  runner.RunUpdateWebApk();
+  EXPECT_TRUE(runner.success());
+}

[pkotwicz, 2016/08/11 22:02:45]

Thanks for adding the test!

[Xi Han, 2016/08/15 21:38:45]

The tests are well written, so it is not difficult to add more:)