Skip to content

Commit

Permalink
[5790] AllWebStateListObservationRegistrar takes directly the Browser…
Browse files Browse the repository at this point in the history
…List

Prior to this CL, AllWebStateListObservationRegistrar took the browser
state as parameter and extracted the browser list.
This CL makes it take directly the browser list, which is more
convenient for use in mediators for example.

(cherry picked from commit b5e9e81)

Bug: 1440947
Fixed: 1450851
Change-Id: I4cbe06cea8ace226b884e843b8d5c003c403bace
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4584032
Reviewed-by: Federica Germinario <fedegermi@google.com>
Auto-Submit: Louis Romero <lpromero@google.com>
Commit-Queue: Louis Romero <lpromero@google.com>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1152622}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4602668
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/branch-heads/5790@{#531}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
Arcank authored and Chromium LUCI CQ committed Jun 9, 2023
1 parent 1097cd9 commit 96546d2
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 35 deletions.
4 changes: 3 additions & 1 deletion ios/chrome/browser/metrics/incognito_web_state_observer.mm
Expand Up @@ -8,6 +8,7 @@

#import "ios/chrome/browser/shared/model/application_context/application_context.h"
#import "ios/chrome/browser/shared/model/browser/all_web_state_list_observation_registrar.h"
#import "ios/chrome/browser/shared/model/browser/browser_list_factory.h"
#import "ios/chrome/browser/shared/model/browser_state/chrome_browser_state.h"
#import "ios/chrome/browser/shared/model/browser_state/chrome_browser_state_manager.h"

Expand All @@ -25,7 +26,8 @@
for (ChromeBrowserState* browser_state : browser_states) {
DCHECK(!browser_state->IsOffTheRecord());
registrars_.insert(std::make_unique<AllWebStateListObservationRegistrar>(
browser_state, std::make_unique<Observer>(this),
BrowserListFactory::GetForBrowserState(browser_state),
std::make_unique<Observer>(this),
AllWebStateListObservationRegistrar::Mode::INCOGNITO));
}
}
Expand Down
Expand Up @@ -109,9 +109,8 @@ void WebStateDetachedAt(WebStateList* web_state_list,
if (browser_state_data_.count(path)) {
return;
}
auto* browser_state = browser_state_manager_->GetBrowserState(path);
BrowserList* browser_list =
BrowserListFactory::GetForBrowserState(browser_state);
BrowserList* browser_list = BrowserListFactory::GetForBrowserState(
browser_state_manager_->GetBrowserState(path));
DCHECK(browser_list);

auto it = browser_state_data_.emplace(
Expand All @@ -120,7 +119,7 @@ void WebStateDetachedAt(WebStateList* web_state_list,
BrowserStateData& data = *it.first->second;
data.all_web_state_observation =
std::make_unique<AllWebStateListObservationRegistrar>(
browser_state,
browser_list,
std::make_unique<WebStateObserver>(path, this, browser_list),
AllWebStateListObservationRegistrar::INCOGNITO);
}
Expand Down
Expand Up @@ -13,10 +13,9 @@
#include "ios/chrome/browser/shared/model/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,
// destroyed for a given BrowserList. Whenever the BrowserList changes,
// AllWebStateListObservationRegistrar registers (or unregisters) a provided
// observer as a WebStateListObserver.
class AllWebStateListObservationRegistrar : public BrowserListObserver {
Expand All @@ -27,20 +26,20 @@ class AllWebStateListObservationRegistrar : public BrowserListObserver {
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.
// Constructs an object that register the given `web_state_list_observer` as
// WebStateListObserver for any regular or OTR Browsers associated with
// `browser_list` according to `mode`.
// Keeps observer registration up to date as Browsers are added and removed
// from `browser_list`.
AllWebStateListObservationRegistrar(
ChromeBrowserState* browser_state,
BrowserList* browser_list,
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,
BrowserList* browser_list,
std::unique_ptr<WebStateListObserver> web_state_list_observer);

// Not copyable or moveable
Expand Down
Expand Up @@ -4,20 +4,18 @@

#import "ios/chrome/browser/shared/model/browser/all_web_state_list_observation_registrar.h"

#import "ios/chrome/browser/shared/model/browser/browser.h"
#import "ios/chrome/browser/shared/model/browser/browser_list.h"

#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif

#import "ios/chrome/browser/shared/model/browser/browser.h"
#import "ios/chrome/browser/shared/model/browser/browser_list.h"
#import "ios/chrome/browser/shared/model/browser/browser_list_factory.h"
#import "ios/chrome/browser/shared/model/browser_state/chrome_browser_state.h"

AllWebStateListObservationRegistrar::AllWebStateListObservationRegistrar(
ChromeBrowserState* browser_state,
BrowserList* browser_list,
std::unique_ptr<WebStateListObserver> web_state_list_observer,
Mode mode)
: browser_list_(BrowserListFactory::GetForBrowserState(browser_state)),
: browser_list_(browser_list),
web_state_list_observer_(std::move(web_state_list_observer)),
scoped_observations_(web_state_list_observer_.get()),
mode_(mode) {
Expand All @@ -40,15 +38,15 @@
}

AllWebStateListObservationRegistrar::AllWebStateListObservationRegistrar(
ChromeBrowserState* browser_state,
BrowserList* browser_list,
std::unique_ptr<WebStateListObserver> web_state_list_observer)
: AllWebStateListObservationRegistrar(browser_state,
: AllWebStateListObservationRegistrar(browser_list,
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 the owning browser state has already shut down, `browser_list_` should
// be nullptr; otherwise, stop observing it.
if (browser_list_) {
browser_list_->RemoveObserver(this);
}
Expand Down
Expand Up @@ -66,7 +66,7 @@ TestBrowser incognito_browser_0(
chrome_browser_state_->GetOffTheRecordChromeBrowserState());
browser_list_->AddIncognitoBrowser(&incognito_browser_0);

AllWebStateListObservationRegistrar registrar(chrome_browser_state_.get(),
AllWebStateListObservationRegistrar registrar(browser_list_,
std::move(owned_observer_));
// Should observe both insertions.
AppendNewWebState(&regular_browser_0);
Expand Down Expand Up @@ -110,7 +110,7 @@ TestBrowser incognito_browser_0(
browser_list_->AddIncognitoBrowser(&incognito_browser_0);

AllWebStateListObservationRegistrar registrar(
chrome_browser_state_.get(), std::move(owned_observer_),
browser_list_, std::move(owned_observer_),
AllWebStateListObservationRegistrar::Mode::REGULAR);
// Should observe only the reugular insertions.
AppendNewWebState(&regular_browser_0);
Expand Down Expand Up @@ -142,7 +142,7 @@ TestBrowser incognito_browser_0(
browser_list_->AddIncognitoBrowser(&incognito_browser_0);

AllWebStateListObservationRegistrar registrar(
chrome_browser_state_.get(), std::move(owned_observer_),
browser_list_, std::move(owned_observer_),
AllWebStateListObservationRegistrar::Mode::INCOGNITO);
// Should observe only the incognito insertions.
AppendNewWebState(&regular_browser_0);
Expand Down Expand Up @@ -172,7 +172,7 @@ TestBrowser incognito_browser_1(
browser_list_->AddBrowser(&regular_browser_0);

{
AllWebStateListObservationRegistrar registrar(chrome_browser_state_.get(),
AllWebStateListObservationRegistrar registrar(browser_list_,
std::move(owned_observer_));
}
}
Expand All @@ -186,7 +186,7 @@ TestBrowser incognito_browser_0(
chrome_browser_state_->GetOffTheRecordChromeBrowserState());
browser_list_->AddIncognitoBrowser(&incognito_browser_0);

AllWebStateListObservationRegistrar registrar(chrome_browser_state_.get(),
AllWebStateListObservationRegistrar registrar(browser_list_,
std::move(owned_observer_));
AppendNewWebState(&regular_browser_0);
AppendNewWebState(&incognito_browser_0);
Expand Down
Expand Up @@ -28,6 +28,7 @@ class IOSChromeLocalSessionEventRouter
: public sync_sessions::LocalSessionEventRouter {
public:
IOSChromeLocalSessionEventRouter(
// TODO(crbug.com/1450909): Pass a BrowserList directly instead.
ChromeBrowserState* browser_state,
sync_sessions::SyncSessionsClient* sessions_client_,
const syncer::SyncableService::StartSyncFlare& flare);
Expand Down Expand Up @@ -95,7 +96,7 @@ class IOSChromeLocalSessionEventRouter
// Called on observation of a change in `web_state`.
void OnWebStateChange(web::WebState* web_state);

// Observation registrars for the associated browser state; owns an instance
// Observation registrar for the associated browser list; owns an instance
// of IOSChromeLocalSessionEventRouter::Observer.
std::unique_ptr<AllWebStateListObservationRegistrar> const registrar_;

Expand Down
Expand Up @@ -14,6 +14,7 @@
#import "components/sync_sessions/synced_tab_delegate.h"
#import "ios/chrome/browser/history/history_service_factory.h"
#import "ios/chrome/browser/shared/model/browser/all_web_state_list_observation_registrar.h"
#import "ios/chrome/browser/shared/model/browser/browser_list_factory.h"
#import "ios/chrome/browser/shared/model/browser_state/chrome_browser_state.h"
#import "ios/chrome/browser/shared/model/web_state_list/web_state_list.h"
#import "ios/chrome/browser/sync/glue/sync_start_util.h"
Expand All @@ -40,7 +41,7 @@
sync_sessions::SyncSessionsClient* sessions_client,
const syncer::SyncableService::StartSyncFlare& flare)
: registrar_(std::make_unique<AllWebStateListObservationRegistrar>(
browser_state,
BrowserListFactory::GetForBrowserState(browser_state),
std::make_unique<Observer>(this),
AllWebStateListObservationRegistrar::Mode::REGULAR)),
sessions_client_(sessions_client),
Expand Down
Expand Up @@ -68,7 +68,7 @@
? AllWebStateListObservationRegistrar::Mode::INCOGNITO
: AllWebStateListObservationRegistrar::Mode::REGULAR;
registrar_ = std::make_unique<AllWebStateListObservationRegistrar>(
chrome_browser_state, std::make_unique<Observer>(this), mode);
browser_list, std::make_unique<Observer>(this), mode);

RegisterMessageCallbacks();
}
Expand Down
Expand Up @@ -11,6 +11,7 @@
#import "components/keyed_service/core/keyed_service.h"
#import "components/keyed_service/ios/browser_state_dependency_manager.h"
#import "ios/chrome/browser/shared/model/browser/all_web_state_list_observation_registrar.h"
#import "ios/chrome/browser/shared/model/browser/browser_list_factory.h"
#import "ios/chrome/browser/shared/model/browser_state/browser_state_otr_helper.h"
#import "ios/chrome/browser/shared/model/browser_state/chrome_browser_state.h"
#import "ios/chrome/browser/web/features.h"
Expand All @@ -27,6 +28,7 @@
class WebSessionStateCacheWrapper : public KeyedService {
public:
explicit WebSessionStateCacheWrapper(
// TODO(crbug.com/1450912): Pass a BrowserList directly instead.
ChromeBrowserState* browser_state,
WebSessionStateCache* web_session_state_cache);

Expand Down Expand Up @@ -54,8 +56,9 @@ explicit WebSessionStateCacheWrapper(
: web_session_state_cache_(web_session_state_cache) {
DCHECK(web_session_state_cache);
registrar_ = std::make_unique<AllWebStateListObservationRegistrar>(
browser_state, std::make_unique<WebSessionStateCacheWebStateListObserver>(
web_session_state_cache));
BrowserListFactory::GetForBrowserState(browser_state),
std::make_unique<WebSessionStateCacheWebStateListObserver>(
web_session_state_cache));
}

WebSessionStateCacheWrapper::~WebSessionStateCacheWrapper() {
Expand Down

0 comments on commit 96546d2

Please sign in to comment.