Commit 01493038 authored by Alexey Baskakov's avatar Alexey Baskakov Committed by Commit Bot

WebApp: Fold BookmarkAppInstallationTask::ResultCode into InstallResultCode.

Merge these two enums.

Move InstallResultCode enum into web_app_constants.h.

kInstallationFailed enumerator entry becomes kFailedUnknownReason.

TBR=jlklein@chromium.org

Bug: 875698
Change-Id: Ia5c2fb46ae0182edf1c8a3b0215f6905ad70f094
Reviewed-on: https://chromium-review.googlesource.com/c/1285871
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: default avatarBen Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600280}
parent d3888464
...@@ -14,7 +14,6 @@ ...@@ -14,7 +14,6 @@
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/extensions/app_launch_params.h" #include "chrome/browser/ui/extensions/app_launch_params.h"
#include "chrome/browser/ui/extensions/application_launch.h" #include "chrome/browser/ui/extensions/application_launch.h"
#include "chrome/browser/web_applications/components/install_result_code.h"
#include "chrome/browser/web_applications/components/pending_app_manager.h" #include "chrome/browser/web_applications/components/pending_app_manager.h"
#include "chrome/browser/web_applications/components/web_app_constants.h" #include "chrome/browser/web_applications/components/web_app_constants.h"
#include "chrome/browser/web_applications/web_app_provider.h" #include "chrome/browser/web_applications/web_app_provider.h"
......
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
source_set("components") { source_set("components") {
sources = [ sources = [
"install_manager.h", "install_manager.h",
"install_result_code.h",
"pending_app_manager.cc", "pending_app_manager.cc",
"pending_app_manager.h", "pending_app_manager.h",
"web_app_constants.h", "web_app_constants.h",
......
// Copyright 2018 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 CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_INSTALL_RESULT_CODE_H_
#define CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_INSTALL_RESULT_CODE_H_
namespace web_app {
// The result of an attempted web app installation, uninstallation or update.
//
// This is an enum, instead of a struct with multiple fields (e.g. one field for
// success or failure, one field for whether action was taken), because we want
// to track metrics for the overall cross product of the these axes.
//
// TODO(nigeltao): fold BookmarkAppInstallationTask::ResultCode into this class.
enum class InstallResultCode {
kSuccess,
kAlreadyInstalled,
// Catch-all failure category. More-specific failure categories are below.
kFailedUnknownReason,
kGetWebApplicationInfoFailed,
kPreviouslyUninstalled,
};
} // namespace web_app
#endif // CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_INSTALL_RESULT_CODE_H_
...@@ -8,7 +8,7 @@ ...@@ -8,7 +8,7 @@
#include <utility> #include <utility>
#include "base/callback.h" #include "base/callback.h"
#include "chrome/browser/web_applications/components/install_result_code.h" #include "chrome/browser/web_applications/components/web_app_constants.h"
#include "url/gurl.h" #include "url/gurl.h"
namespace web_app { namespace web_app {
......
...@@ -17,6 +17,20 @@ enum class LaunchContainer { ...@@ -17,6 +17,20 @@ enum class LaunchContainer {
kWindow, kWindow,
}; };
// The result of an attempted web app installation, uninstallation or update.
//
// This is an enum, instead of a struct with multiple fields (e.g. one field for
// success or failure, one field for whether action was taken), because we want
// to track metrics for the overall cross product of the these axes.
enum class InstallResultCode {
kSuccess,
kAlreadyInstalled,
// Catch-all failure category. More-specific failure categories are below.
kFailedUnknownReason,
kGetWebApplicationInfoFailed,
kPreviouslyUninstalled,
};
// Where an app was installed from. This affects what flags will be used when // Where an app was installed from. This affects what flags will be used when
// installing the app. // installing the app.
// //
......
...@@ -36,10 +36,10 @@ std::unique_ptr<BookmarkAppHelper> BookmarkAppHelperCreateWrapper( ...@@ -36,10 +36,10 @@ std::unique_ptr<BookmarkAppHelper> BookmarkAppHelperCreateWrapper(
} // namespace } // namespace
BookmarkAppInstallationTask::Result::Result(ResultCode code, BookmarkAppInstallationTask::Result::Result(web_app::InstallResultCode code,
base::Optional<std::string> app_id) base::Optional<std::string> app_id)
: code(code), app_id(std::move(app_id)) { : code(code), app_id(std::move(app_id)) {
DCHECK_EQ(code == ResultCode::kSuccess, app_id.has_value()); DCHECK_EQ(code == web_app::InstallResultCode::kSuccess, app_id.has_value());
} }
BookmarkAppInstallationTask::Result::Result(Result&&) = default; BookmarkAppInstallationTask::Result::Result(Result&&) = default;
...@@ -98,7 +98,8 @@ void BookmarkAppInstallationTask::OnGetWebApplicationInfo( ...@@ -98,7 +98,8 @@ void BookmarkAppInstallationTask::OnGetWebApplicationInfo(
std::unique_ptr<WebApplicationInfo> web_app_info) { std::unique_ptr<WebApplicationInfo> web_app_info) {
if (!web_app_info) { if (!web_app_info) {
std::move(result_callback) std::move(result_callback)
.Run(Result(ResultCode::kGetWebApplicationInfoFailed, std::string())); .Run(Result(web_app::InstallResultCode::kGetWebApplicationInfoFailed,
std::string()));
return; return;
} }
...@@ -149,8 +150,10 @@ void BookmarkAppInstallationTask::OnInstalled( ...@@ -149,8 +150,10 @@ void BookmarkAppInstallationTask::OnInstalled(
const Extension* extension, const Extension* extension,
const WebApplicationInfo& web_app_info) { const WebApplicationInfo& web_app_info) {
std::move(result_callback) std::move(result_callback)
.Run(extension ? Result(ResultCode::kSuccess, extension->id()) .Run(extension
: Result(ResultCode::kInstallationFailed, base::nullopt)); ? Result(web_app::InstallResultCode::kSuccess, extension->id())
: Result(web_app::InstallResultCode::kFailedUnknownReason,
base::nullopt));
} }
} // namespace extensions } // namespace extensions
...@@ -24,6 +24,7 @@ class WebContents; ...@@ -24,6 +24,7 @@ class WebContents;
namespace web_app { namespace web_app {
class WebAppDataRetriever; class WebAppDataRetriever;
enum class InstallResultCode;
} }
namespace extensions { namespace extensions {
...@@ -36,19 +37,12 @@ class Extension; ...@@ -36,19 +37,12 @@ class Extension;
// or WebApplicationInfo. Can only be called from the UI thread. // or WebApplicationInfo. Can only be called from the UI thread.
class BookmarkAppInstallationTask { class BookmarkAppInstallationTask {
public: public:
// TODO(nigeltao): fold this into web_app::InstallResultCode.
enum class ResultCode {
kSuccess,
kGetWebApplicationInfoFailed,
kInstallationFailed,
};
struct Result { struct Result {
Result(ResultCode code, base::Optional<std::string> app_id); Result(web_app::InstallResultCode code, base::Optional<std::string> app_id);
Result(Result&&); Result(Result&&);
~Result(); ~Result();
const ResultCode code; const web_app::InstallResultCode code;
const base::Optional<std::string> app_id; const base::Optional<std::string> app_id;
DISALLOW_COPY_AND_ASSIGN(Result); DISALLOW_COPY_AND_ASSIGN(Result);
......
...@@ -40,7 +40,6 @@ ...@@ -40,7 +40,6 @@
namespace extensions { namespace extensions {
using Result = BookmarkAppInstallationTask::Result; using Result = BookmarkAppInstallationTask::Result;
using ResultCode = BookmarkAppInstallationTask::ResultCode;
namespace { namespace {
...@@ -128,7 +127,8 @@ class BookmarkAppInstallationTaskTest : public ChromeRenderViewHostTestHarness { ...@@ -128,7 +127,8 @@ class BookmarkAppInstallationTaskTest : public ChromeRenderViewHostTestHarness {
} }
bool app_installed() { bool app_installed() {
bool app_installed = app_installation_result_->code == ResultCode::kSuccess; bool app_installed =
app_installation_result_->code == web_app::InstallResultCode::kSuccess;
EXPECT_EQ(app_installed, app_installation_result_->app_id.has_value()); EXPECT_EQ(app_installed, app_installation_result_->app_id.has_value());
return app_installed; return app_installed;
} }
...@@ -209,9 +209,8 @@ TEST_F(BookmarkAppInstallationTaskTest, ShortcutFromContents_NoWebAppInfo) { ...@@ -209,9 +209,8 @@ TEST_F(BookmarkAppInstallationTaskTest, ShortcutFromContents_NoWebAppInfo) {
run_loop.Run(); run_loop.Run();
EXPECT_FALSE(app_installed()); EXPECT_FALSE(app_installed());
EXPECT_EQ( EXPECT_EQ(web_app::InstallResultCode::kGetWebApplicationInfoFailed,
BookmarkAppInstallationTask::ResultCode::kGetWebApplicationInfoFailed, app_installation_result().code);
app_installation_result().code);
} }
TEST_F(BookmarkAppInstallationTaskTest, ShortcutFromContents_NoManifest) { TEST_F(BookmarkAppInstallationTaskTest, ShortcutFromContents_NoManifest) {
...@@ -262,7 +261,7 @@ TEST_F(BookmarkAppInstallationTaskTest, ...@@ -262,7 +261,7 @@ TEST_F(BookmarkAppInstallationTaskTest,
run_loop.Run(); run_loop.Run();
EXPECT_FALSE(app_installed()); EXPECT_FALSE(app_installed());
EXPECT_EQ(BookmarkAppInstallationTask::ResultCode::kInstallationFailed, EXPECT_EQ(web_app::InstallResultCode::kFailedUnknownReason,
app_installation_result().code); app_installation_result().code);
} }
......
...@@ -53,7 +53,8 @@ void BookmarkAppShortcutInstallationTask::OnGetWebApplicationInfo( ...@@ -53,7 +53,8 @@ void BookmarkAppShortcutInstallationTask::OnGetWebApplicationInfo(
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (!web_app_info) { if (!web_app_info) {
std::move(result_callback) std::move(result_callback)
.Run(Result(ResultCode::kGetWebApplicationInfoFailed, base::nullopt)); .Run(Result(web_app::InstallResultCode::kGetWebApplicationInfoFailed,
base::nullopt));
return; return;
} }
...@@ -91,10 +92,12 @@ void BookmarkAppShortcutInstallationTask::OnInstalled( ...@@ -91,10 +92,12 @@ void BookmarkAppShortcutInstallationTask::OnInstalled(
const std::string& app_id) { const std::string& app_id) {
if (app_id.empty()) { if (app_id.empty()) {
std::move(result_callback) std::move(result_callback)
.Run(Result(ResultCode::kInstallationFailed, base::nullopt)); .Run(Result(web_app::InstallResultCode::kFailedUnknownReason,
base::nullopt));
return; return;
} }
std::move(result_callback).Run(Result(ResultCode::kSuccess, app_id)); std::move(result_callback)
.Run(Result(web_app::InstallResultCode::kSuccess, app_id));
} }
} // namespace extensions } // namespace extensions
...@@ -14,7 +14,6 @@ ...@@ -14,7 +14,6 @@
#include "base/time/time.h" #include "base/time/time.h"
#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/web_applications/components/install_result_code.h"
#include "chrome/browser/web_applications/components/web_app_constants.h" #include "chrome/browser/web_applications/components/web_app_constants.h"
#include "chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.h" #include "chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.h"
#include "content/public/browser/navigation_controller.h" #include "content/public/browser/navigation_controller.h"
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "chrome/browser/extensions/extension_util.h" #include "chrome/browser/extensions/extension_util.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/web_applications/components/install_result_code.h"
#include "chrome/browser/web_applications/components/web_app_constants.h" #include "chrome/browser/web_applications/components/web_app_constants.h"
#include "chrome/browser/web_applications/extensions/web_app_extension_ids_map.h" #include "chrome/browser/web_applications/extensions/web_app_extension_ids_map.h"
#include "chrome/browser/web_applications/web_app_provider.h" #include "chrome/browser/web_applications/web_app_provider.h"
......
...@@ -16,7 +16,6 @@ ...@@ -16,7 +16,6 @@
#include "base/test/bind_test_util.h" #include "base/test/bind_test_util.h"
#include "base/timer/mock_timer.h" #include "base/timer/mock_timer.h"
#include "chrome/browser/extensions/test_extension_system.h" #include "chrome/browser/extensions/test_extension_system.h"
#include "chrome/browser/web_applications/components/install_result_code.h"
#include "chrome/browser/web_applications/components/pending_app_manager.h" #include "chrome/browser/web_applications/components/pending_app_manager.h"
#include "chrome/browser/web_applications/components/web_app_constants.h" #include "chrome/browser/web_applications/components/web_app_constants.h"
#include "chrome/browser/web_applications/extensions/bookmark_app_installation_task.h" #include "chrome/browser/web_applications/extensions/bookmark_app_installation_task.h"
...@@ -126,11 +125,10 @@ class TestBookmarkAppInstallationTask : public BookmarkAppInstallationTask { ...@@ -126,11 +125,10 @@ class TestBookmarkAppInstallationTask : public BookmarkAppInstallationTask {
void InstallWebAppOrShortcutFromWebContents( void InstallWebAppOrShortcutFromWebContents(
content::WebContents* web_contents, content::WebContents* web_contents,
BookmarkAppInstallationTask::ResultCallback callback) override { BookmarkAppInstallationTask::ResultCallback callback) override {
BookmarkAppInstallationTask::ResultCode result_code = auto result_code = web_app::InstallResultCode::kFailedUnknownReason;
BookmarkAppInstallationTask::ResultCode::kInstallationFailed;
std::string app_id; std::string app_id;
if (succeeds_) { if (succeeds_) {
result_code = BookmarkAppInstallationTask::ResultCode::kSuccess; result_code = web_app::InstallResultCode::kSuccess;
app_id = GenerateFakeAppId(app_info().url); app_id = GenerateFakeAppId(app_info().url);
ExtensionRegistry::Get(profile_)->AddEnabled( ExtensionRegistry::Get(profile_)->AddEnabled(
CreateDummyExtension(app_id)); CreateDummyExtension(app_id));
......
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "chrome/browser/web_applications/components/install_result_code.h" #include "chrome/browser/web_applications/components/web_app_constants.h"
#include "chrome/browser/web_applications/components/web_app_data_retriever.h" #include "chrome/browser/web_applications/components/web_app_data_retriever.h"
#include "chrome/browser/web_applications/components/web_app_helpers.h" #include "chrome/browser/web_applications/components/web_app_helpers.h"
#include "chrome/browser/web_applications/web_app.h" #include "chrome/browser/web_applications/web_app.h"
......
...@@ -8,7 +8,7 @@ ...@@ -8,7 +8,7 @@
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/bind_test_util.h" #include "base/test/bind_test_util.h"
#include "chrome/browser/web_applications/components/install_result_code.h" #include "chrome/browser/web_applications/components/web_app_constants.h"
#include "chrome/browser/web_applications/test/test_data_retriever.h" #include "chrome/browser/web_applications/test/test_data_retriever.h"
#include "chrome/browser/web_applications/test/web_app_test.h" #include "chrome/browser/web_applications/test/web_app_test.h"
#include "chrome/browser/web_applications/web_app.h" #include "chrome/browser/web_applications/web_app.h"
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment