docs/chrome_browser_design_principles.md
//chrome/browser design principlesThese design principles make it easier to write, debug, and maintain code in
//chrome/browser.
//chrome/OWNERS.//chrome/browser and the rest
of the code base, see Google internal
documentation.There are 4 major conceptual primitives: profile, browser window, tab, and
WebContents.
A profile logically represents all data and settings associated with a single user. This includes all cookies, saved passwords, browsing history, installed extensions, etc. Multiple profiles may represent a single person who wants to have multiple different tranches of data, or may represent multiple people who are sharing a single device and want to keep their data separated.
A browser window is the core UI surface for Chrome. There exist other windows (e.g. profile picker), but the majority of user time in Chrome is spent interacting with browser windows.
A browser window contains a set of tabs, which can be navigated to websites.
WebContents is the core primitive vended by //content. It renders websites.
Note that each tab corresponds to exactly 1 WebContents, but not every
WebContents is a tab. For example, the profile picker in a standalone window
uses a WebContents to render the UI, but it is not a tab, and is not associated
with a browser window.
The expression of these primitives in code:
BrowserProcess,
specifically GlobalFeatures. For example, there is a single network stack,
and a single graphics stack.class Profile represents a profile. State is stored in
ProfileKeyedService.class BrowserWindowInterface represents a browser window. State is stored in
BrowserWindowFeatures.class TabInterface represents a tab, and state is stored in TabFeatures.For legacy reasons, class Browser is a god-object anti-pattern that is used
prolifically within //chrome/browser. Project Bedrock is an umbrella project
that covers several workstreams to fix this. See Google internal
documentation. Public contributors can visit
Project Bedrock -
Public.
To keep the dependency graph as precise as possible, we use a pattern called
UnownedUserData. This allows consumers of
BrowserWindowInterface and TabInterface to depend on a specific
BrowserWindowFeature or TabFeature without depending on features. This also
allows for easier unit and integration testing.
All features should be designed in accordance with the above architecture.
The following examples demonstrate best practices for Tab feature and Browser feature development, including dependency injection, construction, and modular BUILD.gn setup.
JsOptimizationsPageActionControllerJsOptimizationsPageActionController
[link]
is a simple tab-scoped feature that controls the visibility of a page action
icon.
Browser* by taking
exactly what it needs in its constructor, being tabs::TabInterface& and
page_actions::PageActionController&.TabFeatures::Init().chrome/browser/ui/views/js_optimization/BUILD.gn
[link],
defining separate js_optimization and impl targets for its headers and
implementation respectively.// chrome/browser/ui/views/js_optimization/js_optimizations_page_action_controller.h
class JsOptimizationsPageActionController : public tabs::ContentsObservingTabFeature {
public:
JsOptimizationsPageActionController(
tabs::TabInterface& tab_interface,
page_actions::PageActionController& page_action_controller);
...
};
// chrome/browser/ui/tabs/tab_features.cc
js_optimizations_page_action_controller_ =
std::make_unique<JsOptimizationsPageActionController>(
tab, *page_action_controller_);
# chrome/browser/ui/views/js_optimization/BUILD.gn
source_set("js_optimization") {
sources = [ "js_optimizations_page_action_controller.h" ]
public_deps = [
"//chrome/browser/ui/tabs",
"//components/tabs:public",
]
}
source_set("impl") {
sources = [ "js_optimizations_page_action_controller.cc" ]
public_deps = [
":js_optimization",
"//chrome/browser:primitives",
"//chrome/browser/site_protection",
"//chrome/browser/site_protection:utils",
"//chrome/browser/ui/actions",
"//chrome/browser/ui/tabs",
"//chrome/browser/ui/views",
"//chrome/browser/ui/views/page_action",
]
}
CommerceUiTabHelperCommerceUiTabHelper
[link]
is a more complex tab-scoped feature that manages tab-specific state and
operations for commerce features. It requires several dependencies and
integrates with TabFeatures::GetUserDataFactory().
ShoppingService*, in its constructor.TabFeatures::Init() using
GetUserDataFactory().CreateInstance<...>().
TabInterface::GetUnownedUserDataHost().CommerceUiTabHelper::From(TabInterface*).chrome/browser/ui/commerce/BUILD.gn
[link],
defining a commerce target for headers for itself and other classes in its
directory, with a separate impl.// chrome/browser/ui/commerce/commerce_ui_tab_helper.h
class CommerceUiTabHelper : public tabs::ContentsObservingTabFeature {
public:
CommerceUiTabHelper(tabs::TabInterface& tab_interface,
ShoppingService* shopping_service,
bookmarks::BookmarkModel* model,
image_fetcher::ImageFetcher* image_fetcher,
SidePanelRegistry* side_panel_registry);
...
};
// chrome/browser/ui/tabs/tab_features.cc
commerce_ui_tab_helper_ =
GetUserDataFactory().CreateInstance<commerce::CommerceUiTabHelper>(
tab, tab,
commerce::ShoppingServiceFactory::GetForBrowserContext(profile),
BookmarkModelFactory::GetForBrowserContext(profile),
ImageFetcherServiceFactory::GetForKey(profile->GetProfileKey())
->GetImageFetcher(image_fetcher::ImageFetcherConfig::kNetworkOnly),
side_panel_registry_.get());
SessionServiceTabGroupSyncObserverSessionServiceTabGroupSyncObserver
[link]
is a simple browser-scoped feature that observes tab group sync events and
updates the session service.
Profile*, TabStripModel*, and SessionID.BrowserWindowFeatures::Init().chrome/browser/ui/tabs/saved_tab_groups/BUILD.gn
[link],
defining a saved_tab_groups rule for headers with a corresponding separate
impl.// chrome/browser/ui/tabs/saved_tab_groups/session_service_tab_group_sync_observer.h
class SessionServiceTabGroupSyncObserver
: public TabGroupSyncService::Observer {
public:
SessionServiceTabGroupSyncObserver(Profile* profile,
TabStripModel* tab_strip_model,
SessionID session_id);
...
};
// chrome/browser/ui/browser_window/internal/browser_window_features.cc
session_service_tab_group_sync_observer_ =
std::make_unique<tab_groups::SessionServiceTabGroupSyncObserver>(
profile, browser->GetTabStripModel(), browser->GetSessionID());
VerticalTabStripStateControllerVerticalTabStripStateController
[link]
is a complex browser-scoped feature that manages the state of the vertical tab
strip.
BrowserWindowInterface*, PrefService*, and
SessionService*.BrowserWindowFeatures::Init()
using GetUserDataFactory().CreateInstance<...>().
BrowserWindowInterface::GetUnownedUserDataHost()VerticalTabStripStateController::From(BrowserWindowInterface*).chrome/browser/ui/tabs/BUILD.gn
[link],
defining a tabs rule for headers with a separate impl.// chrome/browser/ui/tabs/vertical_tab_strip_state_controller.h
class VerticalTabStripStateController : public SessionServiceBaseObserver,
public BrowserListObserver {
public:
DECLARE_USER_DATA(VerticalTabStripStateController);
explicit VerticalTabStripStateController(
BrowserWindowInterface* browser_window,
PrefService* pref_service,
actions::ActionItem* root_action_item,
SessionService* session_service,
SessionID session_id,
std::optional<bool> restored_state_collapsed,
std::optional<int> restored_state_uncollapsed_width);
...
};
// chrome/browser/ui/browser_window/internal/browser_window_features.cc
vertical_tab_strip_state_controller_ =
GetUserDataFactory().CreateInstance<tabs::VerticalTabStripStateController>(
*browser, browser, profile->GetPrefs(),
browser_actions_->root_action_item(),
SessionServiceFactory::GetForProfile(browser_->GetProfile()),
browser_->GetSessionID(), restored_state_collapsed,
restored_state_uncollapsed_width);
All features should be logically grouped in the directory structure with well-defined API surfaces. Dependencies should be injected during construction.
Modularity has many positive externalities:
Requirements:
//component/<feature> and //chrome/browser/<feature> (or
//chrome/browser/ui/<feature>), and not in //chrome/browser/ui/views.
//chrome/browser
and //chrome/browser/ui has been removed.//chrome/browser has been
removed.//chrome/browser/resources/<feature> alongside standalone BUILD.gn
files. This keeps all html/TS/CSS code in //chrome/browser in one
place.//chrome/browser or //chrome/browser/ui/ must
have a standalone BUILD.gn and OWNERS file.
BUILD.gn.//chrome/browser/BUILD.gn:browser,
//chrome/test/BUILD.gn or //chrome/browser/ui/BUILD.gn:ui.//chrome/browser:browser and
//chrome/browser/ui:ui. These circular dependencies will
disappear when all sources are removed from //chrome/browser:browser and //chrome/browser/ui:ui.//chrome/browser/ui:ui,
which will no longer be necessary once NTP is also modularized (crbug.com/382237520).chrome::FindBrowserWithTab (and everything in browser_finder.h)GetBrowserViewForNativeWindow (via browser_view.h)FindBrowserWindowWithWebContents (via browser_window.h)class Browser . This is a god-object anti-pattern that makes
modularization impossible.// Do not do this:
FooFeature(Browser* browser) : browser_(browser) {}
FooFeature::DoStuff() { DoStuffWith(browser_->profile()->GetPrefs()); }
// Do this:
FooFeature(PrefService* prefs) : prefs_(prefs) {}
FooFeature::DoStuff() { DoStuffWith(prefs_); }
TabFeatures (member of TabModel)
content::WebContentsUserData, it should be done in this class.TabHelpers::AttachTabHelpers will become a
remove-only method. Clank/WebView may continue to use section 2 of
TabHelpers::AttachTabHelpers (Clank/WebView only).content::WebContentsUserData is an
anti-pattern.BrowserWindowFeatures (member of Browser)
TabStripModel). These states can be observed on the
TabInterface.
More than one tab can be visible but only one can be active since
multiple content::WebContents can be visible at once.BrowserContextKeyedServiceFactory (functionally a member of Profile)
ServiceIsCreatedWithBrowserContext to return true. This
guarantees precise lifetime semantics.content::ContentMain(). Test
harnesses use a test-suite dependent start-point, e.g.
base::LaunchUnitTests. Tests typically instantiate a subset of
the lazily-instantiated factories instantiated by production code,
at a different time, with a different order. This results in
different, sometimes broken behavior in tests. This is typically
papered over by modifying the production logic to include
otherwise unnecessary conditionals, typically early-exits.
Overriding ServiceIsCreatedWithBrowserContext guarantees
identical behavior between test and production code.TestingProfile::Builder::AddTestingFactory to stub or fake
services.Profile.NoDestructor singleton.// Properly scoped state avoids categories of bugs and subtle issues. For
// example: one common mistake is to store window-scoped state on a tab-scoped
// object. This results in subtle bugs (or crashes) when a tab is dragged into a
// new window.
// Do not do this:
FooTabFeature {
// As per (1) above, we shouldn't be passing or storing Browser* anyways.
// Another common anti-pattern is to dynamically look up Browser* via
// browser_finder methods. These often result in the wrong Browser*
Browser* browser_;
// This will point to the wrong BrowserView if the tab is dragged into a new
// window. Furthermore, BrowserView* itself is a container of hundreds of
// other views. The tab-scoped feature typically wants something much more
// specific.
BrowserView* browser_view_;
// Extensions are profile-scoped, and thus any state/logic should be in a
// ProfileKeyedService. If the user has multiple tabs (possibly across
// multiple windows) simultaneously interacting with FooTabFeature, then it's
// likely that the extension will uninstall while it's still in use.
void InstallExtension();
void UninstallExtension();
};
// Instead do this:
FooTabFeature {
// Guaranteed to remain valid for the lifetime of this class. This can be used
// to dynamically access relevant window state via
// tab_->GetBrowserWindowInterface()->GetFeatures().GetFooWindowFeature()
TabInterface* tab_;
};
FooService : public KeyedService {
void InstallExtension();
void UninstallExtension();
};
base::UTF8ToWide()IsFooFeatureEnabled() wraps the global variable
BASE_FEATURE(kFooFeature,...)BrowserList.//chrome/browser/BUILD.gn and
//chrome/browser/ui/BUILD.gn
ui subdirectory, and plenty of
non-UI code inside of the ui subdirectory. Currently the two BUILD files
allow circular includes. We will continue to treat these directories and
BUILD files as interchangeable.ContentsContainerView. Each ContentsContainerView
contains a single ContentsWebView.// Good, complexity is O(N) and no tight coupling using a well-understood and
// common design pattern: modality. Similar logic will be needed in other modal
// UIs. The logic in this class does not change regardless of how many other new
// modal features are created.
class Sparkles {
// Shows sparkles over the entire tab. Should not be shown over other Chrome
// contents (e.g. print preview)
void Show() {
if (tab_->CanShowModalUI()) {
MakeSparkles();
// Prevents other modals from showing until the member is reset.
modal_ui_ = tab_->ShowModalUI();
}
}
std::unique_ptr<ScopedTabModalUI> modal_ui_;
raw_ptr<TabInterface> tab_;
};
// Bad. Introduces tight coupling between unrelated features. Similar logic is
// needed in PrintPreview and DevTools. Complexity will scale with O(N^2). When
// a new modal feature is created, every modal feature will need to be updated.
class Sparkles {
void Show() {
if (PrintPreview::Showing()) {
return;
}
if (DevTools::Showing()) {
return;
}
MakeSparkles();
}
};
#if BUILDFLAG(FEATURE_FOO). Use
runtime logic (e.g. base::Feature or API keys) or build-time GN
conditionals, e.g. if (is_win).
#if BUILDFLAG(OS_WIN)#if BUILDFLAG(GOOGLE_CHROME_BRANDING)#if !defined(NDEBUG)#if defined(ADDRESS_SANITIZER)BUILDFLAG(ENABLE_EXTENSIONS)) to glue this into the
main source is allowed. The glue code should be kept to a minimum.gn args.CHECK_IS_TEST().// Good. Depending on context, this can be broken into separate tests.
bool IsPrime(int input);
TEST(Math, CheckIsPrime) {
EXPECT_TRUE(IsPrime(2));
EXPECT_TRUE(IsPrime(3));
EXPECT_FALSE(IsPrime(99));
EXPECT_FALSE(IsPrime(-2));
EXPECT_FALSE(IsPrime(0));
EXPECT_FALSE(IsPrime(1));
}
// Bad. This is a change detector test. Change detector tests are easy to spot
// because the test logic looks the same as the production logic.
class ShowButtonOnBrowserActivation : public BrowserActivationListener {
void ShowButton();
bool DidShowButton();
// BrowserActivationListener overrides:
void BrowserDidActivate() override {
ShowButton();
}
};
Test(ShowButtonOnBrowserActivationTest, ShowButtonOnActivate) {
ShowButtonOnBrowserActivation listener;
listener.BrowserDidActivate();
EXPECT_TRUE(listener.DidShowButton());
}
base::Callback, execution should either be always synchronous, or
always asynchronous. Mixing the two means callers have to deal with two distinct
control flows, which often leads to incorrect handling of one.
Avoid the following:if (result.cached) {
RunCallbackSync()
} else {
GetResultAndRunCallbackAsync()
}
class CarFactory {
std::unique_ptr<Car> CreateCar() {
if (!CanCreateCar()) {
return nullptr;
}
if (FactoryIsBusy() && !delegate->ShouldShowCarIfFactoryIsBusy()) {
return nullptr;
}
return std::make_unique<Car>();
}
bool CanCreateCar();
bool FactoryIsBusy();
Delegate* delegate_ = nullptr;
};
class CarSalesPerson : public Delegate {
// Can return nullptr, in which case no car is shown.
std::unique_ptr<Car> ShowCar() {
return car_factory_->CreateCar();
}
// Delegate overrides:
// Whether the car should be shown, even if the factory is busy.
bool ShouldShowCarIfFactoryIsBusy() override;
CarFactory* car_factory_ = nullptr;
};
Good, version 1: Remove delegation. Pass all relevant state to CarFactory so that CreateCar() does not depend on non-local state.
class CarFactory {
std::unique_ptr<Car> CreateCar(bool show_even_if_factory_is_busy) {
if (!CanCreateCar()) {
return nullptr;
}
if (FactoryIsBusy() && !show_even_if_factory_is_busy) {
return nullptr;
}
return std::make_unique<Car>();
}
bool CanCreateCar();
bool FactoryIsBusy();
};
class CarSalesPerson {
// Can return nullptr, in which case no car is shown.
std::unique_ptr<Car> ShowCar() {
return car_factory_->CreateCar(ShouldShowCarIfFactoryIsBusy());
}
// Whether the car should be shown, even if the factory is busy.
bool ShouldShowCarIfFactoryIsBusy();
CarFactory* car_factory_ = nullptr;
};
Good, version 2: Remove delegation. CreateCar always creates a car (fewer conditionals). State only flows from CarFactory to CarSalesPerson (and never backwards).
class CarFactory {
bool CanCreateCar();
bool FactoryIsBusy();
// Never returns nullptr.
std::unique_ptr<Car> CreateCar();
};
class CarSalesPerson {
// Can return nullptr, in which case no car is shown
std::unique_ptr<Car> ShowCar() {
if (!car_factory_->CanCreateCar()) {
return nullptr;
}
if (car_factory_->FactoryIsBusy() && !ShouldShowCarIfFactoryIsBusy()) {
return nullptr;
}
return car_factory_->CreateCar();
}
// Whether the car should be shown, even if the factory is busy.
bool ShouldShowCarIfFactoryIsBusy();
CarFactory* car_factory_ = nullptr;
};
Bad. FeatureShowcase depends on FeatureA. FeatureA depends on FeatureB. FeatureB depends on FeatureShowcase. The root problem is that the design for FeatureShowcase uses both a pull and a push model for control flow.
// Shows an icon per feature. Needs to know whether each icon is visible.
class FeatureShowcase {
FeatureShowcase() {
// Checks whether A should be visible, and if so, shows A.
if (FeatureA::IsVisible())
ShowFeatureA();
}
// Called to make B visible.
void ShowFeatureB();
};
class FeatureA {
// Feature A depends on feature B.
FeatureA(FeatureB* b);
static bool IsVisible();
};
class FeatureB {
FeatureB(FeatureShowcase* showcase) {
if (IsVisible())
showcase->ShowFeatureB();
}
static bool IsVisible();
};
Good, version 1. FeatureShowcase uses a pull model for control flow. FeatureShowcase depends on FeatureA and FeatureB. FeatureA depends on FeatureB. There is no circular dependency.
// Shows an icon per feature. Needs to know whether each icon is visible.
class FeatureShowcase {
FeatureShowcase() {
if (FeatureA::IsVisible())
ShowFeatureA();
if (FeatureB::IsVisible())
ShowFeatureB();
}
};
class FeatureA {
// Feature A depends on feature B.
FeatureA(FeatureB* b);
static bool IsVisible();
};
class FeatureB {
FeatureB();
static bool IsVisible();
};
Good, version 2. FeatureShowcase uses a push model for control flow. FeatureA and FeatureB both depend on FeatureShowcase. There is no circular dependency.
// Shows an icon per feature. Needs to know whether each icon is visible.
class FeatureShowcase {
FeatureShowcase();
// Called to make A visible.
void ShowFeatureA();
// Called to make B visible.
void ShowFeatureB();
};
class FeatureA {
// Feature A depends on feature B.
FeatureA(FeatureB* b, FeatureShowcase* showcase) {
if (IsVisible())
showcase->ShowFeatureA();
}
static bool IsVisible();
};
class FeatureB {
FeatureB(FeatureShowcase* showcase) {
if (IsVisible())
showcase->ShowFeatureB();
}
static bool IsVisible();
};