Refactor LoadWebAppAndCheckInstallability into web_app_install_manager.

Moves LoadWebAppAndCheckInstallability from being the only non-virtual method
defined in the parent InstallManager class to exist alongside other similar
methods in web_app_install_manager child class.
This also allows it to use the web_app_install_task to run the async parts and
use some shared machinery.
This is a step towards unifying and de-duplicating various web app loading,
checking, and installing steps.

In prep for changing behaviour in http://crrev.com/c/1847609

Bug: 1007860
Change-Id: Ib18356719fc0f321cd6bd100d2ce6ec0a450ee0e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1873817
Commit-Queue: Glen Robertson <glenrob@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711098}

chrome/browser/installable/fake_installable_manager.cc

@@ -49,10 +49,10 @@ FakeInstallableManager::CreateForWebContentsWithManifest(
   FakeInstallableManager* installable_manager =
       FakeInstallableManager::CreateForWebContents(web_contents);
 
+  const bool valid_manifest = manifest && !manifest->IsEmpty();
   installable_manager->manifest_url_ = manifest_url;
   installable_manager->manifest_ = std::move(manifest);
 
-  const bool valid_manifest = true;
   const bool has_worker = true;
   std::vector<InstallableStatusCode> errors;
 

chrome/browser/web_applications/components/install_manager.cc

@@ -4,44 +4,10 @@
 
 #include "chrome/browser/web_applications/components/install_manager.h"
 
-#include "chrome/browser/installable/installable_manager.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ssl/security_state_tab_helper.h"
-#include "content/public/browser/web_contents.h"
 
 namespace web_app {
 
-namespace {
-
-void OnInstallabilityCheckCompleted(
-    InstallManager::WebAppInstallabilityCheckCallback callback,
-    std::unique_ptr<content::WebContents> web_contents,
-    const InstallableData& data) {
-  std::move(callback).Run(std::move(web_contents), data.errors.empty());
-}
-
-void OnWebAppUrlLoaded(
-    InstallManager::WebAppInstallabilityCheckCallback callback,
-    std::unique_ptr<content::WebContents> web_contents,
-    WebAppUrlLoader::Result result) {
-  if (result != WebAppUrlLoader::Result::kUrlLoaded) {
-    std::move(callback).Run(std::move(web_contents), false);
-    return;
-  }
-
-  InstallableParams params;
-  params.valid_primary_icon = true;
-  params.valid_manifest = true;
-  params.has_worker = true;
-  params.wait_for_worker = true;
-  InstallableManager::FromWebContents(web_contents.get())
-      ->GetData(params,
-                base::BindOnce(&OnInstallabilityCheckCompleted,
-                               std::move(callback), std::move(web_contents)));
-}
-
-}  // namespace
-
 InstallManager::InstallManager(Profile* profile) : profile_(profile) {}
 
 InstallManager::~InstallManager() = default;
@@ -54,21 +20,4 @@ void InstallManager::SetSubsystems(AppRegistrar* registrar,
   finalizer_ = finalizer;
 }
 
-void InstallManager::LoadWebAppAndCheckInstallability(
-    const GURL& web_app_url,
-    WebAppInstallabilityCheckCallback callback) {
-  std::unique_ptr<content::WebContents> web_contents =
-      content::WebContents::Create(
-          content::WebContents::CreateParams(profile()));
-  InstallableManager::CreateForWebContents(web_contents.get());
-  SecurityStateTabHelper::CreateForWebContents(web_contents.get());
-
-  // Grab WebContents pointer now, before the call to BindOnce might null out
-  // |web_contents|.
-  content::WebContents* web_contents_ptr = web_contents.get();
-  url_loader_.LoadUrl(web_app_url, web_contents_ptr,
-                      base::BindOnce(&OnWebAppUrlLoaded, std::move(callback),
-                                     std::move(web_contents)));
-}
-
 }  // namespace web_app

chrome/browser/web_applications/components/install_manager.h

@@ -34,6 +34,7 @@ class AppShortcutManager;
 // InstallWebAppZZZZ functions.
 class InstallManager {
  public:
+  // |app_id| may be empty on failure.
   using OnceInstallCallback =
       base::OnceCallback<void(const AppId& app_id, InstallResultCode code)>;
 
@@ -132,10 +133,12 @@ class InstallManager {
                      InstallFinalizer* finalizer);
 
   // Loads |web_app_url| in a new WebContents and determines if it is
-  // installable. Returns the WebContents and whether the app is installable or
-  // not.
-  void LoadWebAppAndCheckInstallability(const GURL& web_app_url,
-                                        WebAppInstallabilityCheckCallback);
+  // installable.
+  // Calls back with a unique_ptr owning the WebContents used to check
+  // installability, and whether the app is installable.
+  virtual void LoadWebAppAndCheckInstallability(
+      const GURL& web_app_url,
+      WebAppInstallabilityCheckCallback) = 0;
 
  protected:
   Profile* profile() { return profile_; }

chrome/browser/web_applications/components/web_app_data_retriever.cc

@@ -168,6 +168,8 @@ void WebAppDataRetriever::OnDidPerformInstallableCheck(
   DCHECK(data.manifest_url.is_valid() || data.manifest->IsEmpty());
 
   const bool is_installable = data.errors.empty();
+  DCHECK(!is_installable || data.valid_manifest);
+  DCHECK(!data.valid_manifest || !data.manifest->IsEmpty());
 
   std::move(check_installability_callback_)
       .Run(*data.manifest, data.valid_manifest, is_installable);
@@ -191,7 +193,7 @@ void WebAppDataRetriever::CallCallbackOnError() {
     std::move(get_web_app_info_callback_).Run(nullptr);
   } else if (check_installability_callback_) {
     std::move(check_installability_callback_)
-        .Run(blink::Manifest{}, /*valid_manifest_for_web_app=*/false,
+        .Run(base::nullopt, /*valid_manifest_for_web_app=*/false,
              /*is_installable=*/false);
   } else if (get_icons_callback_) {
     std::move(get_icons_callback_).Run(IconsMap{});

chrome/browser/web_applications/components/web_app_data_retriever.h

@@ -12,6 +12,7 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/optional.h"
 #include "chrome/browser/web_applications/components/web_app_icon_downloader.h"
 #include "chrome/browser/web_applications/components/web_app_install_utils.h"
 #include "chrome/common/chrome_render_frame.mojom.h"
@@ -39,9 +40,11 @@ class WebAppDataRetriever : content::WebContentsObserver {
   // Returns nullptr for WebApplicationInfo if error.
   using GetWebApplicationInfoCallback =
       base::OnceCallback<void(std::unique_ptr<WebApplicationInfo>)>;
-  // |is_installable| is false if installability check failed.
+  // |is_installable| represents installability check result.
+  // If |is_installable| then |valid_manifest_for_web_app| is true.
+  // If |valid_manifest_for_web_app| then manifest is present and non-empty.
   using CheckInstallabilityCallback =
-      base::OnceCallback<void(const blink::Manifest&,
+      base::OnceCallback<void(base::Optional<blink::Manifest> manifest,
                               bool valid_manifest_for_web_app,
                               bool is_installable)>;
   // Returns empty map if error.

chrome/browser/web_applications/components/web_app_data_retriever_unittest.cc

@@ -304,10 +304,10 @@ TEST_F(WebAppDataRetrieverTest,
   WebAppDataRetriever retriever;
   retriever.CheckInstallabilityAndRetrieveManifest(
       web_contents(), /*bypass_service_worker_check=*/false,
-      base::BindLambdaForTesting([&](const blink::Manifest& manifet,
+      base::BindLambdaForTesting([&](base::Optional<blink::Manifest> manifest,
                                      bool valid_manifest_for_web_app,
                                      bool is_installable) {
-        EXPECT_TRUE(manifet.IsEmpty());
+        EXPECT_FALSE(manifest);
         EXPECT_FALSE(valid_manifest_for_web_app);
         EXPECT_FALSE(is_installable);
         run_loop.Quit();
@@ -382,17 +382,17 @@ TEST_F(WebAppDataRetrieverTest, CheckInstallabilityAndRetrieveManifest) {
 
   retriever.CheckInstallabilityAndRetrieveManifest(
       web_contents(), /*bypass_service_worker_check=*/false,
-      base::BindLambdaForTesting(
-          [&](const blink::Manifest& result, bool valid_manifest_for_web_app,
+      base::BindLambdaForTesting([&](base::Optional<blink::Manifest> result,
+                                     bool valid_manifest_for_web_app,
                                      bool is_installable) {
         EXPECT_TRUE(is_installable);
 
         EXPECT_EQ(base::UTF8ToUTF16(manifest_short_name),
-                      result.short_name.string());
-            EXPECT_EQ(base::UTF8ToUTF16(manifest_name), result.name.string());
-            EXPECT_EQ(manifest_start_url, result.start_url);
-            EXPECT_EQ(manifest_scope, result.scope);
-            EXPECT_EQ(manifest_theme_color, result.theme_color);
+                  result->short_name.string());
+        EXPECT_EQ(base::UTF8ToUTF16(manifest_name), result->name.string());
+        EXPECT_EQ(manifest_start_url, result->start_url);
+        EXPECT_EQ(manifest_scope, result->scope);
+        EXPECT_EQ(manifest_theme_color, result->theme_color);
 
         callback_called = true;
         run_loop.Quit();
@@ -418,10 +418,11 @@ TEST_F(WebAppDataRetrieverTest, CheckInstallabilityFails) {
 
   retriever.CheckInstallabilityAndRetrieveManifest(
       web_contents(), /*bypass_service_worker_check=*/false,
-      base::BindLambdaForTesting([&](const blink::Manifest& result,
+      base::BindLambdaForTesting([&](base::Optional<blink::Manifest> result,
                                      bool valid_manifest_for_web_app,
                                      bool is_installable) {
         EXPECT_FALSE(is_installable);
+        EXPECT_FALSE(valid_manifest_for_web_app);
         callback_called = true;
         run_loop.Quit();
       }));

chrome/browser/web_applications/web_app_install_manager.cc

@@ -40,6 +40,22 @@ bool WebAppInstallManager::CanInstallWebApp(
          IsValidWebAppUrl(web_contents->GetLastCommittedURL());
 }
 
+void WebAppInstallManager::LoadWebAppAndCheckInstallability(
+    const GURL& web_app_url,
+    WebAppInstallabilityCheckCallback callback) {
+  auto task = std::make_unique<WebAppInstallTask>(
+      profile(), shortcut_manager(), finalizer(),
+      data_retriever_factory_.Run());
+
+  task->LoadWebAppAndCheckInstallability(
+      web_app_url, WebappInstallSource::MANAGEMENT_API, url_loader_.get(),
+      base::BindOnce(
+          &WebAppInstallManager::OnLoadWebAppAndCheckInstallabilityCompleted,
+          base::Unretained(this), task.get(), std::move(callback)));
+
+  tasks_.insert(std::move(task));
+}
+
 void WebAppInstallManager::InstallWebAppFromManifest(
     content::WebContents* contents,
     WebappInstallSource install_source,
@@ -50,7 +66,7 @@ void WebAppInstallManager::InstallWebAppFromManifest(
       data_retriever_factory_.Run());
   task->InstallWebAppFromManifest(
       contents, install_source, std::move(dialog_callback),
-      base::BindOnce(&WebAppInstallManager::OnTaskCompleted,
+      base::BindOnce(&WebAppInstallManager::OnInstallTaskCompleted,
                      base::Unretained(this), task.get(), std::move(callback)));
 
   tasks_.insert(std::move(task));
@@ -67,7 +83,7 @@ void WebAppInstallManager::InstallWebAppFromManifestWithFallback(
       data_retriever_factory_.Run());
   task->InstallWebAppFromManifestWithFallback(
       contents, force_shortcut_app, install_source, std::move(dialog_callback),
-      base::BindOnce(&WebAppInstallManager::OnTaskCompleted,
+      base::BindOnce(&WebAppInstallManager::OnInstallTaskCompleted,
                      base::Unretained(this), task.get(), std::move(callback)));
 
   tasks_.insert(std::move(task));
@@ -83,7 +99,7 @@ void WebAppInstallManager::InstallWebAppFromInfo(
       data_retriever_factory_.Run());
   task->InstallWebAppFromInfo(
       std::move(web_application_info), for_installable_site, install_source,
-      base::BindOnce(&WebAppInstallManager::OnTaskCompleted,
+      base::BindOnce(&WebAppInstallManager::OnInstallTaskCompleted,
                      base::Unretained(this), task.get(), std::move(callback)));
 
   tasks_.insert(std::move(task));
@@ -99,7 +115,7 @@ void WebAppInstallManager::InstallWebAppWithParams(
       data_retriever_factory_.Run());
   task->InstallWebAppWithParams(
       web_contents, install_params, install_source,
-      base::BindOnce(&WebAppInstallManager::OnTaskCompleted,
+      base::BindOnce(&WebAppInstallManager::OnInstallTaskCompleted,
                      base::Unretained(this), task.get(), std::move(callback)));
 
   tasks_.insert(std::move(task));
@@ -236,12 +252,16 @@ void WebAppInstallManager::SetDataRetrieverFactoryForTesting(
   data_retriever_factory_ = std::move(data_retriever_factory);
 }
 
-void WebAppInstallManager::OnTaskCompleted(WebAppInstallTask* task,
+void WebAppInstallManager::DeleteTask(WebAppInstallTask* task) {
+  DCHECK(tasks_.contains(task));
+  tasks_.erase(task);
+}
+
+void WebAppInstallManager::OnInstallTaskCompleted(WebAppInstallTask* task,
                                                   OnceInstallCallback callback,
                                                   const AppId& app_id,
                                                   InstallResultCode code) {
-  DCHECK(tasks_.contains(task));
-  tasks_.erase(task);
+  DeleteTask(task);
 
   std::move(callback).Run(app_id, code);
 }
@@ -253,7 +273,7 @@ void WebAppInstallManager::OnQueuedTaskCompleted(WebAppInstallTask* task,
   DCHECK(is_running_queued_task_);
   is_running_queued_task_ = false;
 
-  OnTaskCompleted(task, std::move(callback), app_id, code);
+  OnInstallTaskCompleted(task, std::move(callback), app_id, code);
   task = nullptr;
 
   if (task_queue_.empty()) {
@@ -264,6 +284,17 @@ void WebAppInstallManager::OnQueuedTaskCompleted(WebAppInstallTask* task,
   }
 }
 
+void WebAppInstallManager::OnLoadWebAppAndCheckInstallabilityCompleted(
+    WebAppInstallTask* task,
+    WebAppInstallabilityCheckCallback callback,
+    std::unique_ptr<content::WebContents> web_contents,
+    const AppId& app_id,
+    InstallResultCode code) {
+  DeleteTask(task);
+
+  std::move(callback).Run(std::move(web_contents), IsSuccess(code));
+}
+
 void WebAppInstallManager::OnWebAppInstalledAfterSync(
     const AppId& app_in_sync_install_id,
     OnceInstallCallback callback,

chrome/browser/web_applications/web_app_install_manager.h

@@ -38,6 +38,9 @@ class WebAppInstallManager final : public InstallManager,
 
   // InstallManager:
   bool CanInstallWebApp(content::WebContents* web_contents) override;
+  void LoadWebAppAndCheckInstallability(
+      const GURL& web_app_url,
+      WebAppInstallabilityCheckCallback callback) override;
   void InstallWebAppFromManifest(content::WebContents* contents,
                                  WebappInstallSource install_source,
                                  WebAppInstallDialogCallback dialog_callback,
@@ -86,7 +89,9 @@ class WebAppInstallManager final : public InstallManager,
   void EnqueueTask(std::unique_ptr<WebAppInstallTask> task,
                    base::OnceClosure start_task);
   void MaybeStartQueuedTask();
-  void OnTaskCompleted(WebAppInstallTask* task,
+
+  void DeleteTask(WebAppInstallTask* task);
+  void OnInstallTaskCompleted(WebAppInstallTask* task,
                               OnceInstallCallback callback,
                               const AppId& app_id,
                               InstallResultCode code);
@@ -100,6 +105,13 @@ class WebAppInstallManager final : public InstallManager,
                                   const AppId& installed_app_id,
                                   InstallResultCode code);
 
+  void OnLoadWebAppAndCheckInstallabilityCompleted(
+      WebAppInstallTask* task,
+      WebAppInstallabilityCheckCallback callback,
+      std::unique_ptr<content::WebContents> web_contents,
+      const AppId& app_id,
+      InstallResultCode code);
+
   content::WebContents* EnsureWebContentsCreated();
   void OnWebContentsReady(WebAppUrlLoader::Result result);
 

chrome/browser/web_applications/web_app_install_task.cc

@@ -9,8 +9,10 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/logging.h"
+#include "chrome/browser/installable/installable_manager.h"
 #include "chrome/browser/installable/installable_metrics.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ssl/security_state_tab_helper.h"
 #include "chrome/browser/web_applications/components/app_shortcut_manager.h"
 #include "chrome/browser/web_applications/components/install_bounce_metric.h"
 #include "chrome/browser/web_applications/components/install_finalizer.h"
@@ -75,6 +77,40 @@ void WebAppInstallTask::ExpectAppId(const AppId& expected_app_id) {
   expected_app_id_ = expected_app_id;
 }
 
+void WebAppInstallTask::LoadWebAppAndCheckInstallability(
+    const GURL& url,
+    WebappInstallSource install_source,
+    WebAppUrlLoader* url_loader,
+    LoadWebAppAndCheckInstallabilityCallback callback) {
+  DCHECK(url_loader);
+  // Create a WebContents instead of reusing a shared one because we will pass
+  // it back to be used for opening the web app.
+  // TODO(loyso): Implement stealing of shared web_contents in upcoming
+  // WebContentsManager.
+  std::unique_ptr<content::WebContents> web_contents =
+      content::WebContents::Create(
+          content::WebContents::CreateParams(profile_));
+  InstallableManager::CreateForWebContents(web_contents.get());
+  SecurityStateTabHelper::CreateForWebContents(web_contents.get());
+
+  // Grab WebContents pointer now, before the call to BindOnce might null out
+  // |web_contents|.
+  content::WebContents* web_contents_ptr = web_contents.get();
+
+  Observe(web_contents.get());
+  background_installation_ = false;
+  install_callback_ =
+      base::BindOnce(std::move(callback), std::move(web_contents));
+  install_source_ = install_source;
+
+  url_loader->LoadUrl(
+      url, web_contents_ptr,
+      base::BindOnce(
+          &WebAppInstallTask::
+              OnWebAppUrlLoadedCheckInstallabilityAndRetrieveManifest,
+          base::Unretained(this), web_contents_ptr));
+}
+
 void WebAppInstallTask::InstallWebAppFromManifest(
     content::WebContents* contents,
     WebappInstallSource install_source,
@@ -131,8 +167,9 @@ void WebAppInstallTask::LoadAndInstallWebAppFromManifestWithFallback(
   install_callback_ = std::move(install_callback);
   install_source_ = install_source;
 
-  url_loader->LoadUrl(launch_url, contents,
-                      base::BindOnce(&WebAppInstallTask::OnWebAppUrlLoaded,
+  url_loader->LoadUrl(
+      launch_url, contents,
+      base::BindOnce(&WebAppInstallTask::OnWebAppUrlLoadedGetWebApplicationInfo,
                      weak_ptr_factory_.GetWeakPtr()));
 }
 
@@ -271,7 +308,8 @@ bool WebAppInstallTask::ShouldStopInstall() const {
   return !web_contents() || web_contents()->IsBeingDestroyed();
 }
 
-void WebAppInstallTask::OnWebAppUrlLoaded(WebAppUrlLoader::Result result) {
+void WebAppInstallTask::OnWebAppUrlLoadedGetWebApplicationInfo(
+    WebAppUrlLoader::Result result) {
   if (ShouldStopInstall())
     return;
 
@@ -291,6 +329,46 @@ void WebAppInstallTask::OnWebAppUrlLoaded(WebAppUrlLoader::Result result) {
                      base::Unretained(this), /*force_shortcut_app*/ false));
 }
 
+void WebAppInstallTask::OnWebAppUrlLoadedCheckInstallabilityAndRetrieveManifest(
+    content::WebContents* web_contents,
+    WebAppUrlLoader::Result result) {
+  if (ShouldStopInstall())
+    return;
+
+  if (result == WebAppUrlLoader::Result::kRedirectedUrlLoaded) {
+    CallInstallCallback(AppId(), InstallResultCode::kInstallURLRedirected);
+    return;
+  }
+  if (result != WebAppUrlLoader::Result::kUrlLoaded) {
+    CallInstallCallback(AppId(), InstallResultCode::kInstallURLLoadFailed);
+    return;
+  }
+
+  data_retriever_->CheckInstallabilityAndRetrieveManifest(
+      web_contents,
+      /*bypass_service_worker_check=*/false,
+      base::BindOnce(&WebAppInstallTask::OnWebAppInstallabilityChecked,
+                     base::Unretained(this)));
+}
+
+void WebAppInstallTask::OnWebAppInstallabilityChecked(
+    base::Optional<blink::Manifest> manifest,
+    bool valid_manifest_for_web_app,
+    bool is_installable) {
+  if (ShouldStopInstall())
+    return;
+
+  if (is_installable) {
+    DCHECK(manifest);
+    // TODO(loyso): Consider generalizing this class slightly to avoid abusing
+    // kSuccessNewInstall and kFailedUnknownReason to indicate installability.
+    CallInstallCallback(GenerateAppIdFromURL(manifest->start_url),
+                        InstallResultCode::kSuccessNewInstall);
+  } else {
+    CallInstallCallback(AppId(), InstallResultCode::kFailedUnknownReason);
+  }
+}
+
 void WebAppInstallTask::OnGetWebApplicationInfo(
     bool force_shortcut_app,
     std::unique_ptr<WebApplicationInfo> web_app_info) {
@@ -318,7 +396,7 @@ void WebAppInstallTask::OnGetWebApplicationInfo(
 void WebAppInstallTask::OnDidPerformInstallableCheck(
     std::unique_ptr<WebApplicationInfo> web_app_info,
     bool force_shortcut_app,
-    const blink::Manifest& manifest,
+    base::Optional<blink::Manifest> opt_manifest,
     bool valid_manifest_for_web_app,
     bool is_installable) {
   if (ShouldStopInstall())
@@ -338,6 +416,7 @@ void WebAppInstallTask::OnDidPerformInstallableCheck(
                                         ? ForInstallableSite::kYes
                                         : ForInstallableSite::kNo;
 
+  const blink::Manifest manifest = opt_manifest.value_or(blink::Manifest{});
   UpdateWebAppInfoFromManifest(manifest, web_app_info.get(),
                                for_installable_site);
 

chrome/browser/web_applications/web_app_install_task.h

@@ -50,6 +50,17 @@ class WebAppInstallTask : content::WebContentsObserver {
   // The actual resulting app_id is reported as a part of OnceInstallCallback.
   void ExpectAppId(const AppId& expected_app_id);
 
+  using LoadWebAppAndCheckInstallabilityCallback = base::OnceCallback<void(
+      std::unique_ptr<content::WebContents> web_contents,
+      const AppId& app_id,
+      InstallResultCode code)>;
+  // Load a web app from the given URL and check installability.
+  void LoadWebAppAndCheckInstallability(
+      const GURL& url,
+      WebappInstallSource install_source,
+      WebAppUrlLoader* url_loader,
+      LoadWebAppAndCheckInstallabilityCallback callback);
+
   // Checks a WebApp installability, retrieves manifest and icons and
   // then performs the actual installation.
   void InstallWebAppFromManifest(
@@ -134,14 +145,23 @@ class WebAppInstallTask : content::WebContentsObserver {
   // install_callback_.
   bool ShouldStopInstall() const;
 
-  void OnWebAppUrlLoaded(WebAppUrlLoader::Result result);
+  void OnWebAppUrlLoadedGetWebApplicationInfo(WebAppUrlLoader::Result result);
+
+  void OnWebAppUrlLoadedCheckInstallabilityAndRetrieveManifest(
+      content::WebContents* web_contents,
+      WebAppUrlLoader::Result result);
+  void OnWebAppInstallabilityChecked(
+      base::Optional<blink::Manifest> opt_manifest,
+      bool valid_manifest_for_web_app,
+      bool is_installable);
+
   void OnGetWebApplicationInfo(
       bool force_shortcut_app,
       std::unique_ptr<WebApplicationInfo> web_app_info);
   void OnDidPerformInstallableCheck(
       std::unique_ptr<WebApplicationInfo> web_app_info,
       bool force_shortcut_app,
-      const blink::Manifest& manifest,
+      base::Optional<blink::Manifest> opt_manifest,
       bool valid_manifest_for_web_app,
       bool is_installable);