Revert "[iOS] Create WebStateList observation registrar."

This reverts commit 7a9ca38d.

Reason for revert: It appears all the browserlist observers are
busted after closing the last incognito tab.

Original change's description:
> [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}

TBR=rohitrao@chromium.org,marq@chromium.org,justincohen@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

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

ios/chrome/browser/main/BUILD.gn

@@ -4,7 +4,6 @@
 
 source_set("public") {
   sources = [
-    "all_web_state_list_observation_registrar.h",
     "browser.h",
     "browser_list.h",
     "browser_list_factory.h",
@@ -17,14 +16,12 @@ 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",
@@ -51,7 +48,6 @@ 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" ]
@@ -83,7 +79,6 @@ 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,6 +5,7 @@
 #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"
 
@@ -21,10 +22,6 @@ 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>;
 
@@ -36,7 +33,7 @@ class BrowserListFactory : public BrowserStateKeyedServiceFactory {
   web::BrowserState* GetBrowserStateToUse(
       web::BrowserState* context) const override;
 
-  void BrowserStateShutdown(web::BrowserState* context) override;
+  DISALLOW_COPY_AND_ASSIGN(BrowserListFactory);
 };
 
 #endif  // IOS_CHROME_BROWSER_MAIN_BROWSER_LIST_FACTORY_H_

ios/chrome/browser/main/browser_list_factory.mm

@@ -43,7 +43,3 @@ 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,26 +5,18 @@
 #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, public BrowserObserver {
+class BrowserListImpl : public BrowserList {
  public:
   BrowserListImpl();
-
-  // Not copyable or moveable.
-  BrowserListImpl(const BrowserListImpl&) = delete;
-  BrowserListImpl& operator=(const BrowserListImpl&) = delete;
-
   ~BrowserListImpl() override;
 
-  // KeyedService
-  void Shutdown() override;
-
-  // BrowserList
+  // BrowserList:
   void AddBrowser(Browser* browser) override;
   void AddIncognitoBrowser(Browser* browser) override;
   void RemoveBrowser(Browser* browser) override;
@@ -34,13 +26,12 @@ class BrowserListImpl : public BrowserList, public BrowserObserver {
   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,28 +12,12 @@
 #endif
 
 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);
-  }
-}
+BrowserListImpl::~BrowserListImpl() {}
 
 // BrowserList:
 void BrowserListImpl::AddBrowser(Browser* browser) {
   DCHECK(!browser->GetBrowserState()->IsOffTheRecord());
   browsers_.insert(browser);
-  browser->AddObserver(this);
   for (auto& observer : observers_)
     observer.OnBrowserAdded(this, browser);
 }
@@ -41,14 +25,12 @@ 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);
   }
@@ -56,7 +38,6 @@ 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);
   }
@@ -79,13 +60,3 @@ 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,26 +114,6 @@ 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) {
@@ -192,13 +172,3 @@ 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,11 +31,6 @@ 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,6 +8,7 @@ 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",
@@ -17,6 +18,7 @@ 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",
@@ -31,7 +33,7 @@ source_set("snapshots") {
     "//components/keyed_service/ios",
     "//ios/chrome/browser/browser_state",
     "//ios/chrome/browser/infobars",
-    "//ios/chrome/browser/main:public",
+    "//ios/chrome/browser/tabs",
     "//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<AllWebStateListObservationRegistrar> registrar_;
+  std::unique_ptr<SnapshotCacheTabModelListObserver> tab_model_list_observer_;
 
   DISALLOW_COPY_AND_ASSIGN(SnapshotCacheWrapper);
 };
@@ -45,9 +45,10 @@ SnapshotCacheWrapper::SnapshotCacheWrapper(ChromeBrowserState* browser_state,
                                            SnapshotCache* snapshot_cache)
     : snapshot_cache_(snapshot_cache) {
   DCHECK(snapshot_cache);
-  registrar_ = std::make_unique<AllWebStateListObservationRegistrar>(
-      browser_state,
-      std::make_unique<SnapshotCacheWebStateListObserver>(snapshot_cache));
+  tab_model_list_observer_ =
+      std::make_unique<SnapshotCacheTabModelListObserver>(
+          browser_state,
+          std::make_unique<SnapshotCacheWebStateListObserver>(snapshot_cache));
 }
 
 SnapshotCacheWrapper::~SnapshotCacheWrapper() {
@@ -55,7 +56,7 @@ SnapshotCacheWrapper::~SnapshotCacheWrapper() {
 }
 
 void SnapshotCacheWrapper::Shutdown() {
-  registrar_.reset();
+  tab_model_list_observer_.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,7 +153,6 @@ 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

@@ -161,7 +161,6 @@ 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,7 +44,6 @@ 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,7 +46,6 @@ 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",
@@ -73,7 +72,6 @@ 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,6 +8,8 @@ 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",
@@ -31,6 +33,7 @@ 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",
@@ -40,21 +43,6 @@ 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 = [
@@ -81,7 +69,6 @@ source_set("unit_tests") {
     "web_state_opener_unittest.mm",
   ]
   deps = [
-    ":agents",
     ":test_support",
     ":web_state_list",
     "//base",