WebApp: Make sure that UI Manager works in the provider ready scope.

NotifyOnAllAppWindowsClosed API should not cause notifications
before Start or after Shutdown.

Bug: 1084939
Change-Id: I18b1299bd0cc1115a7024bd6ac8fc37a02a3e30a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2214459Reviewed-by: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771510}

chrome/browser/ui/web_applications/web_app_ui_manager_impl.cc

@@ -40,7 +40,16 @@ WebAppUiManagerImpl* WebAppUiManagerImpl::Get(Profile* profile) {
   return provider ? provider->ui_manager().AsImpl() : nullptr;
 }
 
-WebAppUiManagerImpl::WebAppUiManagerImpl(Profile* profile) : profile_(profile) {
+WebAppUiManagerImpl::WebAppUiManagerImpl(Profile* profile)
+    : dialog_manager_(std::make_unique<WebAppDialogManager>(profile)),
+      profile_(profile) {}
+
+WebAppUiManagerImpl::~WebAppUiManagerImpl() = default;
+
+void WebAppUiManagerImpl::Start() {
+  DCHECK(!started_);
+  started_ = true;
+
   for (Browser* browser : *BrowserList::GetInstance()) {
     if (!IsBrowserForInstalledApp(browser))
       continue;
@@ -49,12 +58,11 @@ WebAppUiManagerImpl::WebAppUiManagerImpl(Profile* profile) : profile_(profile) {
   }
 
   BrowserList::AddObserver(this);
-
-  dialog_manager_ = std::make_unique<WebAppDialogManager>(profile);
 }
 
-WebAppUiManagerImpl::~WebAppUiManagerImpl() {
+void WebAppUiManagerImpl::Shutdown() {
   BrowserList::RemoveObserver(this);
+  started_ = false;
 }
 
 WebAppDialogManager& WebAppUiManagerImpl::dialog_manager() {
@@ -66,6 +74,8 @@ WebAppUiManagerImpl* WebAppUiManagerImpl::AsImpl() {
 }
 
 size_t WebAppUiManagerImpl::GetNumWindowsForApp(const AppId& app_id) {
+  DCHECK(started_);
+
   auto it = num_windows_for_apps_map_.find(app_id);
   if (it == num_windows_for_apps_map_.end())
     return 0;
@@ -76,6 +86,8 @@ size_t WebAppUiManagerImpl::GetNumWindowsForApp(const AppId& app_id) {
 void WebAppUiManagerImpl::NotifyOnAllAppWindowsClosed(
     const AppId& app_id,
     base::OnceClosure callback) {
+  DCHECK(started_);
+
   const size_t num_windows_for_app = GetNumWindowsForApp(app_id);
   if (num_windows_for_app == 0) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
@@ -167,17 +179,17 @@ void WebAppUiManagerImpl::ReparentAppTabToWindow(content::WebContents* contents,
 }
 
 void WebAppUiManagerImpl::OnBrowserAdded(Browser* browser) {
-  if (!IsBrowserForInstalledApp(browser)) {
+  DCHECK(started_);
+  if (!IsBrowserForInstalledApp(browser))
     return;
-  }
 
   ++num_windows_for_apps_map_[GetAppIdForBrowser(browser)];
 }
 
 void WebAppUiManagerImpl::OnBrowserRemoved(Browser* browser) {
-  if (!IsBrowserForInstalledApp(browser)) {
+  DCHECK(started_);
+  if (!IsBrowserForInstalledApp(browser))
     return;
-  }
 
   const auto& app_id = GetAppIdForBrowser(browser);
 

chrome/browser/ui/web_applications/web_app_ui_manager_impl.h

@@ -34,6 +34,9 @@ class WebAppUiManagerImpl : public BrowserListObserver, public WebAppUiManager {
   explicit WebAppUiManagerImpl(Profile* profile);
   ~WebAppUiManagerImpl() override;
 
+  void Start() override;
+  void Shutdown() override;
+
   WebAppDialogManager& dialog_manager();
 
   // WebAppUiManager:
@@ -73,6 +76,7 @@ class WebAppUiManagerImpl : public BrowserListObserver, public WebAppUiManager {
 
   std::map<AppId, std::vector<base::OnceClosure>> windows_closed_requests_map_;
   std::map<AppId, size_t> num_windows_for_apps_map_;
+  bool started_ = false;
 
   base::WeakPtrFactory<WebAppUiManagerImpl> weak_ptr_factory_{this};
 

chrome/browser/web_applications/components/web_app_ui_manager.h

@@ -30,6 +30,9 @@ class WebAppUiManager {
 
   virtual ~WebAppUiManager() = default;
 
+  virtual void Start() = 0;
+  virtual void Shutdown() = 0;
+
   // A safe downcast.
   virtual WebAppUiManagerImpl* AsImpl() = 0;
 

chrome/browser/web_applications/test/test_web_app_ui_manager.cc

@@ -17,6 +17,10 @@ TestWebAppUiManager::TestWebAppUiManager() = default;
 
 TestWebAppUiManager::~TestWebAppUiManager() = default;
 
+void TestWebAppUiManager::Start() {}
+
+void TestWebAppUiManager::Shutdown() {}
+
 void TestWebAppUiManager::SetNumWindowsForApp(const AppId& app_id,
                                               size_t num_windows_for_app) {
   app_id_to_num_windows_map_[app_id] = num_windows_for_app;

chrome/browser/web_applications/test/test_web_app_ui_manager.h

@@ -17,6 +17,9 @@ class TestWebAppUiManager : public WebAppUiManager {
   TestWebAppUiManager();
   ~TestWebAppUiManager() override;
 
+  void Start() override;
+  void Shutdown() override;
+
   void SetNumWindowsForApp(const AppId& app_id, size_t num_windows_for_app);
   bool DidUninstallAndReplace(const AppId& from_app, const AppId& to_app);
 

chrome/browser/web_applications/web_app_install_finalizer.cc

@@ -329,11 +329,7 @@ bool WebAppInstallFinalizer::WasExternalAppUninstalledByUser(
 void WebAppInstallFinalizer::FinalizeUpdate(
     const WebApplicationInfo& web_app_info,
     InstallFinalizedCallback callback) {
-  if (!started_) {
-    std::move(callback).Run(AppId(),
-                            InstallResultCode::kWebAppProviderNotReady);
-    return;
-  }
+  CHECK(started_);
 
   const AppId app_id = GenerateAppIdFromURL(web_app_info.app_url);
   const WebApp* existing_web_app = GetWebAppRegistrar().GetAppById(app_id);

chrome/browser/web_applications/web_app_provider.cc

@@ -149,6 +149,7 @@ SystemWebAppManager& WebAppProvider::system_web_app_manager() {
 }
 
 void WebAppProvider::Shutdown() {
+  ui_manager_->Shutdown();
   shortcut_manager_->Shutdown();
   pending_app_manager_->Shutdown();
   install_manager_->Shutdown();
@@ -277,6 +278,7 @@ void WebAppProvider::OnRegistryControllerReady() {
   shortcut_manager_->Start();
   manifest_update_manager_->Start();
   file_handler_manager_->Start();
+  ui_manager_->Start();
 
   on_registry_ready_.Signal();
 }