Split WebAppTabHelper into public interface and impl

This CL moves the implementation of WebAppTabHelper out of
web_applications/components down into a new web_applications:common
source set.
The publicly accessible methods of the class are retained in
WebAppTabHelperBase.

This is in preparation for migrating some of its dependencies out of
web_applications/components. Many of the classes in components should
be one level up as they're not isolated components or part of the
public interface of web_applications.

Bug: 1035242
Change-Id: I9d660e3450f8d0bb7ff7a5346346d117cabd6259
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1972741
Auto-Submit: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733840}

chrome/browser/BUILD.gn

@@ -4729,6 +4729,7 @@ jumbo_static_library("browser") {
       "//chrome/browser/web_applications",
 
       # TODO(loyso): Erase these. crbug.com/877898.
+      "//chrome/browser/web_applications:common",
       "//chrome/browser/web_applications:web_applications_on_extensions",
       "//chrome/browser/web_applications/components",
       "//chrome/browser/web_applications/extensions",
@@ -4737,6 +4738,7 @@ jumbo_static_library("browser") {
       "//apps",
       "//chrome/browser/sync_file_system/drive_backend:sync_file_system_drive_proto",
       "//chrome/browser/web_applications",
+      "//chrome/browser/web_applications:common",
       "//chrome/browser/web_applications:web_applications_on_extensions",
       "//chrome/browser/web_applications/components",
       "//chrome/browser/web_applications/extensions",

chrome/browser/chromeos/child_accounts/time_limits/web_time_limit_navigation_throttle.cc

@@ -18,7 +18,7 @@
 #include "chrome/browser/ui/web_applications/app_browser_controller.h"
 #include "chrome/browser/web_applications/components/app_registrar.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
-#include "chrome/browser/web_applications/components/web_app_tab_helper.h"
+#include "chrome/browser/web_applications/components/web_app_tab_helper_base.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
@@ -129,15 +129,13 @@ ThrottleCheckResult WebTimeLimitNavigationThrottle::WillStartOrRedirectRequest(
   const std::string& app_locale = g_browser_process->GetApplicationLocale();
 
   Browser::Type type = browser->type();
-  web_app::WebAppTabHelper* web_app_helper =
-      web_app::WebAppTabHelper::FromWebContents(web_contents);
+  web_app::WebAppTabHelperBase* web_app_helper =
+      web_app::WebAppTabHelperBase::FromWebContents(web_contents);
 
   bool is_windowed = (type == Browser::Type::TYPE_APP_POPUP) ||
                      (type == Browser::Type::TYPE_APP) ||
                      (type == Browser::Type::TYPE_POPUP);
-  bool is_app = false;
-  if (web_app_helper && !web_app_helper->app_id().empty())
-    is_app = true;
+  bool is_app = web_app_helper && !web_app_helper->GetAppId().empty();
 
   // Don't throttle windowed applications. We show a notification and close
   // them.
@@ -152,7 +150,7 @@ ThrottleCheckResult WebTimeLimitNavigationThrottle::WillStartOrRedirectRequest(
         web_app::WebAppProvider::Get(profile);
     const web_app::AppRegistrar& registrar = web_app_provider->registrar();
     const std::string& app_name =
-        registrar.GetAppShortName(web_app_helper->app_id());
+        registrar.GetAppShortName(web_app_helper->GetAppId());
     interstitial_html =
         GetWebTimeLimitAppErrorPage(time_limit, app_locale, app_name);
   } else {

chrome/browser/extensions/browsertest_util.cc

@@ -24,7 +24,7 @@
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/components/web_app_install_utils.h"
 #include "chrome/browser/web_applications/components/web_app_provider_base.h"
-#include "chrome/browser/web_applications/components/web_app_tab_helper.h"
+#include "chrome/browser/web_applications/components/web_app_tab_helper_base.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/web_application_info.h"
 #include "chrome/test/base/ui_test_utils.h"

chrome/browser/ui/BUILD.gn

@@ -1453,6 +1453,7 @@ jumbo_static_library("ui") {
         "//chrome/browser/web_applications",
 
         # TODO(loyso): Erase these. crbug.com/877898.
+        "//chrome/browser/web_applications:common",
         "//chrome/browser/web_applications:web_applications_on_extensions",
         "//chrome/browser/web_applications/components",
         "//chrome/browser/web_applications/extensions",

chrome/browser/ui/extensions/application_launch.cc

@@ -35,7 +35,7 @@
 #include "chrome/browser/web_applications/components/file_handler_manager.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/components/web_app_provider_base.h"
-#include "chrome/browser/web_applications/components/web_app_tab_helper.h"
+#include "chrome/browser/web_applications/components/web_app_tab_helper_base.h"
 #include "chrome/browser/web_applications/system_web_app_manager.h"
 #include "chrome/browser/web_launch/web_launch_files_helper.h"
 #include "chrome/common/chrome_features.h"
@@ -268,8 +268,8 @@ WebContents* OpenApplicationTab(Profile* profile,
   }
 
   if (extension->from_bookmark()) {
-    web_app::WebAppTabHelper* tab_helper =
-        web_app::WebAppTabHelper::FromWebContents(contents);
+    web_app::WebAppTabHelperBase* tab_helper =
+        web_app::WebAppTabHelperBase::FromWebContents(contents);
     DCHECK(tab_helper);
     tab_helper->SetAppId(extension->id());
   }
@@ -445,8 +445,8 @@ WebContents* NavigateApplicationWindow(Browser* browser,
   // TODO(https://crbug.com/1032443):
   // Eventually move this to browser_navigator.cc: CreateTargetContents().
   if (extension && extension->from_bookmark()) {
-    web_app::WebAppTabHelper* tab_helper =
-        web_app::WebAppTabHelper::FromWebContents(web_contents);
+    web_app::WebAppTabHelperBase* tab_helper =
+        web_app::WebAppTabHelperBase::FromWebContents(web_contents);
     DCHECK(tab_helper);
     tab_helper->SetAppId(extension->id());
   }

chrome/browser/ui/tab_helpers.cc

@@ -139,8 +139,8 @@
 #include "chrome/browser/extensions/api/web_navigation/web_navigation_api.h"
 #include "chrome/browser/extensions/chrome_extension_web_contents_observer.h"
 #include "chrome/browser/extensions/tab_helper.h"
-#include "chrome/browser/web_applications/components/web_app_tab_helper.h"
 #include "chrome/browser/web_applications/components/web_app_utils.h"
+#include "chrome/browser/web_applications/web_app_tab_helper.h"
 #include "extensions/browser/view_type_utils.h"
 #endif
 

chrome/browser/ui/views/page_action/pwa_install_view.cc

@@ -12,7 +12,6 @@
 #include "chrome/browser/ui/views/extensions/pwa_confirmation_bubble_view.h"
 #include "chrome/browser/ui/web_applications/web_app_dialog_utils.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
-#include "chrome/browser/web_applications/components/web_app_tab_helper.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/omnibox/browser/vector_icons.h"
 #include "ui/base/l10n/l10n_util.h"

chrome/browser/ui/web_applications/test/web_app_browsertest_util.cc

@@ -16,7 +16,7 @@
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/components/web_app_provider_base.h"
-#include "chrome/browser/web_applications/components/web_app_tab_helper.h"
+#include "chrome/browser/web_applications/components/web_app_tab_helper_base.h"
 #include "chrome/common/web_application_info.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -65,9 +65,10 @@ Browser* LaunchBrowserForWebAppInTab(Profile* profile, const AppId& app_id) {
           apps::mojom::AppLaunchSource::kSourceTest));
   DCHECK(web_contents);
 
-  WebAppTabHelper* tab_helper = WebAppTabHelper::FromWebContents(web_contents);
+  WebAppTabHelperBase* tab_helper =
+      WebAppTabHelperBase::FromWebContents(web_contents);
   DCHECK(tab_helper);
-  EXPECT_EQ(app_id, tab_helper->app_id());
+  EXPECT_EQ(app_id, tab_helper->GetAppId());
 
   Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
   EXPECT_EQ(browser, chrome::FindLastActive());

chrome/browser/ui/web_applications/web_app_launch_manager.cc

@@ -16,11 +16,11 @@
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/components/web_app_install_utils.h"
-#include "chrome/browser/web_applications/components/web_app_tab_helper.h"
 #include "chrome/browser/web_applications/system_web_app_manager.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
 #include "chrome/browser/web_applications/web_app_registrar.h"
+#include "chrome/browser/web_applications/web_app_tab_helper.h"
 #include "chrome/browser/web_launch/web_launch_files_helper.h"
 #include "content/public/browser/render_view_host.h"
 #include "extensions/common/constants.h"
@@ -65,7 +65,8 @@ content::WebContents* NavigateWebApplicationWindow(
 
   // TODO(https://crbug.com/1032443):
   // Eventually move this to browser_navigator.cc: CreateTargetContents().
-  WebAppTabHelper* tab_helper = WebAppTabHelper::FromWebContents(web_contents);
+  WebAppTabHelperBase* tab_helper =
+      WebAppTabHelperBase::FromWebContents(web_contents);
   DCHECK(tab_helper);
   tab_helper->SetAppId(app_id);
 

chrome/browser/ui/web_applications/web_app_metrics.cc

@@ -11,7 +11,7 @@
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/web_applications/web_app_metrics_factory.h"
 #include "chrome/browser/web_applications/components/app_registrar.h"
-#include "chrome/browser/web_applications/components/web_app_tab_helper.h"
+#include "chrome/browser/web_applications/components/web_app_tab_helper_base.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
 #include "content/public/browser/web_contents.h"
 
@@ -97,9 +97,10 @@ void WebAppMetrics::OnEngagementEvent(
                               engagement_type);
   }
 
-  // A presence of WebAppTabHelper with valid app_id indicates a web app.
-  WebAppTabHelper* tab_helper = WebAppTabHelper::FromWebContents(web_contents);
-  if (!tab_helper || tab_helper->app_id().empty())
+  // A presence of WebAppTabHelperBase with valid app_id indicates a web app.
+  WebAppTabHelperBase* tab_helper =
+      WebAppTabHelperBase::FromWebContents(web_contents);
+  if (!tab_helper || tab_helper->GetAppId().empty())
     return;
 
   // No HostedAppBrowserController if app is running as a tab in common browser.

chrome/browser/web_applications/BUILD.gn

@@ -9,6 +9,26 @@ group("web_app_group") {
 group("web_app_test_group") {
 }
 
+# Implementations that are common to both BMO/Extensions backends.
+# TODO(alancutter): Migrate sources from :web_applications into here where
+# possible.
+# TODO(https://crbug.com/877898): Merge this into :web_applications once all
+# Extension codepaths have been removed.
+source_set("common") {
+  sources = [
+    "web_app_tab_helper.cc",
+    "web_app_tab_helper.h",
+  ]
+
+  deps = [
+    ":web_app_group",
+    "//chrome/browser/web_applications/components",
+    "//chrome/common",
+    "//content/public/browser",
+    "//skia",
+  ]
+}
+
 source_set("web_applications") {
   sources = [
     "external_web_app_manager.cc",
@@ -51,6 +71,7 @@ source_set("web_applications") {
   ]
 
   deps = [
+    ":common",
     ":web_app_group",
     "//chrome/browser/web_applications/components",
     "//chrome/common",

chrome/browser/web_applications/components/BUILD.gn

@@ -57,8 +57,8 @@ source_set("components") {
     "web_app_provider_base_factory.h",
     "web_app_shortcut.cc",
     "web_app_shortcut.h",
-    "web_app_tab_helper.cc",
-    "web_app_tab_helper.h",
+    "web_app_tab_helper_base.cc",
+    "web_app_tab_helper_base.h",
     "web_app_ui_manager.h",
     "web_app_url_loader.cc",
     "web_app_url_loader.h",

chrome/browser/web_applications/components/manifest_update_manager_browsertest.cc

@@ -21,7 +21,6 @@
 #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_provider_base.h"
-#include "chrome/browser/web_applications/components/web_app_tab_helper.h"
 #include "chrome/browser/web_applications/test/web_app_test.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/test/base/in_process_browser_test.h"

chrome/browser/web_applications/components/web_app_tab_helper_base.cc

+// Copyright 2019 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 "chrome/browser/web_applications/components/web_app_tab_helper_base.h"
+
+namespace web_app {
+
+WEB_CONTENTS_USER_DATA_KEY_IMPL(WebAppTabHelperBase)
+
+}  // namespace web_app

chrome/browser/web_applications/components/web_app_tab_helper_base.h

+// Copyright 2019 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_WEB_APP_TAB_HELPER_BASE_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_WEB_APP_TAB_HELPER_BASE_H_
+
+#include "chrome/browser/web_applications/components/web_app_helpers.h"
+#include "content/public/browser/web_contents_user_data.h"
+
+namespace base {
+class UnguessableToken;
+}
+
+namespace web_app {
+
+// Public interface for WebAppTabHelper.
+class WebAppTabHelperBase
+    : public content::WebContentsUserData<WebAppTabHelperBase> {
+ public:
+  using content::WebContentsUserData<WebAppTabHelperBase>::FromWebContents;
+
+  virtual const AppId& GetAppId() const = 0;
+  virtual void SetAppId(const AppId& app_id) = 0;
+
+  // These methods require an app associated with the tab (valid GetAppId()).
+
+  // Returns true if the app was installed by user, false if default installed.
+  virtual bool IsUserInstalled() const = 0;
+  // For user-installed apps:
+  // Returns true if the app was installed through the install button.
+  // Returns false if the app was installed through the create shortcut button.
+  virtual bool IsFromInstallButton() const = 0;
+
+  virtual const base::UnguessableToken& GetAudioFocusGroupIdForTesting()
+      const = 0;
+
+  WEB_CONTENTS_USER_DATA_KEY_DECL();
+};
+
+}  // namespace web_app
+
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_WEB_APP_TAB_HELPER_BASE_H_

chrome/browser/web_applications/extensions/web_app_audio_focus_browsertest.cc

@@ -7,7 +7,7 @@
 #include "chrome/browser/extensions/browsertest_util.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/ui/browser.h"
-#include "chrome/browser/web_applications/components/web_app_tab_helper.h"
+#include "chrome/browser/web_applications/components/web_app_tab_helper_base.h"
 #include "chrome/common/web_application_info.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/notification_service.h"
@@ -75,8 +75,9 @@ class WebAppAudioFocusBrowserTest : public extensions::ExtensionBrowserTest {
 
   const base::UnguessableToken& GetAudioFocusGroupId(
       content::WebContents* web_contents) {
-    WebAppTabHelper* helper = WebAppTabHelper::FromWebContents(web_contents);
-    return helper->audio_focus_group_id_;
+    WebAppTabHelperBase* helper =
+        WebAppTabHelperBase::FromWebContents(web_contents);
+    return helper->GetAudioFocusGroupIdForTesting();
   }
 
  private:

chrome/browser/web_applications/components/web_app_tab_helper.cc → chrome/browser/web_applications/web_app_tab_helper.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/web_applications/components/web_app_tab_helper.h"
+#include "chrome/browser/web_applications/web_app_tab_helper.h"
 
 #include "base/unguessable_token.h"
 #include "chrome/browser/profiles/profile.h"
@@ -17,7 +17,13 @@
 
 namespace web_app {
 
-WEB_CONTENTS_USER_DATA_KEY_IMPL(WebAppTabHelper)
+void WebAppTabHelper::CreateForWebContents(content::WebContents* contents) {
+  DCHECK(contents);
+  if (!FromWebContents(contents)) {
+    contents->SetUserData(UserDataKey(),
+                          std::make_unique<WebAppTabHelper>(contents));
+  }
+}
 
 WebAppTabHelper::WebAppTabHelper(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
@@ -31,6 +37,26 @@ WebAppTabHelper::WebAppTabHelper(content::WebContents* web_contents)
 
 WebAppTabHelper::~WebAppTabHelper() = default;
 
+const AppId& WebAppTabHelper::GetAppId() const {
+  return app_id_;
+}
+
+bool WebAppTabHelper::IsUserInstalled() const {
+  return !app_id_.empty() && provider_->registrar().WasInstalledByUser(app_id_);
+}
+
+bool WebAppTabHelper::IsFromInstallButton() const {
+  // TODO(loyso): Use something better to record apps installed from promoted
+  // UIs. crbug.com/774918.
+  return !app_id_.empty() &&
+         provider_->registrar().GetAppScope(app_id_).has_value();
+}
+
+const base::UnguessableToken& WebAppTabHelper::GetAudioFocusGroupIdForTesting()
+    const {
+  return audio_focus_group_id_;
+}
+
 void WebAppTabHelper::SetAppId(const AppId& app_id) {
   DCHECK(app_id.empty() || provider_->registrar().IsInstalled(app_id));
   if (app_id_ == app_id)
@@ -64,18 +90,7 @@ void WebAppTabHelper::DidCloneToNewWebContents(
   auto* new_tab_helper = FromWebContents(new_web_contents);
 
   // Clone common state:
-  new_tab_helper->SetAppId(app_id());
-}
-
-bool WebAppTabHelper::IsUserInstalled() const {
-  return !app_id_.empty() && provider_->registrar().WasInstalledByUser(app_id_);
-}
-
-bool WebAppTabHelper::IsFromInstallButton() const {
-  // TODO(loyso): Use something better to record apps installed from promoted
-  // UIs. crbug.com/774918.
-  return !app_id_.empty() &&
-         provider_->registrar().GetAppScope(app_id_).has_value();
+  new_tab_helper->SetAppId(GetAppId());
 }
 
 bool WebAppTabHelper::IsInAppWindow() const {
@@ -90,7 +105,7 @@ void WebAppTabHelper::OnWebAppInstalled(const AppId& installed_app_id) {
 }
 
 void WebAppTabHelper::OnWebAppUninstalled(const AppId& uninstalled_app_id) {
-  if (app_id() == uninstalled_app_id)
+  if (GetAppId() == uninstalled_app_id)
     ResetAppId();
 }
 

chrome/browser/web_applications/components/web_app_tab_helper.h → chrome/browser/web_applications/web_app_tab_helper.h

@@ -2,8 +2,8 @@
 // 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_WEB_APP_TAB_HELPER_H_
-#define CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_WEB_APP_TAB_HELPER_H_
+#ifndef CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_TAB_HELPER_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_TAB_HELPER_H_
 
 #include "base/macros.h"
 #include "base/scoped_observer.h"
@@ -11,8 +11,8 @@
 #include "chrome/browser/web_applications/components/app_registrar.h"
 #include "chrome/browser/web_applications/components/app_registrar_observer.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
+#include "chrome/browser/web_applications/components/web_app_tab_helper_base.h"
 #include "content/public/browser/web_contents_observer.h"
-#include "content/public/browser/web_contents_user_data.h"
 
 namespace content {
 class WebContents;
@@ -24,20 +24,21 @@ class WebAppProviderBase;
 
 // Per-tab web app helper. Allows to associate a tab (web page) with a web app
 // (or legacy bookmark app).
-class WebAppTabHelper : public content::WebContentsObserver,
-                        public content::WebContentsUserData<WebAppTabHelper>,
+class WebAppTabHelper : public WebAppTabHelperBase,
+                        public content::WebContentsObserver,
                         public AppRegistrarObserver {
  public:
-  using content::WebContentsUserData<WebAppTabHelper>::CreateForWebContents;
-  using content::WebContentsUserData<WebAppTabHelper>::FromWebContents;
+  static void CreateForWebContents(content::WebContents* contents);
 
   explicit WebAppTabHelper(content::WebContents* web_contents);
   ~WebAppTabHelper() override;
 
-  const AppId& app_id() const { return app_id_; }
-
-  // Set associated app_id.
-  void SetAppId(const AppId& app_id);
+  // WebAppTabHelperBase:
+  const AppId& GetAppId() const override;
+  void SetAppId(const AppId& app_id) override;
+  bool IsUserInstalled() const override;
+  bool IsFromInstallButton() const override;
+  const base::UnguessableToken& GetAudioFocusGroupIdForTesting() const override;
 
   // content::WebContentsObserver:
   void DidFinishNavigation(
@@ -46,18 +47,7 @@ class WebAppTabHelper : public content::WebContentsObserver,
       content::WebContents* old_web_contents,
       content::WebContents* new_web_contents) override;
 
-  // These methods require an app associated with the tab (valid app_id()).
-  //
-  // Returns true if the app was installed by user, false if default installed.
-  bool IsUserInstalled() const;
-  // For user-installed apps:
-  // Returns true if the app was installed through the install button.
-  // Returns false if the app was installed through the create shortcut button.
-  bool IsFromInstallButton() const;
-
  private:
-  WEB_CONTENTS_USER_DATA_KEY_DECL();
-
   friend class WebAppAudioFocusBrowserTest;
   friend class content::WebContentsUserData<WebAppTabHelper>;
 
@@ -98,4 +88,4 @@ class WebAppTabHelper : public content::WebContentsObserver,
 
 }  // namespace web_app
 
-#endif  // CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_WEB_APP_TAB_HELPER_H_
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_TAB_HELPER_H_