WebApp: Avoid using friend classes.

Background: All our app registry read/write access permissions
are enforced by the compiler. We used `friend` declarations before.

Introduce WebAppRegistrarMutable subtype for WebAppRegistrar.

While WebAppRegistrar serves as read-only model, WebAppRegistrarMutable
is a "view" which allows writable access for an owner which has
WebAppRegistrarMutable pointer.

WebAppSyncBridge is the only controller which has that writable access.

Bug: 860583
Change-Id: I4623ce826d25f0c2bbf9ed685a932d7eb734bdcd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1820497
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699248}

chrome/browser/web_applications/test/test_web_app_registry_controller.cc

@@ -7,7 +7,6 @@
 #include "base/run_loop.h"
 #include "base/test/bind_test_util.h"
 #include "chrome/browser/web_applications/test/test_web_app_database_factory.h"
-#include "chrome/browser/web_applications/web_app_registrar.h"
 #include "chrome/browser/web_applications/web_app_sync_bridge.h"
 
 namespace web_app {
@@ -18,10 +17,10 @@ TestWebAppRegistryController::~TestWebAppRegistryController() = default;
 
 void TestWebAppRegistryController::SetUp(Profile* profile) {
   database_factory_ = std::make_unique<TestWebAppDatabaseFactory>();
-  registrar_ = std::make_unique<WebAppRegistrar>(profile);
+  mutable_registrar_ = std::make_unique<WebAppRegistrarMutable>(profile);
 
   sync_bridge_ = std::make_unique<WebAppSyncBridge>(
-      profile, database_factory_.get(), registrar_.get(),
+      profile, database_factory_.get(), mutable_registrar_.get(),
       mock_processor_.CreateForwardingProcessor());
 
   ON_CALL(processor(), IsTrackingMetadata())
@@ -35,7 +34,7 @@ void TestWebAppRegistryController::Init() {
 }
 
 void TestWebAppRegistryController::DestroySubsystems() {
-  registrar_.reset();
+  mutable_registrar_.reset();
   sync_bridge_.reset();
   database_factory_.reset();
 }

chrome/browser/web_applications/test/test_web_app_registry_controller.h

@@ -7,6 +7,7 @@
 
 #include <memory>
 
+#include "chrome/browser/web_applications/web_app_registrar.h"
 #include "components/sync/model/mock_model_type_change_processor.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
@@ -15,7 +16,6 @@ class Profile;
 namespace web_app {
 
 class TestWebAppDatabaseFactory;
-class WebAppRegistrar;
 class WebAppSyncBridge;
 
 class TestWebAppRegistryController {
@@ -32,13 +32,14 @@ class TestWebAppRegistryController {
   void DestroySubsystems();
 
   TestWebAppDatabaseFactory& database_factory() { return *database_factory_; }
-  WebAppRegistrar& registrar() { return *registrar_; }
+  WebAppRegistrar& registrar() { return *mutable_registrar_; }
+  WebAppRegistrarMutable& mutable_registrar() { return *mutable_registrar_; }
   syncer::MockModelTypeChangeProcessor& processor() { return mock_processor_; }
   WebAppSyncBridge& sync_bridge() { return *sync_bridge_; }
 
  private:
   std::unique_ptr<TestWebAppDatabaseFactory> database_factory_;
-  std::unique_ptr<WebAppRegistrar> registrar_;
+  std::unique_ptr<WebAppRegistrarMutable> mutable_registrar_;
   testing::NiceMock<syncer::MockModelTypeChangeProcessor> mock_processor_;
   std::unique_ptr<WebAppSyncBridge> sync_bridge_;
 };

chrome/browser/web_applications/web_app_database_unittest.cc

@@ -10,11 +10,12 @@
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/test/bind_test_util.h"
-#include "base/test/task_environment.h"
 #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/proto/web_app.pb.h"
 #include "chrome/browser/web_applications/test/test_web_app_database_factory.h"
+#include "chrome/browser/web_applications/test/test_web_app_registry_controller.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_registrar.h"
 #include "chrome/browser/web_applications/web_app_sync_bridge.h"
@@ -24,21 +25,14 @@
 
 namespace web_app {
 
-class WebAppDatabaseTest : public testing::Test {
+class WebAppDatabaseTest : public WebAppTest {
  public:
-  WebAppDatabaseTest() {
-    database_factory_ = std::make_unique<TestWebAppDatabaseFactory>();
-    registrar_ = std::make_unique<WebAppRegistrar>(nullptr);
-    sync_bridge_ = std::make_unique<WebAppSyncBridge>(
-        nullptr, database_factory_.get(), registrar_.get());
-  }
-
-  void InitRegistrar() {
-    base::RunLoop run_loop;
-
-    sync_bridge().Init(base::BindLambdaForTesting([&]() { run_loop.Quit(); }));
+  void SetUp() override {
+    WebAppTest::SetUp();
 
-    run_loop.Run();
+    test_registry_controller_ =
+        std::make_unique<TestWebAppRegistryController>();
+    test_registry_controller_->SetUp(profile());
   }
 
   static std::unique_ptr<WebApp> CreateWebApp(const std::string& base_url,
@@ -91,14 +85,14 @@ class WebAppDatabaseTest : public testing::Test {
 
   bool IsDatabaseRegistryEqualToRegistrar() {
     Registry registry = database_factory().ReadRegistry();
-    return IsRegistryEqual(registrar_->registry_for_testing(), registry);
+    return IsRegistryEqual(mutable_registrar().registry(), registry);
   }
 
   void WriteBatch(
       std::unique_ptr<syncer::ModelTypeStore::WriteBatch> write_batch) {
     base::RunLoop run_loop;
 
-    database_factory_->store()->CommitWriteBatch(
+    database_factory().store()->CommitWriteBatch(
         std::move(write_batch),
         base::BindLambdaForTesting(
             [&](const base::Optional<syncer::ModelError>& error) {
@@ -112,7 +106,7 @@ class WebAppDatabaseTest : public testing::Test {
   Registry WriteWebApps(const std::string& base_url, int num_apps) {
     Registry registry;
 
-    auto write_batch = database_factory_->store()->CreateWriteBatch();
+    auto write_batch = database_factory().store()->CreateWriteBatch();
 
     for (int i = 0; i < num_apps; ++i) {
       auto app = CreateWebApp(base_url, i);
@@ -130,22 +124,32 @@ class WebAppDatabaseTest : public testing::Test {
   }
 
  protected:
-  TestWebAppDatabaseFactory& database_factory() { return *database_factory_; }
-  WebAppDatabase& database() { return sync_bridge().database_for_testing(); }
-  WebAppSyncBridge& sync_bridge() { return *sync_bridge_; }
-  WebAppRegistrar& registrar() { return *registrar_; }
+  TestWebAppRegistryController& controller() {
+    return *test_registry_controller_;
+  }
 
- private:
-  // Must be created before TestWebAppDatabaseFactory.
-  base::test::SingleThreadTaskEnvironment task_environment_;
+  TestWebAppDatabaseFactory& database_factory() {
+    return controller().database_factory();
+  }
 
-  std::unique_ptr<TestWebAppDatabaseFactory> database_factory_;
-  std::unique_ptr<WebAppSyncBridge> sync_bridge_;
-  std::unique_ptr<WebAppRegistrar> registrar_;
+  WebAppDatabase& database() {
+    return controller().sync_bridge().database_for_testing();
+  }
+
+  WebAppRegistrar& registrar() { return controller().registrar(); }
+
+  WebAppRegistrarMutable& mutable_registrar() {
+    return controller().mutable_registrar();
+  }
+
+  WebAppSyncBridge& sync_bridge() { return controller().sync_bridge(); }
+
+ private:
+  std::unique_ptr<TestWebAppRegistryController> test_registry_controller_;
 };
 
 TEST_F(WebAppDatabaseTest, WriteAndReadRegistry) {
-  InitRegistrar();
+  controller().Init();
   EXPECT_TRUE(registrar().is_empty());
 
   const int num_apps = 100;
@@ -170,7 +174,7 @@ TEST_F(WebAppDatabaseTest, WriteAndReadRegistry) {
 }
 
 TEST_F(WebAppDatabaseTest, WriteAndDeleteAppsWithCallbacks) {
-  InitRegistrar();
+  controller().Init();
   EXPECT_TRUE(registrar().is_empty());
 
   const int num_apps = 10;
@@ -217,12 +221,12 @@ TEST_F(WebAppDatabaseTest, WriteAndDeleteAppsWithCallbacks) {
 TEST_F(WebAppDatabaseTest, OpenDatabaseAndReadRegistry) {
   Registry registry = WriteWebApps("https://example.com/path", 100);
 
-  InitRegistrar();
-  EXPECT_TRUE(IsRegistryEqual(registrar().registry_for_testing(), registry));
+  controller().Init();
+  EXPECT_TRUE(IsRegistryEqual(mutable_registrar().registry(), registry));
 }
 
 TEST_F(WebAppDatabaseTest, WebAppWithoutOptionalFields) {
-  InitRegistrar();
+  controller().Init();
 
   const auto launch_url = GURL("https://example.com/");
   const AppId app_id = GenerateAppIdFromURL(GURL(launch_url));
@@ -276,7 +280,7 @@ TEST_F(WebAppDatabaseTest, WebAppWithoutOptionalFields) {
 }
 
 TEST_F(WebAppDatabaseTest, WebAppWithManyIcons) {
-  InitRegistrar();
+  controller().Init();
 
   const int num_icons = 32;
   const std::string base_url = "https://example.com/path";

chrome/browser/web_applications/web_app_provider.cc

@@ -161,15 +161,28 @@ void WebAppProvider::CreateCommonSubsystems(Profile* profile) {
 
 void WebAppProvider::CreateWebAppsSubsystems(Profile* profile) {
   database_factory_ = std::make_unique<WebAppDatabaseFactory>(profile);
-  auto registrar = std::make_unique<WebAppRegistrar>(profile);
-  auto sync_bridge = std::make_unique<WebAppSyncBridge>(
-      profile, database_factory_.get(), registrar.get());
+
+  std::unique_ptr<WebAppRegistrar> registrar;
+  std::unique_ptr<WebAppSyncBridge> sync_bridge;
+
+  // Only WebAppSyncBridge must have an access to mutable WebAppRegistrar.
+  {
+    auto mutable_registrar = std::make_unique<WebAppRegistrarMutable>(profile);
+
+    sync_bridge = std::make_unique<WebAppSyncBridge>(
+        profile, database_factory_.get(), mutable_registrar.get());
+
+    // Upcast to read-only WebAppRegistrar.
+    registrar = std::move(mutable_registrar);
+  }
+
   auto icon_manager = std::make_unique<WebAppIconManager>(
       profile, *registrar, std::make_unique<FileUtilsWrapper>());
   install_finalizer_ = std::make_unique<WebAppInstallFinalizer>(
       sync_bridge.get(), icon_manager.get());
   file_handler_manager_ = std::make_unique<WebAppFileHandlerManager>(profile);
 
+  // Upcast to unified subsystem types:
   registrar_ = std::move(registrar);
   registry_controller_ = std::move(sync_bridge);
   icon_manager_ = std::move(icon_manager);

chrome/browser/web_applications/web_app_registrar.cc

@@ -25,11 +25,6 @@ const WebApp* WebAppRegistrar::GetAppById(const AppId& app_id) const {
   return it == registry_.end() ? nullptr : it->second.get();
 }
 
-void WebAppRegistrar::InitRegistry(Registry&& registry) {
-  DCHECK(is_empty());
-  registry_ = std::move(registry);
-}
-
 bool WebAppRegistrar::IsInstalled(const AppId& app_id) const {
   return GetAppById(app_id) != nullptr;
 }
@@ -162,12 +157,8 @@ const WebAppRegistrar::AppSet WebAppRegistrar::AllApps() const {
   return AppSet(this);
 }
 
-WebApp* WebAppRegistrar::GetAppByIdMutable(const AppId& app_id) {
-  return const_cast<WebApp*>(GetAppById(app_id));
-}
-
-WebAppRegistrar::AppSet WebAppRegistrar::AllAppsMutable() {
-  return AppSet(this);
+void WebAppRegistrar::SetRegistry(Registry&& registry) {
+  registry_ = std::move(registry);
 }
 
 void WebAppRegistrar::CountMutation() {
@@ -176,6 +167,24 @@ void WebAppRegistrar::CountMutation() {
 #endif
 }
 
+WebAppRegistrarMutable::WebAppRegistrarMutable(Profile* profile)
+    : WebAppRegistrar(profile) {}
+
+WebAppRegistrarMutable::~WebAppRegistrarMutable() = default;
+
+void WebAppRegistrarMutable::InitRegistry(Registry&& registry) {
+  DCHECK(is_empty());
+  SetRegistry(std::move(registry));
+}
+
+WebApp* WebAppRegistrarMutable::GetAppByIdMutable(const AppId& app_id) {
+  return const_cast<WebApp*>(GetAppById(app_id));
+}
+
+WebAppRegistrar::AppSet WebAppRegistrarMutable::AllAppsMutable() {
+  return AppSet(this);
+}
+
 bool IsRegistryEqual(const Registry& registry, const Registry& registry2) {
   if (registry.size() != registry2.size())
     return false;

chrome/browser/web_applications/web_app_registrar.h

@@ -9,7 +9,6 @@
 #include <memory>
 #include <string>
 
-#include "base/gtest_prod_util.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/optional.h"
@@ -22,7 +21,7 @@ class WebApp;
 
 using Registry = std::map<AppId, std::unique_ptr<WebApp>>;
 
-// A registry. This is a read-only container, which owns WebApp objects.
+// A registry model. This is a read-only container, which owns WebApp objects.
 class WebAppRegistrar : public AppRegistrar {
  public:
   explicit WebAppRegistrar(Profile* profile);
@@ -95,31 +94,36 @@ class WebAppRegistrar : public AppRegistrar {
 
   const AppSet AllApps() const;
 
-  const Registry& registry_for_testing() const { return registry_; }
-
- private:
-  friend class WebAppSyncBridge;
-  FRIEND_TEST_ALL_PREFIXES(WebAppRegistrarTest, AllAppsMutable);
-
-  WebApp* GetAppByIdMutable(const AppId& app_id);
-  AppSet AllAppsMutable();
-
-  void InitRegistry(Registry&& registry);
+ protected:
+  Registry& registry() { return registry_; }
+  void SetRegistry(Registry&& registry);
 
   void CountMutation();
 
-  // Used by WebAppSyncBridge controller to to raw updates.
-  Registry& registry() { return registry_; }
-
+ private:
   Registry registry_;
-
 #if DCHECK_IS_ON()
   size_t mutations_count_ = 0;
 #endif
-
   DISALLOW_COPY_AND_ASSIGN(WebAppRegistrar);
 };
 
+// A writable API for the registry model. Mutable WebAppRegistrar must be used
+// only by WebAppSyncBridge.
+class WebAppRegistrarMutable : public WebAppRegistrar {
+ public:
+  explicit WebAppRegistrarMutable(Profile* profile);
+  ~WebAppRegistrarMutable() override;
+
+  void InitRegistry(Registry&& registry);
+
+  WebApp* GetAppByIdMutable(const AppId& app_id);
+  AppSet AllAppsMutable();
+
+  using WebAppRegistrar::CountMutation;
+  using WebAppRegistrar::registry;
+};
+
 // For testing and debug purposes.
 bool IsRegistryEqual(const Registry& registry, const Registry& registry2);
 

chrome/browser/web_applications/web_app_registrar_unittest.cc

@@ -237,7 +237,7 @@ TEST_F(WebAppRegistrarTest, InitRegistrarAndDoForEachApp) {
 TEST_F(WebAppRegistrarTest, AllAppsMutable) {
   std::set<AppId> ids = InitRegistrarWithApps("https://example.com/path", 10);
 
-  for (WebApp& web_app : registrar().AllAppsMutable()) {
+  for (WebApp& web_app : controller().mutable_registrar().AllAppsMutable()) {
     web_app.SetLaunchContainer(LaunchContainer::kWindow);
     const size_t num_removed = ids.erase(web_app.app_id());
     EXPECT_EQ(1U, num_removed);
@@ -274,11 +274,11 @@ TEST_F(WebAppRegistrarTest, WebAppSyncBridge) {
   sync_bridge().RegisterApp(std::move(web_app));
 
   EXPECT_EQ(101UL, database_factory().ReadAllAppIds().size());
-  EXPECT_EQ(101UL, registrar().registry_for_testing().size());
+  EXPECT_EQ(101UL, controller().mutable_registrar().registry().size());
 
   // Remove 1 app after Init.
   sync_bridge().UnregisterApp(app_id);
-  EXPECT_EQ(100UL, registrar().registry_for_testing().size());
+  EXPECT_EQ(100UL, controller().mutable_registrar().registry().size());
   EXPECT_EQ(100UL, database_factory().ReadAllAppIds().size());
 
   // Remove 100 apps after Init.

chrome/browser/web_applications/web_app_registry_update.cc

@@ -5,28 +5,36 @@
 #include "chrome/browser/web_applications/web_app_registry_update.h"
 
 #include "base/bind_helpers.h"
+#include "chrome/browser/web_applications/web_app_registrar.h"
 #include "chrome/browser/web_applications/web_app_sync_bridge.h"
 
 namespace web_app {
 
-WebAppRegistryUpdate::WebAppRegistryUpdate(WebAppSyncBridge* sync_bridge)
-    : sync_bridge_(sync_bridge) {}
+WebAppRegistryUpdate::WebAppRegistryUpdate(
+    WebAppRegistrarMutable* mutable_registrar)
+    : mutable_registrar_(mutable_registrar) {}
 
 WebAppRegistryUpdate::~WebAppRegistryUpdate() = default;
 
 WebApp* WebAppRegistryUpdate::UpdateApp(const AppId& app_id) {
-  WebApp* app = sync_bridge_->GetAppByIdMutable(app_id);
+  WebApp* app = mutable_registrar_->GetAppByIdMutable(app_id);
   if (app)
     apps_to_update_.insert(app);
 
   return app;
 }
 
+WebAppRegistryUpdate::AppsToUpdate WebAppRegistryUpdate::TakeAppsToUpdate() {
+  AppsToUpdate apps_to_update = std::move(apps_to_update_);
+  apps_to_update_.clear();
+  return apps_to_update;
+}
+
 ScopedRegistryUpdate::ScopedRegistryUpdate(WebAppSyncBridge* sync_bridge)
-    : update_(sync_bridge->BeginUpdate()) {}
+    : update_(sync_bridge->BeginUpdate()), sync_bridge_(sync_bridge) {}
 
 ScopedRegistryUpdate::~ScopedRegistryUpdate() {
-  update_->sync_bridge_->CommitUpdate(std::move(update_), base::DoNothing());
+  sync_bridge_->CommitUpdate(std::move(update_), base::DoNothing());
 }
 
 }  // namespace web_app

chrome/browser/web_applications/web_app_registry_update.h

@@ -15,28 +15,29 @@
 namespace web_app {
 
 class WebApp;
+class WebAppRegistrarMutable;
 class WebAppSyncBridge;
 
 // An explicit writable "view" for the registry. Any write operations must be
-// batched as a part of WebAppRegistryUpdate object.
+// batched as a part of WebAppRegistryUpdate object. Effectively
+// WebAppRegistryUpdate is a part of WebAppSyncBridge class.
 //
 // TODO(loyso): Support creation and deletion of apps as a part of single
 // update (As in Database CRUD: Create/Update/Delete).
 class WebAppRegistryUpdate {
  public:
+  explicit WebAppRegistryUpdate(WebAppRegistrarMutable* mutable_registrar);
   ~WebAppRegistryUpdate();
 
   // Acquire a mutable app object to set new field values.
   WebApp* UpdateApp(const AppId& app_id);
 
- private:
-  friend class WebAppSyncBridge;
-  friend class ScopedRegistryUpdate;
-
-  explicit WebAppRegistryUpdate(WebAppSyncBridge* sync_bridge);
+  using AppsToUpdate = base::flat_set<const WebApp*>;
+  AppsToUpdate TakeAppsToUpdate();
 
-  base::flat_set<const WebApp*> apps_to_update_;
-  WebAppSyncBridge* sync_bridge_;
+ private:
+  AppsToUpdate apps_to_update_;
+  WebAppRegistrarMutable* mutable_registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(WebAppRegistryUpdate);
 };
@@ -52,6 +53,7 @@ class ScopedRegistryUpdate {
 
  private:
   std::unique_ptr<WebAppRegistryUpdate> update_;
+  WebAppSyncBridge* sync_bridge_;
 
   DISALLOW_COPY_AND_ASSIGN(ScopedRegistryUpdate);
 };

chrome/browser/web_applications/web_app_sync_bridge.cc

@@ -24,7 +24,7 @@ namespace web_app {
 WebAppSyncBridge::WebAppSyncBridge(
     Profile* profile,
     AbstractWebAppDatabaseFactory* database_factory,
-    WebAppRegistrar* registrar)
+    WebAppRegistrarMutable* registrar)
     : WebAppSyncBridge(
           profile,
           database_factory,
@@ -37,7 +37,7 @@ WebAppSyncBridge::WebAppSyncBridge(
 WebAppSyncBridge::WebAppSyncBridge(
     Profile* profile,
     AbstractWebAppDatabaseFactory* database_factory,
-    WebAppRegistrar* registrar,
+    WebAppRegistrarMutable* registrar,
     std::unique_ptr<syncer::ModelTypeChangeProcessor> change_processor)
     : AppRegistryController(profile),
       syncer::ModelTypeSyncBridge(std::move(change_processor)),
@@ -74,7 +74,7 @@ std::unique_ptr<WebApp> WebAppSyncBridge::UnregisterApp(const AppId& app_id) {
   DCHECK(it != registrar_->registry().end());
 
   auto web_app = std::move(it->second);
-  registrar_->registry_.erase(it);
+  registrar_->registry().erase(it);
   return web_app;
 }
 
@@ -90,7 +90,7 @@ std::unique_ptr<WebAppRegistryUpdate> WebAppSyncBridge::BeginUpdate() {
   DCHECK(!is_in_update_);
   is_in_update_ = true;
 
-  return base::WrapUnique(new WebAppRegistryUpdate(this));
+  return base::WrapUnique(new WebAppRegistryUpdate(registrar_));
 }
 
 void WebAppSyncBridge::CommitUpdate(
@@ -98,18 +98,27 @@ void WebAppSyncBridge::CommitUpdate(
     CommitCallback callback) {
   DCHECK(is_in_update_);
   is_in_update_ = false;
-  if (update == nullptr || update->apps_to_update_.empty()) {
+
+  if (update == nullptr) {
+    std::move(callback).Run(/*success*/ true);
+    return;
+  }
+
+  WebAppRegistryUpdate::AppsToUpdate apps_to_update =
+      update->TakeAppsToUpdate();
+
+  if (apps_to_update.empty()) {
     std::move(callback).Run(/*success*/ true);
     return;
   }
 
 #if DCHECK_IS_ON()
-  for (auto* app : update->apps_to_update_)
+  for (auto* app : apps_to_update)
     DCHECK(registrar_->GetAppById(app->app_id()));
 #endif
 
   database_->WriteWebApps(
-      std::move(update->apps_to_update_),
+      std::move(apps_to_update),
       base::BindOnce(&WebAppSyncBridge::OnDataWritten,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
@@ -132,10 +141,6 @@ WebAppSyncBridge* WebAppSyncBridge::AsWebAppSyncBridge() {
   return this;
 }
 
-WebApp* WebAppSyncBridge::GetAppByIdMutable(const AppId& app_id) {
-  return registrar_->GetAppByIdMutable(app_id);
-}
-
 void WebAppSyncBridge::OnDatabaseOpened(base::OnceClosure callback,
                                         Registry registry) {
   registrar_->InitRegistry(std::move(registry));

chrome/browser/web_applications/web_app_sync_bridge.h

@@ -35,12 +35,12 @@ class WebAppSyncBridge : public AppRegistryController,
  public:
   WebAppSyncBridge(Profile* profile,
                    AbstractWebAppDatabaseFactory* database_factory,
-                   WebAppRegistrar* registrar);
+                   WebAppRegistrarMutable* registrar);
   // Tests may inject mocks using this ctor.
   WebAppSyncBridge(
       Profile* profile,
       AbstractWebAppDatabaseFactory* database_factory,
-      WebAppRegistrar* registrar,
+      WebAppRegistrarMutable* registrar,
       std::unique_ptr<syncer::ModelTypeChangeProcessor> change_processor);
   ~WebAppSyncBridge() override;
 
@@ -66,10 +66,6 @@ class WebAppSyncBridge : public AppRegistryController,
   WebAppDatabase& database_for_testing() { return *database_; }
 
  private:
-  friend class WebAppRegistryUpdate;
-
-  WebApp* GetAppByIdMutable(const AppId& app_id);
-
   void OnDatabaseOpened(base::OnceClosure callback, Registry registry);
   void OnDataWritten(CommitCallback callback, bool success);
 
@@ -88,7 +84,7 @@ class WebAppSyncBridge : public AppRegistryController,
   std::string GetStorageKey(const syncer::EntityData& entity_data) override;
 
   std::unique_ptr<WebAppDatabase> database_;
-  WebAppRegistrar* registrar_;
+  WebAppRegistrarMutable* registrar_;
 
   bool is_in_update_ = false;