DevTools: Modify Page.getInstallabilityErrors to use structured data

This change modifies the shape of Page.getInstallabilityErrors [1] CDP
event to include the error ids and arguments that are needed to generate
the detail messages that are displayed in the Applications - Manifest
tool. The design is proposed in Localizability of CDP [2].

The strings that are currently present in installable_logging.cc will be
moved to the DevTools frontend. They'll remain in place so we are backwards
compatible. Separate CLs will be made to process the new data in the
DevTools frontend.


[1]: https://chromedevtools.github.io/devtools-protocol/tot/Page#method-getInstallabilityErrors
[2]: https://docs.google.com/document/d/1ZbCiXbVJZJaYqKZkQ_Y5RNpUqQd1JzO17PqFJZk2Ne8/edit?usp=sharing

Bug: 946860
Change-Id: I8af923c1554396c6d79a4233bf342357b3386c7f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2008430
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Lorne Mitchell <lomitch@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#734336}

chrome/browser/devtools/protocol/page_handler.cc

@@ -68,9 +68,30 @@ void PageHandler::GetInstallabilityErrors(
 // static
 void PageHandler::GotInstallabilityErrors(
     std::unique_ptr<GetInstallabilityErrorsCallback> callback,
-    std::vector<std::string> errors) {
+    std::vector<std::string> errors,
+    std::vector<content::InstallabilityError> installability_errors) {
+  auto result_installability_errors =
+      std::make_unique<protocol::Array<protocol::Page::InstallabilityError>>();
+  for (const auto& installability_error : installability_errors) {
+    auto installability_error_arguments = std::make_unique<
+        protocol::Array<protocol::Page::InstallabilityErrorArgument>>();
+    for (const auto& error_argument :
+         installability_error.installability_error_arguments) {
+      installability_error_arguments->emplace_back(
+          protocol::Page::InstallabilityErrorArgument::Create()
+              .SetName(error_argument.name)
+              .SetValue(error_argument.value)
+              .Build());
+    }
+    result_installability_errors->emplace_back(
+        protocol::Page::InstallabilityError::Create()
+            .SetErrorId(installability_error.error_id)
+            .SetErrorArguments(std::move(installability_error_arguments))
+            .Build());
+  }
   callback->sendSuccess(
-      std::make_unique<protocol::Array<std::string>>(std::move(errors)));
+      std::make_unique<protocol::Array<std::string>>(std::move(errors)),
+      std::move(result_installability_errors));
 }
 
 void PageHandler::GetManifestIcons(

chrome/browser/devtools/protocol/page_handler.h

@@ -10,6 +10,7 @@
 #include "content/public/browser/web_contents_observer.h"
 
 namespace content {
+struct InstallabilityError;
 class WebContents;
 }
 
@@ -37,7 +38,8 @@ class PageHandler : public protocol::Page::Backend,
  private:
   static void GotInstallabilityErrors(
       std::unique_ptr<GetInstallabilityErrorsCallback> callback,
-      std::vector<std::string> errors);
+      std::vector<std::string> errors,
+      std::vector<content::InstallabilityError> installability_errors);
 
   static void GotManifestIcons(
       std::unique_ptr<GetManifestIconsCallback> callback,

chrome/browser/installable/installable_logging.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/installable/installable_logging.h"
 
+#include <vector>
+
 #include "base/no_destructor.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/installable/installable_manager.h"
@@ -58,6 +60,35 @@ static const char kNoUrlForServiceWorker[] =
 static const char kPreferRelatedApplications[] =
     "Manifest specifies prefer_related_applications: true";
 
+static const char kNotInMainFrameId[] = "not-in-main-frame";
+static const char kNotFromSecureOriginId[] = "not-from-secure-origin";
+static const char kNoManifestId[] = "no-manifest";
+static const char kManifestEmptyId[] = "manifest-empty";
+static const char kStartUrlNotValidId[] = "start-url-not-valid";
+static const char kManifestMissingNameOrShortNameId[] =
+    "manifest-missing-name-or-short-name";
+static const char kManifestDisplayNotSupportedId[] =
+    "manifest-display-not-supported";
+static const char kManifestMissingSuitableIconId[] =
+    "manifest-missing-suitable-icon";
+static const char kMinimumIconSizeInPixelsId[] = "minimum-icon-size-in-pixels";
+static const char kNoMatchingServiceWorkerId[] = "no-matching-service-worker";
+static const char kNoAcceptableIconId[] = "no-acceptable-icon";
+static const char kCannotDownloadIconId[] = "cannot-download-icon";
+static const char kNoIconAvailableId[] = "no-icon-available";
+static const char kPlatformNotSupportedOnAndroidId[] =
+    "platform-not-supported-on-android";
+static const char kNoIdSpecifiedId[] = "no-id-specified";
+static const char kIdsDoNotMatchId[] = "ids-do-not-match";
+static const char kAlreadyInstalledId[] = "already-installed";
+static const char kUrlNotSupportedForWebApkId[] =
+    "url-not-supported-for-webapk";
+static const char kInIncognitoId[] = "in-incognito";
+static const char kNotOfflineCapableId[] = "not-offline-capable";
+static const char kNoUrlForServiceWorkerId[] = "no-url-for-service-worker";
+static const char kPreferRelatedApplicationsId[] =
+    "prefer-related-applications";
+
 const std::string& GetMessagePrefix() {
   static base::NoDestructor<std::string> message_prefix(
       "Site cannot be installed: ");
@@ -160,6 +191,106 @@ std::string GetErrorMessage(InstallableStatusCode code) {
   return message;
 }
 
+content::InstallabilityError GetInstallabilityError(
+    InstallableStatusCode code) {
+  content::InstallabilityError error;
+  std::string error_id;
+  std::vector<content::InstallabilityErrorArgument> error_arguments;
+  switch (code) {
+    case NO_ERROR_DETECTED:
+    // These codes are solely used for UMA reporting.
+    case RENDERER_EXITING:
+    case RENDERER_CANCELLED:
+    case USER_NAVIGATED:
+    case INSUFFICIENT_ENGAGEMENT:
+    case PACKAGE_NAME_OR_START_URL_EMPTY:
+    case PREVIOUSLY_BLOCKED:
+    case PREVIOUSLY_IGNORED:
+    case SHOWING_NATIVE_APP_BANNER:
+    case SHOWING_WEB_APP_BANNER:
+    case FAILED_TO_CREATE_BANNER:
+    case WAITING_FOR_MANIFEST:
+    case WAITING_FOR_INSTALLABLE_CHECK:
+    case NO_GESTURE:
+    case WAITING_FOR_NATIVE_DATA:
+    case SHOWING_APP_INSTALLATION_DIALOG:
+    case MAX_ERROR_CODE:
+      break;
+    case NOT_IN_MAIN_FRAME:
+      error_id = kNotInMainFrameId;
+      break;
+    case NOT_FROM_SECURE_ORIGIN:
+      error_id = kNotFromSecureOriginId;
+      break;
+    case NO_MANIFEST:
+      error_id = kNoManifestId;
+      break;
+    case MANIFEST_EMPTY:
+      error_id = kManifestEmptyId;
+      break;
+    case START_URL_NOT_VALID:
+      error_id = kStartUrlNotValidId;
+      break;
+    case MANIFEST_MISSING_NAME_OR_SHORT_NAME:
+      error_id = kManifestMissingNameOrShortNameId;
+      break;
+    case MANIFEST_DISPLAY_NOT_SUPPORTED:
+      error_id = kManifestDisplayNotSupportedId;
+      break;
+    case MANIFEST_MISSING_SUITABLE_ICON:
+      error_id = kManifestMissingSuitableIconId;
+      error_arguments.push_back(content::InstallabilityErrorArgument(
+          kMinimumIconSizeInPixelsId,
+          base::NumberToString(InstallableManager::GetMinimumIconSizeInPx())));
+      break;
+    case NO_MATCHING_SERVICE_WORKER:
+      error_id = kNoMatchingServiceWorkerId;
+      break;
+    case NO_ACCEPTABLE_ICON:
+      error_id = kNoAcceptableIconId;
+      error_arguments.push_back(content::InstallabilityErrorArgument(
+          kMinimumIconSizeInPixelsId,
+          base::NumberToString(InstallableManager::GetMinimumIconSizeInPx())));
+      break;
+    case CANNOT_DOWNLOAD_ICON:
+      error_id = kCannotDownloadIconId;
+      break;
+    case NO_ICON_AVAILABLE:
+      error_id = kNoIconAvailableId;
+      break;
+    case PLATFORM_NOT_SUPPORTED_ON_ANDROID:
+      error_id = kPlatformNotSupportedOnAndroidId;
+      break;
+    case NO_ID_SPECIFIED:
+      error_id = kNoIdSpecifiedId;
+      break;
+    case IDS_DO_NOT_MATCH:
+      error_id = kIdsDoNotMatchId;
+      break;
+    case ALREADY_INSTALLED:
+      error_id = kAlreadyInstalledId;
+      break;
+    case URL_NOT_SUPPORTED_FOR_WEBAPK:
+      error_id = kUrlNotSupportedForWebApkId;
+      break;
+    case IN_INCOGNITO:
+      error_id = kInIncognitoId;
+      break;
+    case NOT_OFFLINE_CAPABLE:
+      error_id = kNotOfflineCapableId;
+      break;
+    case NO_URL_FOR_SERVICE_WORKER:
+      error_id = kNoUrlForServiceWorkerId;
+      break;
+    case PREFER_RELATED_APPLICATIONS:
+      error_id = kPreferRelatedApplicationsId;
+      break;
+  }
+  error.error_id = error_id;
+  error.installability_error_arguments = error_arguments;
+  return error;
+}
+
 void LogErrorToConsole(content::WebContents* web_contents,
                        InstallableStatusCode code) {
   if (!web_contents)

chrome/browser/installable/installable_logging.h

@@ -8,6 +8,7 @@
 #include <string>
 
 namespace content {
+struct InstallabilityError;
 class WebContents;
 }
 
@@ -60,6 +61,7 @@ enum InstallableStatusCode {
 // Returns a user-readable description for |code|, or an empty string if |code|
 // should not be exposed.
 std::string GetErrorMessage(InstallableStatusCode code);
+content::InstallabilityError GetInstallabilityError(InstallableStatusCode code);
 
 // Logs a message associated with |code| to the devtools console attached to
 // |web_contents|. Does nothing if |web_contents| is nullptr.

chrome/browser/installable/installable_manager.cc

@@ -180,16 +180,25 @@ bool IsParamsForPwaCheck(const InstallableParams& params) {
 }
 
 void OnDidCompleteGetAllErrors(
-    base::OnceCallback<void(std::vector<std::string> errors)> callback,
+    base::OnceCallback<void(std::vector<std::string> errors,
+                            std::vector<content::InstallabilityError>
+                                installability_errors)> callback,
     const InstallableData& data) {
   std::vector<std::string> error_messages;
+  std::vector<content::InstallabilityError> installability_errors;
   for (auto error : data.errors) {
     std::string message = GetErrorMessage(error);
     if (!message.empty())
       error_messages.push_back(std::move(message));
+
+    content::InstallabilityError installability_error =
+        GetInstallabilityError(error);
+    if (!installability_error.error_id.empty())
+      installability_errors.push_back(installability_error);
   }
 
-  std::move(callback).Run(std::move(error_messages));
+  std::move(callback).Run(std::move(error_messages),
+                          std::move(installability_errors));
 }
 
 void OnDidCompleteGetPrimaryIcon(
@@ -292,7 +301,9 @@ void InstallableManager::GetData(const InstallableParams& params,
 }
 
 void InstallableManager::GetAllErrors(
-    base::OnceCallback<void(std::vector<std::string> errors)> callback) {
+    base::OnceCallback<void(std::vector<std::string> errors,
+                            std::vector<content::InstallabilityError>
+                                installability_errors)> callback) {
   InstallableParams params;
   params.check_eligibility = true;
   params.valid_manifest = true;

chrome/browser/installable/installable_manager.h

@@ -18,6 +18,7 @@
 #include "chrome/browser/installable/installable_logging.h"
 #include "chrome/browser/installable/installable_params.h"
 #include "chrome/browser/installable/installable_task_queue.h"
+#include "content/public/browser/installability_error.h"
 #include "content/public/browser/service_worker_context.h"
 #include "content/public/browser/service_worker_context_observer.h"
 #include "content/public/browser/web_contents_observer.h"
@@ -64,7 +65,9 @@ class InstallableManager
   // passing a list of human-readable strings describing the errors encountered
   // during the run. The list is empty if no errors were encountered.
   void GetAllErrors(
-      base::OnceCallback<void(std::vector<std::string> errors)> callback);
+      base::OnceCallback<void(std::vector<std::string> errors,
+                              std::vector<content::InstallabilityError>
+                                  installability_errors)> callback);
 
   void GetPrimaryIcon(
       base::OnceCallback<void(const SkBitmap* primaryIcon)> callback);

chrome/browser/installable/installable_manager_browsertest.cc

@@ -252,8 +252,9 @@ class InstallableManagerBrowserTest : public InProcessBrowserTest {
     base::RunLoop run_loop;
     std::vector<std::string> result;
 
-    manager->GetAllErrors(
-        base::BindLambdaForTesting([&](std::vector<std::string> errors) {
+    manager->GetAllErrors(base::BindLambdaForTesting(
+        [&](std::vector<std::string> errors,
+            std::vector<content::InstallabilityError> installability_errors) {
           result = std::move(errors);
           run_loop.Quit();
         }));
@@ -261,6 +262,26 @@ class InstallableManagerBrowserTest : public InProcessBrowserTest {
     return result;
   }
 
+  std::vector<content::InstallabilityError>
+  NavigateAndGetAllInstallabilityErrors(Browser* browser,
+                                        const std::string& url) {
+    GURL test_url = embedded_test_server()->GetURL(url);
+    ui_test_utils::NavigateToURL(browser, test_url);
+    InstallableManager* manager = GetManager(browser);
+
+    base::RunLoop run_loop;
+    std::vector<content::InstallabilityError> result;
+
+    manager->GetAllErrors(base::BindLambdaForTesting(
+        [&](std::vector<std::string> errors,
+            std::vector<content::InstallabilityError> installability_errors) {
+          result = std::move(installability_errors);
+          run_loop.Quit();
+        }));
+    run_loop.Run();
+    return result;
+  }
+
   void RunInstallableManager(Browser* browser,
                              CallbackTester* tester,
                              const InstallableParams& params) {
@@ -1516,6 +1537,34 @@ IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
                                         "/banners/play_app_manifest.json")));
 }
 
+IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
+                       GetAllInatallabilityErrorsNoErrors) {
+  EXPECT_EQ(std::vector<content::InstallabilityError>{},
+            NavigateAndGetAllInstallabilityErrors(
+                browser(), "/banners/manifest_test_page.html"));
+}
+
+IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
+                       GetAllInatallabilityErrorsWithNoManifest) {
+  EXPECT_EQ(std::vector<content::InstallabilityError>{GetInstallabilityError(
+                NO_MANIFEST)},
+            NavigateAndGetAllInstallabilityErrors(
+                browser(), "/banners/no_manifest_test_page.html"));
+}
+
+IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
+                       GetAllInatallabilityErrorsWithPlayAppManifest) {
+  EXPECT_EQ(std::vector<content::InstallabilityError>(
+                {GetInstallabilityError(START_URL_NOT_VALID),
+                 GetInstallabilityError(MANIFEST_MISSING_NAME_OR_SHORT_NAME),
+                 GetInstallabilityError(MANIFEST_DISPLAY_NOT_SUPPORTED),
+                 GetInstallabilityError(MANIFEST_MISSING_SUITABLE_ICON),
+                 GetInstallabilityError(NO_ACCEPTABLE_ICON)}),
+            NavigateAndGetAllInstallabilityErrors(
+                browser(), GetURLOfPageWithServiceWorkerAndManifest(
+                               "/banners/play_app_manifest.json")));
+}
+
 IN_PROC_BROWSER_TEST_F(InstallableManagerAllowlistOriginBrowserTest,
                        SecureOriginCheckRespectsUnsafeFlag) {
   // The allowlisted origin should be regarded as secure.

content/browser/devtools/protocol/page_handler.cc

@@ -1169,9 +1169,11 @@ Response PageHandler::SetWebLifecycleState(const std::string& state) {
 void PageHandler::GetInstallabilityErrors(
     std::unique_ptr<GetInstallabilityErrorsCallback> callback) {
   auto errors = std::make_unique<protocol::Array<std::string>>();
+  auto installability_errors =
+      std::make_unique<protocol::Array<Page::InstallabilityError>>();
   // TODO: Use InstallableManager once it moves into content/.
   // Until then, this code is only used to return empty array in the tests.
-  callback->sendSuccess(std::move(errors));
+  callback->sendSuccess(std::move(errors), std::move(installability_errors));
 }
 
 void PageHandler::GetManifestIcons(

content/public/browser/BUILD.gn

@@ -183,6 +183,8 @@ jumbo_source_set("browser_sources") {
     "hid_delegate.h",
     "histogram_fetcher.h",
     "indexed_db_context.h",
+    "installability_error.cc",
+    "installability_error.h",
     "interstitial_page.h",
     "interstitial_page_delegate.cc",
     "interstitial_page_delegate.h",

content/public/browser/installability_error.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/public/browser/installability_error.h"
+
+#include <utility>
+
+namespace content {
+
+InstallabilityErrorArgument::InstallabilityErrorArgument(std::string name,
+                                                         std::string value)
+    : name(std::move(name)), value(std::move(value)) {}
+
+bool InstallabilityErrorArgument::operator==(
+    const InstallabilityErrorArgument& other) const {
+  return name == other.name && value == other.value;
+}
+
+InstallabilityErrorArgument::~InstallabilityErrorArgument() = default;
+
+InstallabilityError::InstallabilityError() = default;
+
+InstallabilityError::InstallabilityError(std::string error_id)
+    : error_id(std::move(error_id)) {}
+
+InstallabilityError::InstallabilityError(const InstallabilityError& other) =
+    default;
+
+InstallabilityError::InstallabilityError(InstallabilityError&& other) = default;
+
+bool InstallabilityError::operator==(const InstallabilityError& other) const {
+  return error_id == other.error_id &&
+         installability_error_arguments == other.installability_error_arguments;
+}
+
+InstallabilityError::~InstallabilityError() = default;
+
+}  // namespace content

content/public/browser/installability_error.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_PUBLIC_BROWSER_INSTALLABILITY_ERROR_H_
+#define CONTENT_PUBLIC_BROWSER_INSTALLABILITY_ERROR_H_
+
+#include <string>
+#include <vector>
+
+#include "content/common/content_export.h"
+
+namespace content {
+
+struct CONTENT_EXPORT InstallabilityErrorArgument {
+  InstallabilityErrorArgument() = default;
+  InstallabilityErrorArgument(std::string name, std::string value);
+  bool operator==(const InstallabilityErrorArgument& other) const;
+  ~InstallabilityErrorArgument();
+
+  std::string name;
+  std::string value;
+};
+
+struct CONTENT_EXPORT InstallabilityError {
+  InstallabilityError();
+  explicit InstallabilityError(std::string error_id);
+  InstallabilityError(const InstallabilityError& other);
+  InstallabilityError(InstallabilityError&& other);
+  bool operator==(const InstallabilityError& other) const;
+  ~InstallabilityError();
+
+  std::string error_id;
+  std::vector<InstallabilityErrorArgument> installability_error_arguments;
+};
+
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_BROWSER_INSTALLABILITY_ERROR_H_

third_party/blink/public/devtools_protocol/browser_protocol.pdl

@@ -5338,6 +5338,21 @@ domain Page
       reload
       anchorClick
 
+  experimental type InstallabilityErrorArgument extends object
+    properties
+      # Argument name (e.g. name:'minimum-icon-size-in-pixels').
+      string name
+      # Argument value (e.g. value:'64').
+      string value
+
+  # The installability error
+  experimental type InstallabilityError extends object
+    properties
+      # The error id (e.g. 'manifest-missing-suitable-icon').
+      string errorId
+      # The list of error arguments (e.g. {name:'minimum-icon-size-in-pixels', value:'64'}).
+      array of InstallabilityErrorArgument errorArguments
+
   # Deprecated, please use addScriptToEvaluateOnNewDocument instead.
   experimental deprecated command addScriptToEvaluateOnLoad
     parameters
@@ -5446,7 +5461,8 @@ domain Page
 
   experimental command getInstallabilityErrors
     returns
-      array of string errors
+      deprecated array of string errors
+      experimental array of InstallabilityError installabilityErrors
 
   experimental command getManifestIcons
     returns