extensions: Port ViewExtensionSourceTest to desktop Android [chromium/src : main]
0 views
Skip to first unread message
James Cook (Gerrit)
May 29, 2026, 8:02:06 PM (3 days ago) May 29
to Zoraiz Naeem, Achuith Bhandarkar, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Zoraiz Naeem
James Cook added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention is currently required from:
- Zoraiz Naeem
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Zoraiz Naeem (Gerrit)
May 29, 2026, 8:41:05 PM (3 days ago) May 29
to James Cook, Achuith Bhandarkar, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from James Cook
Zoraiz Naeem added 2 comments![]( "Open in Gerrit")
File chrome/browser/extensions/view_extension_source_browsertest.cc
Line 34, Patchset 1 (Latest):
class TabAddedWaiter : public TabListInterfaceObserver {Zoraiz Naeem . unresolved
I remember that we defined some sort of waiter in a util file for extensions? (Most probably it wasnt for tab addition)
Line 37, Patchset 1 (Latest):
tab_list_->AddTabListInterfaceObserver(this);Zoraiz Naeem . unresolved
nit: can we used ScopedObservation here?
Related details
Attention is currently required from:
- James Cook
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. |
Achuith Bhandarkar (Gerrit)
May 30, 2026, 3:46:40 PM (2 days ago) May 30
to James Cook, Zoraiz Naeem, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from James Cook
Achuith Bhandarkar added 1 comment![]( "Open in Gerrit")
File chrome/browser/extensions/view_extension_source_browsertest.cc
Line 37, Patchset 1 (Latest):
tab_list_->AddTabListInterfaceObserver(this);Zoraiz Naeem . unresolved
nit: can we used ScopedObservation here?
Achuith Bhandarkar
lol
James Cook (Gerrit)
12:38 PM (4 hours ago) 12:38 PM
to Zoraiz Naeem, Achuith Bhandarkar, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Achuith Bhandarkar and Zoraiz Naeem
James Cook voted and added 3 comments![]( "Open in Gerrit")
Votes added by James Cook
| Auto-Submit | +1 |
3 comments
Patchset-level comments
File-level comment, Patchset 2 (Latest):
James Cook . resolved
Zoraiz, please take another look. Thanks.
File chrome/browser/extensions/view_extension_source_browsertest.cc
I remember that we defined some sort of waiter in a util file for extensions? (Most probably it wasnt for tab addition)
James Cook
I added BrowserCreatedWaiter a few months ago because it was used in a couple places. I was planning to leave this one here unless we need it in multiple places.
Achuith Bhandarkarnit: can we used ScopedObservation here?
lol
James Cook
In the spirit of "the reviewer is always right", done. However, please read my 1 page doc about why I don't like ScopedObservation. It can lead to hard-to-spot use-after-free. Maybe we can use AddObserver/RemoveObserver in the future?
Related details
Attention is currently required from:
- Achuith Bhandarkar
- Zoraiz Naeem
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Zoraiz Naeem (Gerrit)
1:51 PM (3 hours ago) 1:51 PM
to James Cook, Achuith Bhandarkar, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Achuith Bhandarkar and James Cook
Zoraiz Naeem voted and added 1 comment![]( "Open in Gerrit")
Votes added by Zoraiz Naeem
| Code-Review | +1 |
| Commit-Queue | +2 |
1 comment
Patchset-level comments
Attention is currently required from:
- Achuith Bhandarkar
- James Cook
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
1:55 PM (3 hours ago) 1:55 PM
to James Cook, Zoraiz Naeem, Achuith Bhandarkar, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
extensions: Port ViewExtensionSourceTest to desktop Android
This required migrating away from browser_commands.h which isn't used
on Android, hence chrome::CloseTab() and chrome::RestoreTab() aren't
available. It also required a custom TabAddedWaiter, as ui_test_utils
is also not available on Android.
Bug: 469417243
Change-Id: Ia93350941f9fc5c2f0e25a3a7fb70692f844efa8
Auto-Submit: James Cook <jame...@chromium.org>
Commit-Queue: Zoraiz Naeem <zorai...@chromium.org>
Reviewed-by: Zoraiz Naeem <zorai...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1639518}
Files:
- M chrome/browser/extensions/view_extension_source_browsertest.cc
- M chrome/test/BUILD.gn
Change size: M
Delta: 2 files changed, 77 insertions(+), 16 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Zoraiz Naeem
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages