Fix AuthScreenExtensionsExternalLoaderTest flakiness [chromium/src : main]

[Aida Zolić (Gerrit)]

Attention needed from Emilia Paz and Maria Petrisor

Aida Zolić voted and added 1 comment

Votes added by Aida Zolić

Commit-Queue

+1

1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Aida Zolić . resolved

@emil...@chromium.org, can you PTAL as @rdevlin...@chromium.org is OOO until next week?
@mpet...@chromium.org for Badge Auth context.

Thanks!

Open in Gerrit

Related details

Attention is currently required from:

• Emilia Paz
• Maria Petrisor

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• Review-Enforcement

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

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: Ic48294d077251d9ff0c6329360c213fa72a6193f

Gerrit-Change-Number: 7086688

Gerrit-PatchSet: 3

Gerrit-Owner: Aida Zolić <aida...@chromium.org>

Gerrit-Reviewer: Aida Zolić <aida...@chromium.org>

Gerrit-Reviewer: Emilia Paz <emil...@chromium.org>

Gerrit-Reviewer: Maria Petrisor <mpet...@chromium.org>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Devlin Cronin <rdevlin...@chromium.org>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Maria Petrisor <mpet...@chromium.org>

Gerrit-Attention: Emilia Paz <emil...@chromium.org>

Gerrit-Comment-Date: Mon, 27 Oct 2025 16:20:10 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Emilia Paz (Gerrit)]

Attention needed from Aida Zolić and Maria Petrisor

Emilia Paz added 3 comments

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Emilia Paz . resolved

Thanks Aida! Just one comment to make sure we are still testing the desired behavior, since I'm not familiar with lock screen installations

File chrome/browser/ash/extensions/authentication_screen_extensions_external_loader_browsertest.cc

Line 228, Patchset 3 (Latest): if (IsExtensionInstalledOnLockScreen(kExtensionId)) {
observer.WaitForExtensionLoaded();
}

Emilia Paz . unresolved

What happens when the extension is not installed on lock screen? Before we were always waiting for extension to be loaded, and now not anymore.
Is this wanted? Or should extensions be always be installed on lock screen here?

Line 236, Patchset 3 (Latest): if (IsExtensionInstalledOnLockScreen(kExtensionId)) {
observer.WaitForExtensionUnloaded();
}

Emilia Paz . unresolved

similar comment

Open in Gerrit

Related details

Attention is currently required from:

• Aida Zolić
• Maria Petrisor

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

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: Ic48294d077251d9ff0c6329360c213fa72a6193f

Gerrit-Change-Number: 7086688

Gerrit-PatchSet: 3

Gerrit-Owner: Aida Zolić <aida...@chromium.org>

Gerrit-Reviewer: Aida Zolić <aida...@chromium.org>

Gerrit-Reviewer: Emilia Paz <emil...@chromium.org>

Gerrit-Reviewer: Maria Petrisor <mpet...@chromium.org>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Devlin Cronin <rdevlin...@chromium.org>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Aida Zolić <aida...@chromium.org>

Gerrit-Attention: Maria Petrisor <mpet...@chromium.org>

Gerrit-Comment-Date: Mon, 27 Oct 2025 20:58:42 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Aida Zolić (Gerrit)]

Attention needed from Emilia Paz

Aida Zolić added 2 comments

File chrome/browser/ash/extensions/authentication_screen_extensions_external_loader_browsertest.cc

Line 228, Patchset 3: if (IsExtensionInstalledOnLockScreen(kExtensionId)) {
observer.WaitForExtensionLoaded();
}

Emilia Paz . resolved

What happens when the extension is not installed on lock screen? Before we were always waiting for extension to be loaded, and now not anymore.
Is this wanted? Or should extensions be always be installed on lock screen here?

Aida Zolić

Good catch, this was a copy-paste mistake here.

We should only wait if it's not yet installed.

Line 236, Patchset 3: if (IsExtensionInstalledOnLockScreen(kExtensionId)) {
observer.WaitForExtensionUnloaded();
}

Emilia Paz . resolved

similar comment

Aida Zolić

We only wait for the extension to be unloaded if it is still installed.

Open in Gerrit

Related details

Attention is currently required from:

• Emilia Paz

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review

• Review-Enforcement

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

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: Ic48294d077251d9ff0c6329360c213fa72a6193f

Gerrit-Change-Number: 7086688

Gerrit-PatchSet: 4

Gerrit-Owner: Aida Zolić <aida...@chromium.org>

Gerrit-Reviewer: Aida Zolić <aida...@chromium.org>

Gerrit-Reviewer: Emilia Paz <emil...@chromium.org>

Gerrit-Reviewer: Maria Petrisor <mpet...@chromium.org>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Devlin Cronin <rdevlin...@chromium.org>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Emilia Paz <emil...@chromium.org>

Gerrit-Comment-Date: Tue, 28 Oct 2025 08:14:02 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Emilia Paz <emil...@chromium.org>

[Emilia Paz (Gerrit)]

Attention needed from Aida Zolić and Maria Petrisor

Emilia Paz voted and added 1 comment

Votes added by Emilia Paz

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Emilia Paz . resolved

That makes sense now, lgtm. Thanks!

Open in Gerrit

Related details

Attention is currently required from:

• Aida Zolić
• Maria Petrisor

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• Review-Enforcement

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

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: Ic48294d077251d9ff0c6329360c213fa72a6193f

Gerrit-Change-Number: 7086688

Gerrit-PatchSet: 4

Gerrit-Owner: Aida Zolić <aida...@chromium.org>

Gerrit-Reviewer: Aida Zolić <aida...@chromium.org>

Gerrit-Reviewer: Emilia Paz <emil...@chromium.org>

Gerrit-Reviewer: Maria Petrisor <mpet...@chromium.org>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Devlin Cronin <rdevlin...@chromium.org>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Aida Zolić <aida...@chromium.org>

Gerrit-Attention: Maria Petrisor <mpet...@chromium.org>

Gerrit-Comment-Date: Wed, 29 Oct 2025 03:03:58 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes