[iOS] Create WebStateList observation registrar.

This CL creates AllWebStateListObservationRegistrar, which can be used
in any case when an object needs to observe all web state lists for a
given BrowserState.

This is a generalized version of the snapshot cache tab model observer,
which this replaces.

For cases where only regular or OTR browsers' WebStateLists should be
observed, AllWebStateListObservationRegistrar has an optional param that
can be set at creation time.

To avoid circular dependencies, one target in web_state_list/ had to
be split from the main target (hence the scattershot BUILD.gn updates).

Change-Id: Ic3977b709d5b089ebf31afd0fa868067833f2249
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1997306
Commit-Queue: Mark Cogan <marq@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742172}

ios/chrome/browser/main/BUILD.gn

@@ -4,6 +4,7 @@
 
 source_set("public") {
   sources = [
+    "all_web_state_list_observation_registrar.h",
     "browser.h",
     "browser_list.h",
     "browser_list_factory.h",
@@ -16,12 +17,14 @@ source_set("public") {
     "//components/keyed_service/core",
     "//components/keyed_service/ios",
     "//ios/chrome/browser/browser_state",
+    "//ios/chrome/browser/web_state_list",
   ]
   configs += [ "//build/config/compiler:enable_arc" ]
 }
 
 source_set("main") {
   sources = [
+    "all_web_state_list_observation_registrar.mm",
     "browser_agent_util.h",
     "browser_agent_util.mm",
     "browser_impl.h",
@@ -48,6 +51,7 @@ source_set("main") {
     "//ios/chrome/browser/tabs",
     "//ios/chrome/browser/ui/commands",
     "//ios/chrome/browser/web_state_list",
+    "//ios/chrome/browser/web_state_list:agents",
     "//ios/public/provider/chrome/browser",
   ]
   configs += [ "//build/config/compiler:enable_arc" ]
@@ -79,6 +83,7 @@ source_set("test_support") {
 source_set("unit_tests") {
   testonly = true
   sources = [
+    "all_web_state_list_observation_registrar_unittest.mm",
     "browser_impl_unittest.mm",
     "browser_list_impl_unittest.mm",
   ]

ios/chrome/browser/main/all_web_state_list_observation_registrar.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 IOS_CHROME_BROWSER_MAIN_ALL_WEB_STATE_LIST_OBSERVATION_REGISTRAR_H_
+#define IOS_CHROME_BROWSER_MAIN_ALL_WEB_STATE_LIST_OBSERVATION_REGISTRAR_H_
+
+#include <memory>
+
+#include "base/scoped_observer.h"
+#include "ios/chrome/browser/main/browser_list_observer.h"
+#include "ios/chrome/browser/web_state_list/web_state_list.h"
+#include "ios/chrome/browser/web_state_list/web_state_list_observer.h"
+
+class BrowserList;
+class ChromeBrowserState;
+
+// AllWebStateListObservationRegistrar tracks when Browsers are created and
+// destroyed for a given ChromeBrowserState. Whenever the BrowserList changes,
+// AllWebStateListObservationRegistrar registers (or unregisters) a provided
+// observer as a WebStateListObserver.
+class AllWebStateListObservationRegistrar : public BrowserListObserver {
+ public:
+  // Observation mode optionally used for constructors.
+  enum Mode {
+    REGULAR = 1 << 0,          // Only register regular web states.
+    INCOGNITO = 1 << 1,        // Only register incognito web states.
+    ALL = REGULAR | INCOGNITO  // Register all web states.
+  };
+  // Constructs an object that registers the given |web_state_list_observer| as
+  // a WebStateListObserver for any Browsers associated with |browser_state| or
+  // |browser_state|'s OTR browser state, according to the value of |mode|.
+  // Keeps observer registration up to date as Browsers are added and
+  // removed from |browser_state|'s BrowserList.
+  AllWebStateListObservationRegistrar(
+      ChromeBrowserState* browser_state,
+      std::unique_ptr<WebStateListObserver> web_state_list_observer,
+      Mode mode);
+
+  // Convenience constructor; creates a registrar as described above, with a
+  // |mode| of ALL.
+  AllWebStateListObservationRegistrar(
+      ChromeBrowserState* browser_state,
+      std::unique_ptr<WebStateListObserver> web_state_list_observer);
+
+  // Not copyable or moveable
+  AllWebStateListObservationRegistrar(
+      const AllWebStateListObservationRegistrar&) = delete;
+  AllWebStateListObservationRegistrar& operator=(
+      const AllWebStateListObservationRegistrar&) = delete;
+
+  ~AllWebStateListObservationRegistrar() override;
+
+  // BrowserListObserver
+  void OnBrowserAdded(const BrowserList* browser_list,
+                      Browser* browser) override;
+  void OnIncognitoBrowserAdded(const BrowserList* browser_list,
+                               Browser* browser) override;
+  void OnBrowserRemoved(const BrowserList* browser_list,
+                        Browser* browser) override;
+  void OnIncognitoBrowserRemoved(const BrowserList* browser_list,
+                                 Browser* browser) override;
+  void OnBrowserListShutdown(BrowserList* browser_list) override;
+
+ private:
+  BrowserList* browser_list_;
+  std::unique_ptr<WebStateListObserver> web_state_list_observer_;
+  std::unique_ptr<ScopedObserver<WebStateList, WebStateListObserver>>
+      scoped_observer_;
+  Mode mode_;
+};
+
+#endif  // IOS_CHROME_BROWSER_MAIN_ALL_WEB_STATE_LIST_OBSERVATION_REGISTRAR_H_

ios/chrome/browser/main/all_web_state_list_observation_registrar.mm

+// 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.
+
+#import "ios/chrome/browser/main/all_web_state_list_observation_registrar.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
+#include "ios/chrome/browser/main/browser.h"
+#include "ios/chrome/browser/main/browser_list.h"
+#include "ios/chrome/browser/main/browser_list_factory.h"
+
+AllWebStateListObservationRegistrar::AllWebStateListObservationRegistrar(
+    ChromeBrowserState* browser_state,
+    std::unique_ptr<WebStateListObserver> web_state_list_observer,
+    Mode mode)
+    : browser_list_(BrowserListFactory::GetForBrowserState(browser_state)),
+      web_state_list_observer_(std::move(web_state_list_observer)),
+      scoped_observer_(
+          std::make_unique<ScopedObserver<WebStateList, WebStateListObserver>>(
+              web_state_list_observer_.get())),
+      mode_(mode) {
+  browser_list_->AddObserver(this);
+
+  // There may already be browsers in |browser_list| when this object is
+  // created. Register as an observer for (mode permitting) both the regular and
+  // incognito browsers' WebStateLists.
+  if (mode_ & Mode::REGULAR) {
+    for (Browser* browser : browser_list_->AllRegularBrowsers()) {
+      scoped_observer_->Add(browser->GetWebStateList());
+    }
+  }
+
+  if (mode_ & Mode::INCOGNITO) {
+    for (Browser* browser : browser_list_->AllIncognitoBrowsers()) {
+      scoped_observer_->Add(browser->GetWebStateList());
+    }
+  }
+}
+
+AllWebStateListObservationRegistrar::AllWebStateListObservationRegistrar(
+    ChromeBrowserState* browser_state,
+    std::unique_ptr<WebStateListObserver> web_state_list_observer)
+    : AllWebStateListObservationRegistrar(browser_state,
+                                          std::move(web_state_list_observer),
+                                          Mode::ALL) {}
+
+AllWebStateListObservationRegistrar::~AllWebStateListObservationRegistrar() {
+  // If the browser state has already shut down, |browser_list_| should be
+  // nullptr; otherwise, stop observing it.
+  if (browser_list_)
+    browser_list_->RemoveObserver(this);
+}
+
+void AllWebStateListObservationRegistrar::OnBrowserAdded(
+    const BrowserList* browser_list,
+    Browser* browser) {
+  if (mode_ & Mode::REGULAR)
+    scoped_observer_->Add(browser->GetWebStateList());
+}
+
+void AllWebStateListObservationRegistrar::OnIncognitoBrowserAdded(
+    const BrowserList* browser_list,
+    Browser* browser) {
+  if (mode_ & Mode::INCOGNITO)
+    scoped_observer_->Add(browser->GetWebStateList());
+}
+
+void AllWebStateListObservationRegistrar::OnBrowserRemoved(
+    const BrowserList* browser_list,
+    Browser* browser) {
+  if (mode_ & Mode::REGULAR)
+    scoped_observer_->Remove(browser->GetWebStateList());
+}
+
+void AllWebStateListObservationRegistrar::OnIncognitoBrowserRemoved(
+    const BrowserList* browser_list,
+    Browser* browser) {
+  if (mode_ & Mode::INCOGNITO)
+    scoped_observer_->Remove(browser->GetWebStateList());
+}
+
+void AllWebStateListObservationRegistrar::OnBrowserListShutdown(
+    BrowserList* browser_list) {
+  DCHECK_EQ(browser_list, browser_list_);
+  // Stop observing all observed web state lists.
+  if (mode_ & Mode::REGULAR) {
+    for (Browser* browser : browser_list_->AllRegularBrowsers()) {
+      scoped_observer_->Remove(browser->GetWebStateList());
+    }
+  }
+
+  if (mode_ & Mode::INCOGNITO) {
+    for (Browser* browser : browser_list_->AllIncognitoBrowsers()) {
+      scoped_observer_->Remove(browser->GetWebStateList());
+    }
+  }
+  // Stop observimg the browser list, and clear |browser_list_|.
+  browser_list_->RemoveObserver(this);
+  browser_list_ = nullptr;
+}

ios/chrome/browser/main/all_web_state_list_observation_registrar_unittest.mm

+// 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.
+
+#import "ios/chrome/browser/main/all_web_state_list_observation_registrar.h"
+
+#include "base/test/task_environment.h"
+#include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
+#import "ios/chrome/browser/main/browser_list.h"
+#import "ios/chrome/browser/main/browser_list_factory.h"
+#import "ios/chrome/browser/main/test_browser.h"
+#import "ios/chrome/browser/web_state_list/web_state_list.h"
+#import "ios/chrome/browser/web_state_list/web_state_opener.h"
+#import "ios/web/public/test/fakes/test_web_state.h"
+#include "ios/web/public/test/web_task_environment.h"
+#include "testing/platform_test.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+class TestRegisteredWebStateListObserver : public WebStateListObserver {
+ public:
+  void WebStateInsertedAt(WebStateList* web_state_list,
+                          web::WebState* web_state,
+                          int index,
+                          bool activating) override {
+    insertion_count_++;
+  }
+  int insertion_count_;
+};
+
+class AllWebStateListObservationRegistrarTest : public PlatformTest {
+ protected:
+  AllWebStateListObservationRegistrarTest()
+      : owned_observer_(std::make_unique<TestRegisteredWebStateListObserver>()),
+        observer_(owned_observer_.get()) {
+    TestChromeBrowserState::Builder test_cbs_builder;
+    chrome_browser_state_ = test_cbs_builder.Build();
+
+    browser_list_ =
+        BrowserListFactory::GetForBrowserState(chrome_browser_state_.get());
+  }
+
+  void AppendNewWebState(Browser* browser) {
+    auto test_web_state = std::make_unique<web::TestWebState>();
+    browser->GetWebStateList()->InsertWebState(
+        WebStateList::kInvalidIndex, std::move(test_web_state),
+        WebStateList::INSERT_NO_FLAGS, WebStateOpener());
+  }
+
+  base::test::TaskEnvironment task_environment_;
+  // Unique pointer to an observer moved into the registrar under test.
+  std::unique_ptr<TestRegisteredWebStateListObserver> owned_observer_;
+  // Weak pointer to the the moved observer
+  TestRegisteredWebStateListObserver* observer_;
+  std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
+  BrowserList* browser_list_;
+};
+
+// Test
+TEST_F(AllWebStateListObservationRegistrarTest, RegisterAllLists) {
+  TestBrowser regular_browser_0(chrome_browser_state_.get());
+  browser_list_->AddBrowser(&regular_browser_0);
+  TestBrowser incognito_browser_0(
+      chrome_browser_state_->GetOffTheRecordChromeBrowserState());
+  browser_list_->AddIncognitoBrowser(&incognito_browser_0);
+
+  AllWebStateListObservationRegistrar registrar(chrome_browser_state_.get(),
+                                                std::move(owned_observer_));
+  // Should observe both insertions.
+  AppendNewWebState(&regular_browser_0);
+  EXPECT_EQ(1, observer_->insertion_count_);
+  AppendNewWebState(&incognito_browser_0);
+  EXPECT_EQ(2, observer_->insertion_count_);
+
+  // Create a second regular browser and add it.
+  TestBrowser regular_browser_1(chrome_browser_state_.get());
+  browser_list_->AddBrowser(&regular_browser_1);
+  AppendNewWebState(&regular_browser_1);
+  // Expect observed insertion.
+  EXPECT_EQ(3, observer_->insertion_count_);
+
+  // Create a second incognito  browser and add it.
+  TestBrowser incognito_browser_1(
+      chrome_browser_state_->GetOffTheRecordChromeBrowserState());
+  browser_list_->AddIncognitoBrowser(&incognito_browser_1);
+  AppendNewWebState(&incognito_browser_1);
+  // Expect observed insertion.
+  EXPECT_EQ(4, observer_->insertion_count_);
+
+  // Remove a regular browser
+  browser_list_->RemoveBrowser(&regular_browser_1);
+  AppendNewWebState(&regular_browser_1);
+  // Expect no observed insertion.
+  EXPECT_EQ(4, observer_->insertion_count_);
+
+  // Remove an incognito browser
+  browser_list_->RemoveIncognitoBrowser(&incognito_browser_1);
+  AppendNewWebState(&incognito_browser_1);
+  // Expect no observed insertion.
+  EXPECT_EQ(4, observer_->insertion_count_);
+}
+
+TEST_F(AllWebStateListObservationRegistrarTest, RegisterRegularLists) {
+  TestBrowser regular_browser_0(chrome_browser_state_.get());
+  browser_list_->AddBrowser(&regular_browser_0);
+  TestBrowser incognito_browser_0(
+      chrome_browser_state_->GetOffTheRecordChromeBrowserState());
+  browser_list_->AddIncognitoBrowser(&incognito_browser_0);
+
+  AllWebStateListObservationRegistrar registrar(
+      chrome_browser_state_.get(), std::move(owned_observer_),
+      AllWebStateListObservationRegistrar::Mode::REGULAR);
+  // Should observe only the reugular insertions.
+  AppendNewWebState(&regular_browser_0);
+  EXPECT_EQ(1, observer_->insertion_count_);
+  AppendNewWebState(&incognito_browser_0);
+  EXPECT_EQ(1, observer_->insertion_count_);
+
+  // Create a second regular browser and add it.
+  TestBrowser regular_browser_1(chrome_browser_state_.get());
+  browser_list_->AddBrowser(&regular_browser_1);
+  AppendNewWebState(&regular_browser_1);
+  // Expect observed insertion.
+  EXPECT_EQ(2, observer_->insertion_count_);
+
+  // Create a second incognito  browser and add it.
+  TestBrowser incognito_browser_1(
+      chrome_browser_state_->GetOffTheRecordChromeBrowserState());
+  browser_list_->AddIncognitoBrowser(&incognito_browser_1);
+  AppendNewWebState(&incognito_browser_0);
+  // Expect no observed insertion.
+  EXPECT_EQ(2, observer_->insertion_count_);
+}
+
+TEST_F(AllWebStateListObservationRegistrarTest, RegisterIncognitoLists) {
+  TestBrowser regular_browser_0(chrome_browser_state_.get());
+  browser_list_->AddBrowser(&regular_browser_0);
+  TestBrowser incognito_browser_0(
+      chrome_browser_state_->GetOffTheRecordChromeBrowserState());
+  browser_list_->AddIncognitoBrowser(&incognito_browser_0);
+
+  AllWebStateListObservationRegistrar registrar(
+      chrome_browser_state_.get(), std::move(owned_observer_),
+      AllWebStateListObservationRegistrar::Mode::INCOGNITO);
+  // Should observe only the incognito insertions.
+  AppendNewWebState(&regular_browser_0);
+  EXPECT_EQ(0, observer_->insertion_count_);
+  AppendNewWebState(&incognito_browser_0);
+  EXPECT_EQ(1, observer_->insertion_count_);
+
+  // Create a second regular browser and add it.
+  TestBrowser regular_browser_1(chrome_browser_state_.get());
+  browser_list_->AddBrowser(&regular_browser_1);
+  AppendNewWebState(&regular_browser_1);
+  // Expect no observed insertion.
+  EXPECT_EQ(1, observer_->insertion_count_);
+
+  // Create a second incognito  browser and add it.
+  TestBrowser incognito_browser_1(
+      chrome_browser_state_->GetOffTheRecordChromeBrowserState());
+  browser_list_->AddIncognitoBrowser(&incognito_browser_1);
+  AppendNewWebState(&incognito_browser_0);
+  // Expect observed insertion.
+  EXPECT_EQ(2, observer_->insertion_count_);
+}
+
+TEST_F(AllWebStateListObservationRegistrarTest, DeleteWithObservers) {
+  // Test that deleting a registrar with active observers is safe.
+  TestBrowser regular_browser_0(chrome_browser_state_.get());
+  browser_list_->AddBrowser(&regular_browser_0);
+
+  {
+    AllWebStateListObservationRegistrar registrar(chrome_browser_state_.get(),
+                                                  std::move(owned_observer_));
+  }
+}
+
+// Tests that deleting the browser state is safe.
+TEST_F(AllWebStateListObservationRegistrarTest, DeleteBrowserState) {
+  // Create some browsers and a registrar, as above.
+  TestBrowser regular_browser_0(chrome_browser_state_.get());
+  browser_list_->AddBrowser(&regular_browser_0);
+  TestBrowser incognito_browser_0(
+      chrome_browser_state_->GetOffTheRecordChromeBrowserState());
+  browser_list_->AddIncognitoBrowser(&incognito_browser_0);
+
+  AllWebStateListObservationRegistrar registrar(chrome_browser_state_.get(),
+                                                std::move(owned_observer_));
+  AppendNewWebState(&regular_browser_0);
+  AppendNewWebState(&incognito_browser_0);
+  TestBrowser regular_browser_1(chrome_browser_state_.get());
+  browser_list_->AddBrowser(&regular_browser_1);
+  AppendNewWebState(&regular_browser_1);
+
+  // Now delete the browser state. Nothing should explode.
+  chrome_browser_state_.reset();
+}

ios/chrome/browser/main/browser_list_factory.h

@@ -5,7 +5,6 @@
 #ifndef IOS_CHROME_BROWSER_MAIN_BROWSER_LIST_FACTORY_H_
 #define IOS_CHROME_BROWSER_MAIN_BROWSER_LIST_FACTORY_H_
 
-#include "base/macros.h"
 #include "base/no_destructor.h"
 #include "components/keyed_service/ios/browser_state_keyed_service_factory.h"
 
@@ -22,6 +21,10 @@ class BrowserListFactory : public BrowserStateKeyedServiceFactory {
   // Getter for singleton instance.
   static BrowserListFactory* GetInstance();
 
+  // Not copyable or moveable.
+  BrowserListFactory(const BrowserListFactory&) = delete;
+  BrowserListFactory& operator=(const BrowserListFactory&) = delete;
+
  private:
   friend class base::NoDestructor<BrowserListFactory>;
 
@@ -33,7 +36,7 @@ class BrowserListFactory : public BrowserStateKeyedServiceFactory {
   web::BrowserState* GetBrowserStateToUse(
       web::BrowserState* context) const override;
 
-  DISALLOW_COPY_AND_ASSIGN(BrowserListFactory);
+  void BrowserStateShutdown(web::BrowserState* context) override;
 };
 
 #endif  // IOS_CHROME_BROWSER_MAIN_BROWSER_LIST_FACTORY_H_

ios/chrome/browser/main/browser_list_factory.mm

@@ -43,3 +43,7 @@ web::BrowserState* BrowserListFactory::GetBrowserStateToUse(
   // Incognito browser states use same service as regular browser states.
   return GetBrowserStateRedirectedInIncognito(context);
 }
+
+void BrowserListFactory::BrowserStateShutdown(web::BrowserState* context) {
+  GetForBrowserState(ChromeBrowserState::FromBrowserState(context))->Shutdown();
+}

ios/chrome/browser/main/browser_list_impl.h

@@ -5,18 +5,26 @@
 #ifndef IOS_CHROME_BROWSER_MAIN_BROWSER_LIST_IMPL_H_
 #define IOS_CHROME_BROWSER_MAIN_BROWSER_LIST_IMPL_H_
 
-#include "base/macros.h"
 #import "ios/chrome/browser/main/browser_list.h"
 #include "ios/chrome/browser/main/browser_list_observer.h"
+#import "ios/chrome/browser/main/browser_observer.h"
 
 // The concrete implementation of BrowserList returned by the
 // BrowserListFactory.
-class BrowserListImpl : public BrowserList {
+class BrowserListImpl : public BrowserList, public BrowserObserver {
  public:
   BrowserListImpl();
+
+  // Not copyable or moveable.
+  BrowserListImpl(const BrowserListImpl&) = delete;
+  BrowserListImpl& operator=(const BrowserListImpl&) = delete;
+
   ~BrowserListImpl() override;
 
-  // BrowserList:
+  // KeyedService
+  void Shutdown() override;
+
+  // BrowserList
   void AddBrowser(Browser* browser) override;
   void AddIncognitoBrowser(Browser* browser) override;
   void RemoveBrowser(Browser* browser) override;
@@ -26,12 +34,13 @@ class BrowserListImpl : public BrowserList {
   void AddObserver(BrowserListObserver* observer) override;
   void RemoveObserver(BrowserListObserver* observer) override;
 
+  // BrowserObserver:
+  void BrowserDestroyed(Browser* browser) override;
+
  private:
   std::set<Browser*> browsers_;
   std::set<Browser*> incognito_browsers_;
   base::ObserverList<BrowserListObserver, true>::Unchecked observers_;
-
-  DISALLOW_COPY_AND_ASSIGN(BrowserListImpl);
 };
 
 #endif  // IOS_CHROME_BROWSER_MAIN_BROWSER_LIST_IMPL_H_

ios/chrome/browser/main/browser_list_impl.mm

@@ -12,12 +12,28 @@
 #endif
 
 BrowserListImpl::BrowserListImpl() {}
-BrowserListImpl::~BrowserListImpl() {}
+BrowserListImpl::~BrowserListImpl() {
+  observers_.Clear();
+}
+
+// KeyedService:
+void BrowserListImpl::Shutdown() {
+  for (auto& observer : observers_)
+    observer.OnBrowserListShutdown(this);
+  observers_.Clear();
+  for (Browser* browser : browsers_) {
+    browser->RemoveObserver(this);
+  }
+  for (Browser* browser : incognito_browsers_) {
+    browser->RemoveObserver(this);
+  }
+}
 
 // BrowserList:
 void BrowserListImpl::AddBrowser(Browser* browser) {
   DCHECK(!browser->GetBrowserState()->IsOffTheRecord());
   browsers_.insert(browser);
+  browser->AddObserver(this);
   for (auto& observer : observers_)
     observer.OnBrowserAdded(this, browser);
 }
@@ -25,12 +41,14 @@ void BrowserListImpl::AddBrowser(Browser* browser) {
 void BrowserListImpl::AddIncognitoBrowser(Browser* browser) {
   DCHECK(browser->GetBrowserState()->IsOffTheRecord());
   incognito_browsers_.insert(browser);
+  browser->AddObserver(this);
   for (auto& observer : observers_)
     observer.OnIncognitoBrowserAdded(this, browser);
 }
 
 void BrowserListImpl::RemoveBrowser(Browser* browser) {
   if (browsers_.erase(browser) > 0) {
+    browser->RemoveObserver(this);
     for (auto& observer : observers_)
       observer.OnBrowserRemoved(this, browser);
   }
@@ -38,6 +56,7 @@ void BrowserListImpl::RemoveBrowser(Browser* browser) {
 
 void BrowserListImpl::RemoveIncognitoBrowser(Browser* browser) {
   if (incognito_browsers_.erase(browser) > 0) {
+    browser->RemoveObserver(this);
     for (auto& observer : observers_)
       observer.OnIncognitoBrowserRemoved(this, browser);
   }
@@ -60,3 +79,13 @@ void BrowserListImpl::AddObserver(BrowserListObserver* observer) {
 void BrowserListImpl::RemoveObserver(BrowserListObserver* observer) {
   observers_.RemoveObserver(observer);
 }
+
+// BrowserObserver
+void BrowserListImpl::BrowserDestroyed(Browser* browser) {
+  if (browser->GetBrowserState()->IsOffTheRecord()) {
+    RemoveIncognitoBrowser(browser);
+  } else {
+    RemoveBrowser(browser);
+  }
+  browser->RemoveObserver(this);
+}

ios/chrome/browser/main/browser_list_impl_unittest.mm

@@ -114,6 +114,26 @@ TEST_F(BrowserListImplTest, AddRemoveIncognitoBrowsers) {
   EXPECT_EQ(0UL, browser_list_->AllRegularBrowsers().size());
 }
 
+// Test that destroyed browsers are auto-removed.
+TEST_F(BrowserListImplTest, AutoRemoveBrowsers) {
+  {
+    // Create and add scoped browsers
+    TestBrowser browser_1(chrome_browser_state_.get());
+    browser_list_->AddBrowser(&browser_1);
+    EXPECT_EQ(1UL, browser_list_->AllRegularBrowsers().size());
+
+    ChromeBrowserState* incognito_browser_state =
+        chrome_browser_state_->GetOffTheRecordChromeBrowserState();
+    TestBrowser incognito_browser_1(incognito_browser_state);
+    browser_list_->AddIncognitoBrowser(&incognito_browser_1);
+    EXPECT_EQ(1UL, browser_list_->AllIncognitoBrowsers().size());
+  }
+
+  // Expect that the browsers going out of scope will have triggered removal.
+  EXPECT_EQ(0UL, browser_list_->AllRegularBrowsers().size());
+  EXPECT_EQ(0UL, browser_list_->AllIncognitoBrowsers().size());
+}
+
 // Test that values returned from AllRegularBrowsers and AllIncognitoBrowsers
 // aren't affected by subsequent changes to the browser list.
 TEST_F(BrowserListImplTest, AllBrowserValuesDontChange) {
@@ -172,3 +192,13 @@ TEST_F(BrowserListImplTest, DISABLED_BrowserListObserver) {
 
   browser_list_->RemoveObserver(observer);
 }
+
+TEST_F(BrowserListImplTest, DeleteBrowserState) {
+  TestBrowserListObserver* observer = new TestBrowserListObserver;
+  browser_list_->AddObserver(observer);
+  Browser* browser_1 = new TestBrowser(chrome_browser_state_.get());
+  browser_list_->AddBrowser(browser_1);
+
+  // Now delete the browser state. Nothing should explode.
+  chrome_browser_state_.reset();
+}

ios/chrome/browser/main/browser_list_observer.h

@@ -31,6 +31,11 @@ class BrowserListObserver {
   virtual void OnIncognitoBrowserRemoved(const BrowserList* browser_list,
                                          Browser* browser) {}
 
+  // Called before the browserlist is destroyed, in case the observer needs to
+  // do any cleanup. After this method is called, all observers will be removed
+  // from |browser_list|, and no firther obeserver methods will be called.
+  virtual void OnBrowserListShutdown(BrowserList* browser_list) {}
+
  private:
   DISALLOW_COPY_AND_ASSIGN(BrowserListObserver);
 };

ios/chrome/browser/snapshots/BUILD.gn

@@ -8,7 +8,6 @@ source_set("snapshots") {
     "snapshot_cache_factory.h",
     "snapshot_cache_internal.h",
     "snapshot_cache_observer.h",
-    "snapshot_cache_tab_model_list_observer.h",
     "snapshot_cache_web_state_list_observer.h",
     "snapshot_generator_delegate.h",
     "snapshot_lru_cache.h",
@@ -18,7 +17,6 @@ source_set("snapshots") {
   sources = [
     "snapshot_cache.mm",
     "snapshot_cache_factory.mm",
-    "snapshot_cache_tab_model_list_observer.mm",
     "snapshot_cache_web_state_list_observer.mm",
     "snapshot_generator.h",
     "snapshot_generator.mm",
@@ -33,7 +31,7 @@ source_set("snapshots") {
     "//components/keyed_service/ios",
     "//ios/chrome/browser/browser_state",
     "//ios/chrome/browser/infobars",
-    "//ios/chrome/browser/tabs",
+    "//ios/chrome/browser/main:public",
     "//ios/chrome/browser/ui:feature_flags",
     "//ios/chrome/browser/ui/infobars:feature_flags",
     "//ios/chrome/browser/ui/util",

ios/chrome/browser/snapshots/snapshot_cache_factory.mm

@@ -12,8 +12,8 @@
 #include "components/keyed_service/ios/browser_state_dependency_manager.h"
 #include "ios/chrome/browser/browser_state/browser_state_otr_helper.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
+#import "ios/chrome/browser/main/all_web_state_list_observation_registrar.h"
 #import "ios/chrome/browser/snapshots/snapshot_cache.h"
-#import "ios/chrome/browser/snapshots/snapshot_cache_tab_model_list_observer.h"
 #import "ios/chrome/browser/snapshots/snapshot_cache_web_state_list_observer.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -36,7 +36,7 @@ class SnapshotCacheWrapper : public KeyedService {
 
  private:
   __strong SnapshotCache* snapshot_cache_;
-  std::unique_ptr<SnapshotCacheTabModelListObserver> tab_model_list_observer_;
+  std::unique_ptr<AllWebStateListObservationRegistrar> registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(SnapshotCacheWrapper);
 };
@@ -45,10 +45,9 @@ SnapshotCacheWrapper::SnapshotCacheWrapper(ChromeBrowserState* browser_state,
                                            SnapshotCache* snapshot_cache)
     : snapshot_cache_(snapshot_cache) {
   DCHECK(snapshot_cache);
-  tab_model_list_observer_ =
-      std::make_unique<SnapshotCacheTabModelListObserver>(
-          browser_state,
-          std::make_unique<SnapshotCacheWebStateListObserver>(snapshot_cache));
+  registrar_ = std::make_unique<AllWebStateListObservationRegistrar>(
+      browser_state,
+      std::make_unique<SnapshotCacheWebStateListObserver>(snapshot_cache));
 }
 
 SnapshotCacheWrapper::~SnapshotCacheWrapper() {
@@ -56,7 +55,7 @@ SnapshotCacheWrapper::~SnapshotCacheWrapper() {
 }
 
 void SnapshotCacheWrapper::Shutdown() {
-  tab_model_list_observer_.reset();
+  registrar_.reset();
   [snapshot_cache_ shutdown];
   snapshot_cache_ = nil;
 }

ios/chrome/browser/snapshots/snapshot_cache_tab_model_list_observer.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 IOS_CHROME_BROWSER_SNAPSHOTS_SNAPSHOT_CACHE_TAB_MODEL_LIST_OBSERVER_H_
-#define IOS_CHROME_BROWSER_SNAPSHOTS_SNAPSHOT_CACHE_TAB_MODEL_LIST_OBSERVER_H_
-
-#include <memory>
-
-#include "base/macros.h"
-#include "base/scoped_observer.h"
-#import "ios/chrome/browser/tabs/tab_model_list_observer.h"
-#include "ios/chrome/browser/web_state_list/web_state_list.h"
-#include "ios/chrome/browser/web_state_list/web_state_list_observer.h"
-
-class ChromeBrowserState;
-@class TabModel;
-
-// SnapshotCacheTabModelListObserver tracks when TabModels are created and
-// destroyed for a given ChromeBrowserState.  Whenever the TabModelList changes,
-// SnapshotCacheTabModelListObserver registers a provided observer as a
-// WebStateListObserver.
-class SnapshotCacheTabModelListObserver : public TabModelListObserver {
- public:
-  // Constructs an object that registers the given |web_state_list_observer| as
-  // a WebStateListObserver for any TabModels associated with |browser_state| or
-  // |browser_state|'s OTR browser state.  Keeps observer registration up to
-  // date as TabModels are created and destroyed. |browser_state| must be a
-  // normal (non-OTR) browser state.
-  SnapshotCacheTabModelListObserver(
-      ChromeBrowserState* browser_state,
-      std::unique_ptr<WebStateListObserver> web_state_list_observer);
-
-  ~SnapshotCacheTabModelListObserver() override;
-
-  // TabModelListObserver.
-  void TabModelRegisteredWithBrowserState(
-      TabModel* tab_model,
-      ChromeBrowserState* browser_state) override;
-  void TabModelUnregisteredFromBrowserState(
-      TabModel* tab_model,
-      ChromeBrowserState* browser_state) override;
-
- private:
-  ChromeBrowserState* browser_state_;
-  std::unique_ptr<WebStateListObserver> web_state_list_observer_;
-  std::unique_ptr<ScopedObserver<WebStateList, WebStateListObserver>>
-      scoped_observer_;
-
-  DISALLOW_COPY_AND_ASSIGN(SnapshotCacheTabModelListObserver);
-};
-
-#endif  // IOS_CHROME_BROWSER_SNAPSHOTS_SNAPSHOT_CACHE_TAB_MODEL_LIST_OBSERVER_H_

ios/chrome/browser/snapshots/snapshot_cache_tab_model_list_observer.mm

-// 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.
-
-#import "ios/chrome/browser/snapshots/snapshot_cache_tab_model_list_observer.h"
-
-#if !defined(__has_feature) || !__has_feature(objc_arc)
-#error "This file requires ARC support."
-#endif
-
-#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
-#import "ios/chrome/browser/tabs/tab_model.h"
-#import "ios/chrome/browser/tabs/tab_model_list.h"
-#import "ios/chrome/browser/tabs/tab_model_list_observer.h"
-
-SnapshotCacheTabModelListObserver::SnapshotCacheTabModelListObserver(
-    ChromeBrowserState* browser_state,
-    std::unique_ptr<WebStateListObserver> web_state_list_observer)
-    : browser_state_(browser_state),
-      web_state_list_observer_(std::move(web_state_list_observer)),
-      scoped_observer_(
-          std::make_unique<ScopedObserver<WebStateList, WebStateListObserver>>(
-              web_state_list_observer_.get())) {
-  TabModelList::AddObserver(this);
-
-  // Register as an observer for all TabModels for both the normal and otr
-  // browser states.
-  DCHECK(!browser_state_->IsOffTheRecord());
-  for (TabModel* model :
-       TabModelList::GetTabModelsForChromeBrowserState(browser_state_)) {
-    scoped_observer_->Add(model.webStateList);
-  }
-
-  if (browser_state_->HasOffTheRecordChromeBrowserState()) {
-    ChromeBrowserState* otr_state =
-        browser_state->GetOffTheRecordChromeBrowserState();
-    for (TabModel* model :
-         TabModelList::GetTabModelsForChromeBrowserState(otr_state)) {
-      scoped_observer_->Add(model.webStateList);
-    }
-  }
-}
-
-SnapshotCacheTabModelListObserver::~SnapshotCacheTabModelListObserver() {
-  TabModelList::RemoveObserver(this);
-}
-
-void SnapshotCacheTabModelListObserver::TabModelRegisteredWithBrowserState(
-    TabModel* tab_model,
-    ChromeBrowserState* browser_state) {
-  // Normal and Incognito browser states share a SnapshotCache.
-  if (browser_state_ == browser_state->GetOriginalChromeBrowserState()) {
-    scoped_observer_->Add(tab_model.webStateList);
-  }
-}
-
-void SnapshotCacheTabModelListObserver::TabModelUnregisteredFromBrowserState(
-    TabModel* tab_model,
-    ChromeBrowserState* browser_state) {
-  // Normal and Incognito browser states share a SnapshotCache.
-  if (browser_state_ == browser_state->GetOriginalChromeBrowserState()) {
-    scoped_observer_->Remove(tab_model.webStateList);
-  }
-}

ios/chrome/browser/tabs/BUILD.gn

@@ -153,6 +153,7 @@ source_set("unit_tests") {
     "//ios/chrome/browser/web",
     "//ios/chrome/browser/web:web_internal",
     "//ios/chrome/browser/web_state_list",
+    "//ios/chrome/browser/web_state_list:agents",
     "//ios/chrome/browser/web_state_list:test_support",
     "//ios/chrome/browser/web_state_list/web_usage_enabler",
     "//ios/chrome/test:test_support",

ios/chrome/browser/ui/browser_view/BUILD.gn

@@ -160,6 +160,7 @@ source_set("browser_view") {
     "//ios/chrome/browser/web:tab_helper_delegates",
     "//ios/chrome/browser/web:web_internal",
     "//ios/chrome/browser/web_state_list",
+    "//ios/chrome/browser/web_state_list:agents",
     "//ios/chrome/browser/web_state_list/web_usage_enabler",
     "//ios/chrome/browser/webui",
     "//ios/chrome/common",

ios/chrome/browser/ui/tab_grid/BUILD.gn

@@ -44,6 +44,7 @@ source_set("tab_grid") {
     "//ios/chrome/browser/url_loading",
     "//ios/chrome/browser/web",
     "//ios/chrome/browser/web_state_list",
+    "//ios/chrome/browser/web_state_list:agents",
     "//ios/chrome/browser/web_state_list/web_usage_enabler",
     "//ios/web",
     "//ui/base",

ios/chrome/browser/url_loading/BUILD.gn

@@ -46,6 +46,7 @@ source_set("url_loading") {
     "//ios/chrome/browser/ui/ntp:util",
     "//ios/chrome/browser/web",
     "//ios/chrome/browser/web_state_list",
+    "//ios/chrome/browser/web_state_list:agents",
     "//ios/web/public",
     "//ui/base",
     "//url",
@@ -72,6 +73,7 @@ source_set("unit_tests") {
     "//ios/chrome/browser/web",
     "//ios/chrome/browser/web:web_internal",
     "//ios/chrome/browser/web_state_list",
+    "//ios/chrome/browser/web_state_list:agents",
     "//ios/chrome/browser/web_state_list:test_support",
     "//ios/chrome/browser/web_state_list/web_usage_enabler",
     "//ios/chrome/test:test_support",

ios/chrome/browser/web_state_list/BUILD.gn

@@ -8,8 +8,6 @@ source_set("web_state_list") {
     "active_web_state_observation_forwarder.mm",
     "all_web_state_observation_forwarder.h",
     "all_web_state_observation_forwarder.mm",
-    "tab_insertion_browser_agent.h",
-    "tab_insertion_browser_agent.mm",
     "web_state_list.h",
     "web_state_list.mm",
     "web_state_list_delegate.h",
@@ -33,7 +31,6 @@ source_set("web_state_list") {
     "//components/favicon/core",
     "//components/favicon/ios",
     "//ios/chrome/browser/browser_state",
-    "//ios/chrome/browser/main:public",
     "//ios/chrome/browser/sessions:restoration_observer",
     "//ios/chrome/browser/sessions:serialisation",
     "//ios/web",
@@ -43,6 +40,21 @@ source_set("web_state_list") {
   configs += [ "//build/config/compiler:enable_arc" ]
 }
 
+source_set("agents") {
+  sources = [
+    "tab_insertion_browser_agent.h",
+    "tab_insertion_browser_agent.mm",
+  ]
+  deps = [
+    ":web_state_list",
+    "//ios/chrome/browser/browser_state",
+    "//ios/chrome/browser/main:public",
+    "//ios/web/public",
+  ]
+  libs = [ "Foundation.framework" ]
+  configs += [ "//build/config/compiler:enable_arc" ]
+}
+
 source_set("test_support") {
   testonly = true
   sources = [
@@ -69,6 +81,7 @@ source_set("unit_tests") {
     "web_state_opener_unittest.mm",
   ]
   deps = [
+    ":agents",
     ":test_support",
     ":web_state_list",
     "//base",