[Extensions] Implement a unit test to verify context menu persistence.

Bug: 1136568
Change-Id: I6e3e121e1d6c682c1465284755d4e518388cb970
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2551740
Commit-Queue: David Bertoni <dbertoni@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#830851}

chrome/browser/extensions/BUILD.gn

@@ -1280,6 +1280,8 @@ static_library("test_support") {
   sources = [
     "external_testing_loader.cc",
     "external_testing_loader.h",
+    "menu_manager_test_observer.cc",
+    "menu_manager_test_observer.h",
   ]
 
   deps = [

chrome/browser/extensions/extension_context_menu_browsertest.cc

@@ -16,6 +16,7 @@
 #include "chrome/browser/extensions/browsertest_util.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/lazy_background_page_test_util.h"
+#include "chrome/browser/extensions/menu_manager_test_observer.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_context_menu/render_view_context_menu.h"
 #include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h"
@@ -94,54 +95,6 @@ class StateStoreObserver : public StateStore::TestObserver {
   ScopedObserver<StateStore, StateStore::TestObserver> observed_{this};
 };
 
-// Observe when a extension's context menu data is read from storage.
-class MenuManagerObserver : public MenuManager::TestObserver {
- public:
-  explicit MenuManagerObserver(MenuManager* menu_manager)
-      : menu_manager_(menu_manager) {
-    observed_.Add(menu_manager_);
-  }
-
-  ~MenuManagerObserver() final = default;
-
-  void WaitForExtension(const std::string& extension_id) {
-    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-
-    // The extension's menus may have already been loaded before we were
-    // able to observe it.
-    if (MenusItemsFound(extension_id))
-      return;
-
-    if (ids_with_reads_.count(extension_id) == 0) {
-      waiting_for_id_ = extension_id;
-      run_loop_.Run();
-      DCHECK(MenusItemsFound(extension_id));
-    }
-  }
-
-  void DidReadFromStorage(const std::string& extension_id) override {
-    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-    if (extension_id == waiting_for_id_) {
-      run_loop_.Quit();
-    } else {
-      ids_with_reads_.insert(extension_id);
-    }
-  }
-
- private:
-  bool MenusItemsFound(const std::string& extension_id) {
-    const extensions::MenuItem::ExtensionKey key(extension_id);
-    return menu_manager_->MenuItems(key) &&
-           !menu_manager_->MenuItems(key)->empty();
-  }
-
-  MenuManager* const menu_manager_;
-  std::set<std::string> ids_with_reads_;
-  std::string waiting_for_id_;
-  base::RunLoop run_loop_;
-  ScopedObserver<MenuManager, MenuManager::TestObserver> observed_{this};
-};
-
 constexpr char kPersistentExtensionId[] = "cmgkkmeeoiceijkpmaabbmpgnkpaaela";
 
 }  // namespace
@@ -458,7 +411,7 @@ IN_PROC_BROWSER_TEST_P(ExtensionContextMenuLazyTest, PRE_Persistent) {
 }
 
 IN_PROC_BROWSER_TEST_P(ExtensionContextMenuLazyTest, Persistent) {
-  MenuManagerObserver observer(menu_manager());
+  extensions::MenuManagerTestObserver observer(menu_manager());
   ResultCatcher catcher;
 
   // Wait for the context menu to finish loading.

chrome/browser/extensions/menu_manager.cc

@@ -821,6 +821,9 @@ void MenuManager::WriteToStorage(const Extension* extension,
     }
   }
 
+  for (TestObserver& observer : observers_)
+    observer.WillWriteToStorage(extension->id());
+
   if (store_) {
     store_->SetExtensionValue(extension->id(), kContextMenusKey,
                               MenuItemsToValue(all_items));

chrome/browser/extensions/menu_manager.h

@@ -296,7 +296,8 @@ class MenuManager : public ProfileObserver,
   class TestObserver : public base::CheckedObserver {
    public:
     ~TestObserver() override = default;
-    virtual void DidReadFromStorage(const std::string& extension_id) = 0;
+    virtual void DidReadFromStorage(const std::string& extension_id) {}
+    virtual void WillWriteToStorage(const std::string& extension_id) {}
   };
 
   MenuManager(content::BrowserContext* context, StateStore* store_);

chrome/browser/extensions/menu_manager_test_observer.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 "chrome/browser/extensions/menu_manager_test_observer.h"
+
+#include "content/public/browser/browser_thread.h"
+
+namespace extensions {
+
+MenuManagerTestObserver::MenuManagerTestObserver(MenuManager* menu_manager)
+    : menu_manager_(menu_manager) {
+  observation_.Observe(menu_manager_);
+}
+
+MenuManagerTestObserver::~MenuManagerTestObserver() = default;
+
+void MenuManagerTestObserver::WaitForExtension(
+    const ExtensionId& extension_id) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // The extension's menus may have already been loaded before we were
+  // able to observe it.
+  if (MenusItemsFound(extension_id))
+    return;
+
+  if (ids_with_reads_.count(extension_id) == 0) {
+    waiting_for_id_ = extension_id;
+    run_loop_.Run();
+    DCHECK(MenusItemsFound(extension_id));
+  }
+}
+
+void MenuManagerTestObserver::DidReadFromStorage(
+    const ExtensionId& extension_id) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  ids_with_reads_.insert(extension_id);
+  if (extension_id == waiting_for_id_) {
+    run_loop_.Quit();
+  }
+}
+
+void MenuManagerTestObserver::WillWriteToStorage(
+    const ExtensionId& extension_id) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  ids_with_writes_.insert(extension_id);
+  if (extension_id == waiting_for_id_) {
+    run_loop_.Quit();
+  }
+}
+
+bool MenuManagerTestObserver::MenusItemsFound(const ExtensionId& extension_id) {
+  const MenuItem::ExtensionKey key(extension_id);
+  return menu_manager_->MenuItems(key) &&
+         !menu_manager_->MenuItems(key)->empty();
+}
+
+}  // namespace extensions

chrome/browser/extensions/menu_manager_test_observer.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 CHROME_BROWSER_EXTENSIONS_MENU_MANAGER_TEST_OBSERVER_H_
+#define CHROME_BROWSER_EXTENSIONS_MENU_MANAGER_TEST_OBSERVER_H_
+
+#include "chrome/browser/extensions/menu_manager.h"
+
+#include <set>
+#include <string>
+
+#include "base/run_loop.h"
+#include "base/scoped_observation.h"
+#include "extensions/common/extension_id.h"
+
+namespace extensions {
+
+class MenuManagerTestObserver : public MenuManager::TestObserver {
+ public:
+  explicit MenuManagerTestObserver(MenuManager* menu_manager);
+
+  ~MenuManagerTestObserver() override;
+
+  MenuManagerTestObserver(const MenuManagerTestObserver&) = delete;
+  MenuManagerTestObserver& operator=(const MenuManagerTestObserver&) = delete;
+
+  // MenuManager::TestObserver overrides.
+  void DidReadFromStorage(const ExtensionId& extension_id) override;
+  void WillWriteToStorage(const ExtensionId& extension_id) override;
+
+  // Wait for a MenuManager storage read or write for the specified
+  // extension.
+  void WaitForExtension(const ExtensionId& extension_id);
+
+  bool did_read_for_extension(const ExtensionId& extension_id) const {
+    return ids_with_reads_.count(extension_id) != 0;
+  }
+
+  bool will_write_for_extension(const ExtensionId& extension_id) const {
+    return ids_with_writes_.count(extension_id) != 0;
+  }
+
+ private:
+  bool MenusItemsFound(const ExtensionId& extension_id);
+
+  MenuManager* const menu_manager_;
+  std::set<ExtensionId> ids_with_reads_;
+  std::set<ExtensionId> ids_with_writes_;
+  ExtensionId waiting_for_id_;
+  base::RunLoop run_loop_;
+  base::ScopedObservation<MenuManager, MenuManager::TestObserver> observation_{
+      this};
+};
+
+}  // namespace extensions
+
+#endif  // CHROME_BROWSER_EXTENSIONS_MENU_MANAGER_TEST_OBSERVER_H_

chrome/browser/extensions/test_extension_prefs.cc

@@ -143,21 +143,17 @@ scoped_refptr<Extension> TestExtensionPrefs::AddExtension(
 
 scoped_refptr<Extension> TestExtensionPrefs::AddApp(const std::string& name) {
   base::DictionaryValue dictionary;
-  dictionary.SetString(manifest_keys::kName, name);
-  dictionary.SetString(manifest_keys::kVersion, "0.1");
+  AddDefaultManifestKeys(name, &dictionary);
   dictionary.SetString(manifest_keys::kApp, "true");
   dictionary.SetString(manifest_keys::kLaunchWebURL, "http://example.com");
   return AddExtensionWithManifest(dictionary, Manifest::INTERNAL);
-
 }
 
 scoped_refptr<Extension> TestExtensionPrefs::AddExtensionWithLocation(
     const std::string& name,
     Manifest::Location location) {
   base::DictionaryValue dictionary;
-  dictionary.SetString(manifest_keys::kName, name);
-  dictionary.SetString(manifest_keys::kVersion, "0.1");
-  dictionary.SetInteger(manifest_keys::kManifestVersion, 2);
+  AddDefaultManifestKeys(name, &dictionary);
   return AddExtensionWithManifest(dictionary, location);
 }
 
@@ -218,4 +214,12 @@ ChromeAppSorting* TestExtensionPrefs::app_sorting() {
       ExtensionSystem::Get(&profile_)->app_sorting());
 }
 
+void TestExtensionPrefs::AddDefaultManifestKeys(const std::string& name,
+                                                base::DictionaryValue* dict) {
+  DCHECK(dict);
+  dict->SetString(manifest_keys::kName, name);
+  dict->SetString(manifest_keys::kVersion, "0.1");
+  dict->SetInteger(manifest_keys::kManifestVersion, 2);
+}
+
 }  // namespace extensions

chrome/browser/extensions/test_extension_prefs.h

@@ -99,6 +99,9 @@ class TestExtensionPrefs {
 
   ChromeAppSorting* app_sorting();
 
+  static void AddDefaultManifestKeys(const std::string& name,
+                                     base::DictionaryValue* dict);
+
  protected:
   class IncrementalClock;
 

chrome/browser/extensions/test_extension_system.cc

@@ -43,6 +43,10 @@ namespace extensions {
 TestExtensionSystem::TestExtensionSystem(Profile* profile)
     : profile_(profile),
       store_factory_(new TestValueStoreFactory()),
+      state_store_(new StateStore(profile_,
+                                  store_factory_,
+                                  ValueStoreFrontend::BackendType::RULES,
+                                  false)),
       info_map_(new InfoMap()),
       quota_service_(new QuotaService()),
       app_sorting_(new ChromeAppSorting(profile_)) {
@@ -65,8 +69,6 @@ ExtensionService* TestExtensionSystem::CreateExtensionService(
     const base::FilePath& install_directory,
     bool autoupdate_enabled,
     bool extensions_enabled) {
-  state_store_.reset(new StateStore(
-      profile_, store_factory_, ValueStoreFrontend::BackendType::RULES, false));
   management_policy_.reset(new ManagementPolicy());
   management_policy_->RegisterProviders(
       ExtensionManagementFactory::GetForBrowserContext(profile_)

chrome/browser/extensions/test_extension_system.h

@@ -97,8 +97,9 @@ class TestExtensionSystem : public ExtensionSystem {
   Profile* profile_;
 
  private:
-  std::unique_ptr<StateStore> state_store_;
   scoped_refptr<TestValueStoreFactory> store_factory_;
+  // This depends on store_factory_.
+  std::unique_ptr<StateStore> state_store_;
   std::unique_ptr<ManagementPolicy> management_policy_;
   std::unique_ptr<RuntimeData> runtime_data_;
   std::unique_ptr<ExtensionService> extension_service_;