Lu Huang (Gerrit)

unread,

Dec 17, 2025, 9:07:03 AM (2 days ago) Dec 17

to Kristin Lee, Lia Hiscock, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org

Attention needed from Kristin Lee and Lia Hiscock

Lu Huang added 2 comments

File chrome/browser/web_applications/install_element_browsertest.cc

Line 226, Patchset 2 (Latest):

  const GURL install_url =
      https_server()->GetURL("/web_apps/custom_id/install_url.html");

Lu Huang . unresolved

Is this a cross-origin URL? It's using the same server.

Line 269, Patchset 2 (Latest):

  EXPECT_EQ(install_future.Get<webapps::InstallResultCode>(),
            webapps::InstallResultCode::kSuccessNewInstall);

Lu Huang . unresolved

Also check
```
  EXPECT_EQ(provider().registrar_unsafe().GetAppIds().size(), 1u);
```

Open in Gerrit

Related details

Attention is currently required from:

Kristin Lee
Lia Hiscock

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

Kristin Lee (Gerrit)

unread,

Dec 17, 2025, 9:23:49 PM (2 days ago) Dec 17

to Lia Hiscock, Lu Huang, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org

Attention needed from Lia Hiscock and Lu Huang

Kristin Lee added 2 comments

File chrome/browser/web_applications/install_element_browsertest.cc

Line 226, Patchset 2:

  const GURL install_url =
      https_server()->GetURL("/web_apps/custom_id/install_url.html");

Lu Huang . unresolved

Is this a cross-origin URL? It's using the same server.

Kristin Lee

This isn't a cross-origin URL, but for our case, we only need it to be not the current document. I think mentioning cross-origin is confusing here so I've removed that. Our API browsertests used the titles of crossorigin, but I will omit that here to avoid confusion. Thanks for pointing this out!

Line 269, Patchset 2:

  EXPECT_EQ(install_future.Get<webapps::InstallResultCode>(),
            webapps::InstallResultCode::kSuccessNewInstall);

Lu Huang . resolved

Also check
```
  EXPECT_EQ(provider().registrar_unsafe().GetAppIds().size(), 1u);
```

Kristin Lee

Done

Open in Gerrit

Related details

Attention is currently required from:

Lia Hiscock
Lu Huang

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

Kristin Lee (Gerrit)

unread,

Dec 17, 2025, 10:16:59 PM (2 days ago) Dec 17

to Daniel Murphy, Mike West, Lia Hiscock, Lu Huang, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org

Attention needed from Daniel Murphy and Mike West

Kristin Lee added 1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Kristin Lee . resolved

Hi Dan and Mike, my CL is also ready for review but note that it is chained to be after Lia's CL that you both are currently reviewing, https://chromium-review.googlesource.com/c/chromium/src/+/7241206, because I needed to add test cases to the new install_element_browsertest.cc file she created in her CL. Otherwise, the code logic should be fairly separate so I'm hoping an early review is still possible if you have time. Thanks!

Open in Gerrit

Related details

Attention is currently required from:

Daniel Murphy
Mike West

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

Mike West (Gerrit)

unread,

Dec 18, 2025, 1:47:32 AM (yesterday) Dec 18

to Kristin Lee, Daniel Murphy, Lia Hiscock, Lu Huang, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org

Attention needed from Daniel Murphy and Kristin Lee

Mike West voted and added 1 comment

Votes added by Mike West

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Mike West . resolved

blink LGTM, thanks!

Open in Gerrit

Related details

Attention is currently required from:

Daniel Murphy
Kristin Lee

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

Daniel Murphy (Gerrit)

unread,

Dec 18, 2025, 12:10:57 PM (19 hours ago) Dec 18

to Kristin Lee, Daniel Murphy, Mike West, Lia Hiscock, Lu Huang, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org

Attention needed from Kristin Lee

Daniel Murphy voted and added 1 comment

Votes added by Daniel Murphy

Code-Review

+1

1 comment

File chrome/browser/web_applications/install_element_browsertest.cc

Line 57, Patchset 3 (Latest): // WebInstallFromUrlCommand which also fetches the manifest.

Daniel Murphy . unresolved

can you do
```
// TODO(https://crbug.com/468047211): Change to ui_test_utils::NavigateToURL after WebInstallServiceImpl stops using InstallableManager.
```

And if possible, also add this to the WebInstallServiceImpl part where we talk to the InstallableManager

Open in Gerrit

Related details

Attention is currently required from:

Kristin Lee

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

Kristin Lee (Gerrit)

unread,

Dec 18, 2025, 7:27:38 PM (12 hours ago) Dec 18

to Daniel Murphy, Mike West, Lia Hiscock, Lu Huang, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org

Kristin Lee added 2 comments

File chrome/browser/web_applications/install_element_browsertest.cc

Line 57, Patchset 3: // WebInstallFromUrlCommand which also fetches the manifest.

Daniel Murphy . resolved

can you do
```
// TODO(https://crbug.com/468047211): Change to ui_test_utils::NavigateToURL after WebInstallServiceImpl stops using InstallableManager.
```
And if possible, also add this to the WebInstallServiceImpl part where we talk to the InstallableManager

Kristin Lee

Sounds good, added to both places!

Line 226, Patchset 2:

  const GURL install_url =
      https_server()->GetURL("/web_apps/custom_id/install_url.html");

Lu Huang . resolved

Is this a cross-origin URL? It's using the same server.

Kristin Lee

This isn't a cross-origin URL, but for our case, we only need it to be not the current document. I think mentioning cross-origin is confusing here so I've removed that. Our API browsertests used the titles of crossorigin, but I will omit that here to avoid confusion. Thanks for pointing this out!

Kristin Lee

Done

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

satisfied_requirement

open

diffy

Kristin Lee (Gerrit)

unread,

Dec 18, 2025, 7:28:19 PM (12 hours ago) Dec 18

to Daniel Murphy, Mike West, Lia Hiscock, Lu Huang, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org

Kristin Lee voted Commit-Queue+2

Commit-Queue

+2

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Dec 18, 2025, 8:29:42 PM (11 hours ago) Dec 18

to Kristin Lee, Daniel Murphy, Mike West, Lia Hiscock, Lu Huang, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

3 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/install_element_browsertest.cc
Insertions: 45, Deletions: 33.

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

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

Change information

Commit message:

[<install> Element] Do not show the permission prompt

Adds `InstallFromElement()` to the mojo interface that accepts the same
parameters as `Install()` but is used for installations triggered from
an <install> element. We do not want to show the permission prompt for
triggers from an <install> element so the permission is allowed by
default unless previously denied.

- Feature flag: InstallElement
- Internal design doc: https://docs.google.com/document/d/1rGvLhD4SR8Y9M1wVmqgyesPNkbZGU7HOqlttjEFJ5Vo/edit?tab=t.0
- Test site: https://kbhlee2121.github.io/pwa/web-install/index.html

- Explainer: https://github.com/WICG/install-element

Bug: 454827186, 462477497

Change-Id: I9fdda4b6c6eeb191a313dc935a03cd6dbe188beb

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

Reviewed-by: Mike West <mk...@chromium.org>

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

Commit-Queue: Kristin Lee <krist...@microsoft.com>

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

Files:

M chrome/browser/web_applications/install_element_browsertest.cc
M chrome/browser/web_applications/web_install_service_impl.cc
M chrome/browser/web_applications/web_install_service_impl.h
M third_party/blink/public/mojom/web_install/web_install.mojom
M third_party/blink/renderer/core/html/html_install_element.cc
M third_party/blink/renderer/core/html/html_install_element_test.cc

Change size: M

Delta: 6 files changed, 122 insertions(+), 13 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Mike West, +1 by Daniel Murphy

Open in Gerrit

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

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

[<install> Element] Do not show the permission prompt [chromium/src : main]

Lu Huang (Gerrit)

Lu Huang added 2 comments

Related details

Kristin Lee (Gerrit)

Kristin Lee added 2 comments

Related details

Kristin Lee (Gerrit)

Kristin Lee added 1 comment

Related details

Mike West (Gerrit)

Mike West voted and added 1 comment

Votes added by Mike West

1 comment

Related details

Daniel Murphy (Gerrit)

Daniel Murphy voted and added 1 comment

Votes added by Daniel Murphy

1 comment

Related details

Kristin Lee (Gerrit)

Kristin Lee added 2 comments

Related details

Kristin Lee (Gerrit)

Kristin Lee voted Commit-Queue+2

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information