Eliminate implementation from AppBannerManager::Observer

Make it a pure virtual interface. Per the style guide, classes should
have no more than one superclass that actually provides implementation.
This fixes BrowserView in that regard. This is also necessary for
upcoming ScopedObserver updates.

Bug: 998625
Change-Id: I387e2ef98af0035023a72853b10cc5fba8db1df4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1828280Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700822}

chrome/browser/banners/app_banner_manager.cc

@@ -47,16 +47,6 @@ InstallableParams ParamsToGetManifest() {
 
 namespace banners {
 
-AppBannerManager::Observer::Observer() = default;
-AppBannerManager::Observer::~Observer() = default;
-
-void AppBannerManager::Observer::ObserveAppBannerManager(
-    AppBannerManager* manager) {
-  scoped_observer_.RemoveAll();
-  if (manager)
-    scoped_observer_.Add(manager);
-}
-
 // static
 base::Time AppBannerManager::GetCurrentTime() {
   return base::Time::Now() +
@@ -216,7 +206,7 @@ void AppBannerManager::MigrateObserverListForTesting(
     content::WebContents* web_contents) {
   AppBannerManager* existing_manager = FromWebContents(web_contents);
   for (Observer& observer : existing_manager->observer_list_)
-    observer.ObserveAppBannerManager(this);
+    observer.OnAppBannerManagerChanged(this);
   DCHECK(existing_manager->observer_list_.begin() ==
          existing_manager->observer_list_.end())
       << "Old observer list must be empty after transfer to test instance.";
@@ -248,11 +238,7 @@ AppBannerManager::AppBannerManager(content::WebContents* web_contents)
   AppBannerSettingsHelper::UpdateFromFieldTrial();
 }
 
-AppBannerManager::~AppBannerManager() {
-  for (Observer& observer : observer_list_)
-    observer.ObserveAppBannerManager(nullptr);
-  CHECK(!observer_list_.might_have_observers());
-}
+AppBannerManager::~AppBannerManager() = default;
 
 bool AppBannerManager::CheckIfShouldShowBanner() {
   if (ShouldBypassEngagementChecks())

chrome/browser/banners/app_banner_manager.h

@@ -12,7 +12,6 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
-#include "base/scoped_observer.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/engagement/site_engagement_observer.h"
 #include "chrome/browser/installable/installable_logging.h"
@@ -53,15 +52,8 @@ class AppBannerManager : public content::WebContentsObserver,
  public:
   class Observer : public base::CheckedObserver {
    public:
-    Observer();
-    ~Observer() override;
-
-    void ObserveAppBannerManager(AppBannerManager* manager);
-
+    virtual void OnAppBannerManagerChanged(AppBannerManager* new_manager) = 0;
     virtual void OnInstallableWebAppStatusUpdated() = 0;
-
-   private:
-    ScopedObserver<AppBannerManager, Observer> scoped_observer_{this};
   };
 
   // A StatusReporter handles the reporting of |InstallableStatusCode|s.

chrome/browser/ui/views/frame/browser_view.cc

@@ -876,7 +876,7 @@ void BrowserView::OnActiveTabChanged(content::WebContents* old_contents,
   infobar_container_->ChangeInfoBarManager(
       InfoBarService::FromWebContents(new_contents));
 
-  ObserveAppBannerManager(
+  OnAppBannerManagerChanged(
       banners::AppBannerManager::FromWebContents(new_contents));
 
   UpdateUIForContents(new_contents);
@@ -933,7 +933,7 @@ void BrowserView::OnTabDetached(content::WebContents* contents,
     web_contents_close_handler_->ActiveTabChanged();
     contents_web_view_->SetWebContents(nullptr);
     infobar_container_->ChangeInfoBarManager(nullptr);
-    ObserveAppBannerManager(nullptr);
+    app_banner_manager_observer_.RemoveAll();
     UpdateDevToolsForContents(nullptr, true);
   }
 }
@@ -3250,6 +3250,12 @@ void BrowserView::OnImmersiveModeControllerDestroyed() {
 
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserView, banners::AppBannerManager::Observer implementation:
+void BrowserView::OnAppBannerManagerChanged(
+    banners::AppBannerManager* new_manager) {
+  app_banner_manager_observer_.RemoveAll();
+  app_banner_manager_observer_.Add(new_manager);
+}
+
 void BrowserView::OnInstallableWebAppStatusUpdated() {
   UpdatePageActionIcon(PageActionIconType::kPwaInstall);
 }

chrome/browser/ui/views/frame/browser_view.h

@@ -540,6 +540,8 @@ class BrowserView : public BrowserWindow,
   void OnImmersiveModeControllerDestroyed() override;
 
   // banners::AppBannerManager::Observer:
+  void OnAppBannerManagerChanged(
+      banners::AppBannerManager* new_manager) override;
   void OnInstallableWebAppStatusUpdated() override;
 
   // Creates an accessible tab label for screen readers that includes the tab
@@ -846,6 +848,9 @@ class BrowserView : public BrowserWindow,
 
   ReopenTabPromoController reopen_tab_promo_controller_{this};
 
+  ScopedObserver<banners::AppBannerManager, banners::AppBannerManager::Observer>
+      app_banner_manager_observer_{this};
+
   struct ResizeSession {
     // The time when user started resizing the window.
     base::TimeTicks begin_timestamp;