extensions: Add test for window restore to sessions API [chromium/src : main]

0 views

Skip to first unread message

James Cook (Gerrit)

unread,

Jan 30, 2026, 6:04:51 PM (6 hours ago) Jan 30

to Zoraiz Naeem, Achuith Bhandarkar, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, mac-r...@chromium.org

Attention needed from Zoraiz Naeem

James Cook voted and added 3 comments

Votes added by James Cook

Commit-Queue

3 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

James Cook . resolved

Zoraiz, please take a look. Thanks!

File chrome/browser/extensions/api/sessions/sessions_apitest.cc

Line 89, Patchset 3: observation_.Observe(GlobalBrowserCollection::GetInstance());

James Cook . unresolved

FYI - In this case I can't do my usual AddObserver/RemoveObserver because BrowserCollection forces clients to use ScopedObservation.

Line 427, Patchset 3: // restore service.

James Cook . unresolved

This is a little odd, and does not match production behavior, but there are comments in TabRestoreServiceHelper code that say this happens in tests. Shrug.

Open in Gerrit

Related details

Attention is currently required from:

Zoraiz Naeem

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Achuith Bhandarkar (Gerrit)

unread,

Jan 30, 2026, 8:07:59 PM (4 hours ago) Jan 30

to James Cook, Zoraiz Naeem, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, mac-r...@chromium.org

Attention needed from James Cook and Zoraiz Naeem

Achuith Bhandarkar added 4 comments

File chrome/browser/extensions/api/sessions/sessions_apitest.cc

Line 87, Patchset 4 (Latest): explicit BrowserCloseWaiter(BrowserWindowInterface* browser)

Achuith Bhandarkar . unresolved

Don't you need to delete the copy ctor and assignment operator, and set the dtor to default?

Line 89, Patchset 3: observation_.Observe(GlobalBrowserCollection::GetInstance());

James Cook . resolved

FYI - In this case I can't do my usual AddObserver/RemoveObserver because BrowserCollection forces clients to use ScopedObservation.

Achuith Bhandarkar

Acknowledged

Line 100, Patchset 4 (Latest): void OnBrowserClosed(BrowserWindowInterface* browser) override {

Achuith Bhandarkar . unresolved

Does this need to be protected? We only want this called through the BrowserCollectionObserver interface, right?

Line 409, Patchset 4 (Latest):// supported.

Achuith Bhandarkar . unresolved

Since this is a long test, it would be useful to know what it's trying to do without having to read the code.

Open in Gerrit

Related details

Attention is currently required from:

James Cook
Zoraiz Naeem

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

0 new messages