[PWA Migration] Implement changes to WebAppInstallFinalizer [chromium/src : main]

[Marijn Kruisselbrink (Gerrit)]

Attention needed from Daniel Murphy

New activity on the change

Open in Gerrit

Related details

Attention is currently required from:

• Daniel Murphy

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

Gerrit-Change-Number: 7417993

Gerrit-PatchSet: 14

Gerrit-Owner: Marijn Kruisselbrink <m...@chromium.org>

Gerrit-Reviewer: Daniel Murphy <dmu...@chromium.org>

Gerrit-Reviewer: Marijn Kruisselbrink <m...@chromium.org>

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

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

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Dibyajyoti Pal <diby...@chromium.org>

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

Gerrit-Attention: Daniel Murphy <dmu...@chromium.org>

Gerrit-Comment-Date: Tue, 13 Jan 2026 23:30:47 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: No

[Daniel Murphy (Gerrit)]

Attention needed from Marijn Kruisselbrink

Daniel Murphy voted and added 6 comments

Votes added by Daniel Murphy

Code-Review

+1

6 comments

File chrome/browser/web_applications/web_app_install_finalizer.cc

Line 290, Patchset 15 (Latest): !web_app_info.validated_scope_extensions.has_value();

Daniel Murphy . unresolved

note: we allow installers to... pre-validate scope extensions, IDK if we need that for our feature here, but just noting that we don't do that for migration. seems good to me.

Line 354, Patchset 15 (Latest): validated_origin_associations.migration_sources;

Daniel Murphy . unresolved

nit: you can probably just inline this?

Line 661, Patchset 15 (Latest): validated_origin_associations.migration_sources;

Daniel Murphy . unresolved

nit: inline maybe?

File chrome/browser/web_applications/web_app_install_finalizer_unittest.cc

Line 683, Patchset 15 (Latest): .WillOnce([](base::OnceClosure callback, const base::Location& location) {

Daniel Murphy . unresolved

nit: I "think" you can maybe do WillOnce(base::RunCallback<0>()) to simplify this?
From https://source.chromium.org/chromium/chromium/src/+/main:base/test/gmock_callback_support.h;l=168?q=gmock%20RunCallback&sq=&ss=chromium

or RunClosure actually? https://source.chromium.org/chromium/chromium/src/+/main:base/test/gmock_callback_support.h;l=90?q=gmock%20RunCallback&ss=chromium

Line 774, Patchset 15 (Latest): std::move(callback).Run();

Daniel Murphy . unresolved

nit: same as above

File chrome/browser/web_applications/web_app_install_info.h

Line 526, Patchset 15 (Latest): std::vector<proto::WebAppMigrationSource> migration_sources;

Daniel Murphy . unresolved

ultra possible future nit: wrap this with something in the model/ directory, to help with invariant enforcement.

Open in Gerrit

Related details

Attention is currently required from:

• Marijn Kruisselbrink

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

Gerrit-Change-Number: 7417993

Gerrit-PatchSet: 15

Gerrit-Owner: Marijn Kruisselbrink <m...@chromium.org>

Gerrit-Reviewer: Daniel Murphy <dmu...@chromium.org>

Gerrit-Reviewer: Marijn Kruisselbrink <m...@chromium.org>

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

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

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Dibyajyoti Pal <diby...@chromium.org>

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

Gerrit-Attention: Marijn Kruisselbrink <m...@chromium.org>

Gerrit-Comment-Date: Wed, 14 Jan 2026 22:14:28 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Marijn Kruisselbrink (Gerrit)]

Marijn Kruisselbrink added 6 comments

File chrome/browser/web_applications/web_app_install_finalizer.cc

Line 290, Patchset 15: !web_app_info.validated_scope_extensions.has_value();

Daniel Murphy . resolved

note: we allow installers to... pre-validate scope extensions, IDK if we need that for our feature here, but just noting that we don't do that for migration. seems good to me.

Marijn Kruisselbrink

yeah, I started adding it for migrations too, but wasn't sure what the use case would be, so it seemed simpler to leave out (at least some of the scope extensions pre-validation code also has "remove this when ManifestUpdateCheckCommand is deleted" comments, at least in the update path...)

Line 354, Patchset 15: validated_origin_associations.migration_sources;

Daniel Murphy . resolved

nit: you can probably just inline this?

Marijn Kruisselbrink

Done

Line 661, Patchset 15: validated_origin_associations.migration_sources;

Daniel Murphy . resolved

nit: inline maybe?

Marijn Kruisselbrink

Done

File chrome/browser/web_applications/web_app_install_finalizer_unittest.cc

Line 683, Patchset 15: .WillOnce([](base::OnceClosure callback, const base::Location& location) {

Daniel Murphy . resolved

nit: I "think" you can maybe do WillOnce(base::RunCallback<0>()) to simplify this?
From https://source.chromium.org/chromium/chromium/src/+/main:base/test/gmock_callback_support.h;l=168?q=gmock%20RunCallback&sq=&ss=chromium

or RunClosure actually? https://source.chromium.org/chromium/chromium/src/+/main:base/test/gmock_callback_support.h;l=90?q=gmock%20RunCallback&ss=chromium

Marijn Kruisselbrink

Done

Line 774, Patchset 15: std::move(callback).Run();

Daniel Murphy . resolved

nit: same as above

Marijn Kruisselbrink

Done

File chrome/browser/web_applications/web_app_install_info.h

Line 526, Patchset 15: std::vector<proto::WebAppMigrationSource> migration_sources;

Daniel Murphy . resolved

ultra possible future nit: wrap this with something in the model/ directory, to help with invariant enforcement.

Marijn Kruisselbrink

yeah, just returning the raw proto type in WebApp isn't great either. I filed b/475920501 to consider improving this.

Open in Gerrit

Related details

Attention set is empty

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

Gerrit-Change-Number: 7417993

Gerrit-PatchSet: 15

Gerrit-Owner: Marijn Kruisselbrink <m...@chromium.org>

Gerrit-Reviewer: Daniel Murphy <dmu...@chromium.org>

Gerrit-Reviewer: Marijn Kruisselbrink <m...@chromium.org>

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

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

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Dibyajyoti Pal <diby...@chromium.org>

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

Gerrit-Comment-Date: Wed, 14 Jan 2026 23:19:31 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Daniel Murphy <dmu...@chromium.org>

[Marijn Kruisselbrink (Gerrit)]

Marijn Kruisselbrink voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

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

Gerrit-Change-Number: 7417993

Gerrit-PatchSet: 16

Gerrit-Owner: Marijn Kruisselbrink <m...@chromium.org>

Gerrit-Reviewer: Daniel Murphy <dmu...@chromium.org>

Gerrit-Reviewer: Marijn Kruisselbrink <m...@chromium.org>

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

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

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Dibyajyoti Pal <diby...@chromium.org>

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

Gerrit-Comment-Date: Wed, 14 Jan 2026 23:19:44 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: Yes

[Chromium LUCI CQ (Gerrit)]

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

15 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: chrome/browser/web_applications/web_app_install_finalizer.cc
Insertions: 4, Deletions: 8.

The diff is too large to show. Please review the diff.
```
```
The name of the file: chrome/browser/web_applications/web_app_install_finalizer_unittest.cc
Insertions: 4, Deletions: 10.

The diff is too large to show. Please review the diff.
```

Change information

Commit message:

[PWA Migration] Implement changes to WebAppInstallFinalizer

This adds (unvalidated) migration sources as loaded from the
manifest to WebAppInstallInfo, and updates WebAppInstallFinalizer
to populate this in the resulting WebApp. Additionally
ResolveWebAppPendingMigrationInfoCommand is triggered any time
the validated migration sources change.

WebAppInstallFinalizer will reject installing an app with install
state SUGGESTED_FROM_MIGRATION if no migration sources are
specified in the install info, but will let the install continue
if all of the migration sources fail validation. More general
cleanup of outdated SUGGESTED_FROM_MIGRATION apps will be implemented
separately.

Bug: 475544460

Change-Id: Idfb113542499328b5d099dd8c77ba0e2c275dff0

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7417993

Commit-Queue: Marijn Kruisselbrink <m...@chromium.org>

Reviewed-by: Daniel Murphy <dmu...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#1569435}

Files:

• M chrome/browser/sync/test/integration/web_apps/two_client_web_apps_sync_test.cc
• M chrome/browser/web_applications/commands/install_app_locally_command_unittest.cc
• M chrome/browser/web_applications/commands/launch_web_app_command_browsertest.cc
• M chrome/browser/web_applications/commands/os_integration_synchronize_command_unittest.cc
• M chrome/browser/web_applications/commands/set_user_display_mode_command_unittest.cc
• M chrome/browser/web_applications/manifest_update_utils.cc
• M chrome/browser/web_applications/model/web_app_comparison.cc
• M chrome/browser/web_applications/test/fake_web_app_origin_association_manager.cc
• M chrome/browser/web_applications/test/fake_web_app_origin_association_manager.h
• M chrome/browser/web_applications/web_app_command_scheduler.h
• M chrome/browser/web_applications/web_app_install_finalizer.cc
• M chrome/browser/web_applications/web_app_install_finalizer_unittest.cc
• M chrome/browser/web_applications/web_app_install_info.h
• M chrome/browser/web_applications/web_app_install_utils.cc
• M components/webapps/browser/install_result_code.cc
• M components/webapps/browser/install_result_code.h
• M tools/metrics/histograms/enums.xml

Change size: L

Delta: 17 files changed, 287 insertions(+), 15 deletions(-)

Branch: refs/heads/main

Submit Requirements:

• Code-Review: +1 by Daniel Murphy

Open in Gerrit

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

Gerrit

Gerrit-MessageType: merged

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: Idfb113542499328b5d099dd8c77ba0e2c275dff0

Gerrit-Change-Number: 7417993

Gerrit-PatchSet: 17

Gerrit-Owner: Marijn Kruisselbrink <m...@chromium.org>

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

Gerrit-Reviewer: Daniel Murphy <dmu...@chromium.org>

Gerrit-Reviewer: Marijn Kruisselbrink <m...@chromium.org>

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