[Sync] Fix flaky PasswordManagerSyncTest on Linux ASAN [chromium/src : main]

[Colin Blundell (Gerrit)]

Attention needed from Marc Treib and plantree

Colin Blundell added 1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Colin Blundell . resolved

Thanks! -> a closer OWNER (who can add a second reviewer as needed)

Open in Gerrit

Related details

Attention is currently required from:

• Marc Treib
• plantree

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: I9877402bd7f7a950118e9e9fa82c820076710ce4

Gerrit-Change-Number: 7874503

Gerrit-PatchSet: 3

Gerrit-Owner: plantree <pengyu...@microsoft.com>

Gerrit-Reviewer: Marc Treib <tr...@chromium.org>

Gerrit-Reviewer: plantree <pengyu...@microsoft.com>

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

Gerrit-CC: android-bu...@system.gserviceaccount.com <android-bu...@system.gserviceaccount.com>

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

Gerrit-Attention: plantree <pengyu...@microsoft.com>

Gerrit-Attention: Marc Treib <tr...@chromium.org>

Gerrit-Comment-Date: Wed, 27 May 2026 07:08:36 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Marc Treib (Gerrit)]

Attention needed from plantree

Marc Treib added 2 comments

File chrome/browser/sync/test/integration/password_manager_sync_test.cc

Line 743, Patchset 3 (Latest): // OnPasswordFormsRendered; otherwise the save prompt may not appear.

Marc Treib . unresolved

How does this call guarantee that? IIUC, at this point, both IPCs are already racing?

Line 779, Patchset 3 (Latest): // See OfferToSaveNonPrimaryAccountCredential for why this is needed.

Marc Treib . unresolved

nit: I'd rather copy the original two-line comment here. As it is, it's likely that the other test will get modified at some point, and then this would become a dangling reference.

Open in Gerrit

Related details

Attention is currently required from:

• plantree

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: I9877402bd7f7a950118e9e9fa82c820076710ce4

Gerrit-Change-Number: 7874503

Gerrit-PatchSet: 3

Gerrit-Owner: plantree <pengyu...@microsoft.com>

Gerrit-Reviewer: Marc Treib <tr...@chromium.org>

Gerrit-Reviewer: plantree <pengyu...@microsoft.com>

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

Gerrit-CC: android-bu...@system.gserviceaccount.com <android-bu...@system.gserviceaccount.com>

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

Gerrit-Attention: plantree <pengyu...@microsoft.com>

Gerrit-Comment-Date: Wed, 27 May 2026 08:33:05 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[plantree (Gerrit)]

Attention needed from Marc Treib

plantree added 2 comments

File chrome/browser/sync/test/integration/password_manager_sync_test.cc

Line 743, Patchset 3 (Latest): // OnPasswordFormsRendered; otherwise the save prompt may not appear.

Marc Treib . unresolved

How does this call guarantee that? IIUC, at this point, both IPCs are already racing?

plantree

You're right that both IPCs are already in flight by this point, and RunAllTasksUntilIdle can't retroactively fix the ordering between PasswordFormSubmitted (Mojo) and DidNavigateMainFrame (WebContentsObserver) — that race already played out during observer.Wait() inside FillAndSubmitPasswordForm. Let me rethink the approach. Would it make sense to move the flush inside FillAndSubmitPasswordForm (before DidFinishLoad triggers), or is there a better synchronization point you'd suggest?

Line 779, Patchset 3 (Latest): // See OfferToSaveNonPrimaryAccountCredential for why this is needed.

Marc Treib . unresolved

nit: I'd rather copy the original two-line comment here. As it is, it's likely that the other test will get modified at some point, and then this would become a dangling reference.

plantree

That makes sense. Let's address the previous comment first.

Open in Gerrit

Related details

Attention is currently required from:

• Marc Treib

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: I9877402bd7f7a950118e9e9fa82c820076710ce4

Gerrit-Change-Number: 7874503

Gerrit-PatchSet: 3

Gerrit-Owner: plantree <pengyu...@microsoft.com>

Gerrit-Reviewer: Marc Treib <tr...@chromium.org>

Gerrit-Reviewer: plantree <pengyu...@microsoft.com>

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

Gerrit-CC: android-bu...@system.gserviceaccount.com <android-bu...@system.gserviceaccount.com>

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

Gerrit-Attention: Marc Treib <tr...@chromium.org>

Gerrit-Comment-Date: Mon, 01 Jun 2026 02:41:52 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Marc Treib <tr...@chromium.org>