agents/skills/browser-window-feature-refactor/SKILL.md
Use this skill for Project Bedrock refactors that modularize desktop Chrome
browser-window logic by moving ownership, methods, or initialization into
BrowserWindowFeatures (BWF) and its feature controllers.
BrowserWindowInterface, Profile,
TabStripModel, BrowserWindow, or feature-controller dependencies before
reaching for Browser* or GetBrowserForMigrationOnly().Use this when BrowserWindow/BrowserView/WebUIBrowserWindow exposes
cohesive feature-specific behavior and no existing BWF-owned controller matches
it.
Workflow:
Create the controller near the feature's UI domain.
Pass only the dependencies the controller needs. Prefer
BrowserWindowInterface*, Profile*, TabStripModel*, BrowserWindow*,
concrete view dependencies, or other feature controller dependencies owned by
BrowserWindowFeatures over Browser* when practical.
Add GN sources and deps to the narrow owning target. Verify a modular
BUILD.gn exists in the feature controller's own directory and add the
controller there, using public/sources separation (public headers in
public, implementation in sources). Do not cargo-cult the sources into a
monolithic target such as //chrome/browser or //chrome/browser/ui; if no
modular target exists yet, create one in the controller's directory.
Own the controller with a std::unique_ptr<FooController> member, and in the
same edit add a matching forward declaration class FooController; to the
BWF header (see step 5 — never skip it). Initialize the member in the
earliest correct lifecycle hook via the BrowserWindowFeatures user data
factory rather than std::make_unique. Configuring new controllers through
the factory keeps them easy to fake or override in tests. Add
Must be before/after comments when ordering matters.
Example:
foo_controller_ =
GetUserDataFactory().CreateInstance<FooController>(*browser, ...);
Always pair the std::unique_ptr<FooController> member with a forward
declaration in the same header. Add class FooController; alongside the
other forward declarations in the BWF header — never #include the
controller header (its #include belongs in the .cc). The
std::unique_ptr<FooController> member from step 4 names an incomplete type,
so omitting class FooController; breaks compilation. Adding the member
without the forward declaration is the most common mistake in this migration
— double-check the header has both before moving on.
The header needs two coupled edits — the forward declaration near the top and the member lower down. They live far apart, so it is easy to land the member but forget the forward declaration. Make both edits, as shown:
// browser_window_features.h
// Forward declarations (keep this block sorted).
class BarController;
class FooController; // <-- EDIT 1: add alongside the existing declarations.
class QuxController;
class BrowserWindowFeatures {
// ...
private:
std::unique_ptr<FooController> foo_controller_; // <-- EDIT 2: the member.
};
Before moving on, open the BWF header and confirm class FooController; is
present in the forward-declaration block. If it is missing, add it now — a
std::unique_ptr<FooController> member with no matching forward declaration
is a guaranteed compile failure.
Expose the controller through UnownedUserData: declare it with
DECLARE_USER_DATA(FooController), hold a
ui::ScopedUnownedUserData<FooController> member, and provide a static
From(browser) that returns the instance corresponding to the
BrowserWindowInterface. Do not add a public BWF accessor; even when
sibling controllers already expose accessors, do not mirror them for the new
controller; add one only if an existing caller genuinely needs it.
Example setup in foo_controller.h:
#include "ui/base/unowned_user_data/scoped_unowned_user_data.h"
class FooController {
public:
DECLARE_USER_DATA(FooController);
explicit FooController(BrowserWindowInterface* browser);
// Returns the instance owned by `browser`, or nullptr.
static FooController* From(BrowserWindowInterface* browser);
private:
ui::ScopedUnownedUserData<FooController> scoped_user_data_;
};
Matching implementation in foo_controller.cc:
FooController::FooController(BrowserWindowInterface* browser)
: scoped_user_data_(browser->GetUnownedUserDataHost(), *this) {}
// static
FooController* FooController::From(BrowserWindowInterface* browser) {
return Get(browser->GetUnownedUserDataHost());
}
Update all callsites — in both production code and tests — to reach the
controller through FooController::From(browser). Missing callsites stay
silent until step 8 removes the old API, then break as compilation errors in
production and test targets; refactor them now instead of later hunting for
the removed BrowserWindow methods.
Remove obsolete
BrowserWindow/BrowserView/WebUIBrowserWindow/test-window API.
Use this when the target feature already has a BWF-owned controller.
Workflow:
BrowserWindow or BrowserView virtual methods.BrowserWindow shim.Use this when a controller is already BWF-owned but constructed in a later hook than its dependencies require.
Lifecycle decision tree:
Init(): Feature does not require the concrete window object or view
hierarchy. It can depend on BrowserWindowInterface, Profile,
TabStripModel, session id, type, and other BWF state already created there.InitPostWindowConstruction(): Feature needs BrowserWindow, widget focus
manager, BrowserView vs WebUIBrowserWindow dispatch, the view hierarchy,
or window-level platform objects.TearDownPreBrowserWindowDestruction(): Feature has observers, raw pointers,
view/window dependencies, or explicit teardown requirements that must be
cleared before the window is destroyed.Before modifying BrowserWindowFeatures, read
bwf-ordering.md. Preserve the lifecycle and
ownership layout; do not mechanically alphabetize private members.
Avoid:
BrowserWindow virtual methods or thin BWF wrappers around
BrowserView.FooController::From(browser)).MANDATORY — do not report the task complete until this passes. For a new
feature controller, re-open the BWF header and confirm it literally contains
both class FooController; (forward-declaration block) and the
std::unique_ptr<FooController> member. These are two separate edits and a tool
edit can silently fail to land; if either is missing, re-apply it and re-read
the file to verify. Treat a header missing either line as a blocking failure,
not done — a member without its forward declaration (or vice versa) does not
compile.
In the final report, state:
GetBrowserForMigrationOnly() usage and why it remains;class FooController; and the std::unique_ptr<FooController> member;