Block Content Index APIs from fenced frames [chromium/src : main]

Shunya Shishido (Gerrit)

unread,

Jun 22, 2022, 12:48:55 AM6/22/22

to Yoshisato Yanagisawa, Tsuyoshi Horo, blink-...@chromium.org, rayanka...@chromium.org

Attention is currently required from: Tsuyoshi Horo, Yoshisato Yanagisawa.

Shunya Shishido would like Yoshisato Yanagisawa and Tsuyoshi Horo to review this change.

Block Content Index APIs from fenced frames

This CL is a follow-up CL for crrev.com/c/3688774 . This CL will add
error handlings to content index APIs when it's called from fenced
frames.

Bug: 1276419
Change-Id: Ia839d602f919032201ea266bcb38b18026af385d
---
M content/browser/content_index/content_index_service_impl.cc
M third_party/blink/renderer/modules/content_index/content_index.cc
A third_party/blink/web_tests/wpt_internal/fenced_frame/content-index.https.html
A third_party/blink/web_tests/wpt_internal/fenced_frame/resources/content-index-sw.js
4 files changed, 243 insertions(+), 0 deletions(-)

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Shunya Shishido (Gerrit)

unread,

Jun 22, 2022, 12:48:59 AM6/22/22

to blink-...@chromium.org, rayanka...@chromium.org, Yoshisato Yanagisawa, Tsuyoshi Horo, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Tsuyoshi Horo, Yoshisato Yanagisawa.

View Change

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Yoshisato Yanagisawa (Gerrit)

unread,

Jun 22, 2022, 1:22:00 AM6/22/22

to Shunya Shishido, blink-...@chromium.org, rayanka...@chromium.org, Tsuyoshi Horo, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Shunya Shishido, Tsuyoshi Horo.

Patch set 1:Code-Review +1Commit-Queue +1

View Change

1 comment:

Patchset:
- Patch Set #1:
  lgtm
  I guess the event won't be fired but are we also going to deny registering contentdelete event?
  https://developer.mozilla.org/en-US/docs/Web/API/ContentIndexEvent

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Tsuyoshi Horo (Gerrit)

unread,

Jun 22, 2022, 2:05:17 AM6/22/22

to Shunya Shishido, blink-...@chromium.org, rayanka...@chromium.org, Yoshisato Yanagisawa, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Shunya Shishido.

Patch set 1:Code-Review +1

View Change

1 comment:

Patchset:
- Patch Set #1:
  lgtm

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Tsuyoshi Horo (Gerrit)

unread,

Jun 22, 2022, 2:06:14 AM6/22/22

to Shunya Shishido, blink-...@chromium.org, rayanka...@chromium.org, Yoshisato Yanagisawa, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Shunya Shishido.

Patch set 1:-Code-Review

View Change

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Tsuyoshi Horo (Gerrit)

unread,

Jun 22, 2022, 2:07:38 AM6/22/22

to Shunya Shishido, blink-...@chromium.org, rayanka...@chromium.org, Yoshisato Yanagisawa, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Shunya Shishido.

View Change

3 comments:

File third_party/blink/web_tests/wpt_internal/fenced_frame/content-index.https.html:
- Patch Set #1, Line 94: assert_unreached('index.add executed without error; want error');
  We don't need try catch?
- Patch Set #1, Line 125: assert_unreached('index.delete executed without error; want error');
  We don't need try catch?
- Patch Set #1, Line 156: assert_unreached('index.getAll executed without error; want error');
  We don't need try catch?

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Shunya Shishido (Gerrit)

unread,

Jun 22, 2022, 3:06:14 AM6/22/22

to blink-...@chromium.org, rayanka...@chromium.org, Tsuyoshi Horo, Yoshisato Yanagisawa, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Tsuyoshi Horo, Yoshisato Yanagisawa.

View Change

4 comments:

Patchset:
- Patch Set #2:
  Thank you! Sorry for pointing this again and again.
File third_party/blink/web_tests/wpt_internal/fenced_frame/content-index.https.html:

- Patch Set #1, Line 94: assert_unreached('index.add executed without error; want error');
  We don't need try catch?

- Done

- Patch Set #1, Line 125: assert_unreached('index.delete executed without error; want error');
  We don't need try catch?

- Done

- Patch Set #1, Line 156: assert_unreached('index.getAll executed without error; want error');
  We don't need try catch?

- Done

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Shunya Shishido (Gerrit)

unread,

Jun 22, 2022, 4:26:15 AM6/22/22

to Rayan Kanso, blink-...@chromium.org, rayanka...@chromium.org, Tsuyoshi Horo, Yoshisato Yanagisawa

Attention is currently required from: Rayan Kanso, Tsuyoshi Horo, Yoshisato Yanagisawa.

Shunya Shishido would like Rayan Kanso to review this change.

View Change

Block Content Index APIs from fenced frames

This CL is a follow-up CL for crrev.com/c/3688774 . This CL will add
error handlings to content index APIs when it's called from fenced
frames.

Bug: 1276419
Change-Id: Ia839d602f919032201ea266bcb38b18026af385d
---
M content/browser/content_index/content_index_service_impl.cc
M third_party/blink/renderer/modules/content_index/content_index.cc
A third_party/blink/web_tests/wpt_internal/fenced_frame/content-index.https.html
A third_party/blink/web_tests/wpt_internal/fenced_frame/resources/content-index-sw.js

4 files changed, 233 insertions(+), 0 deletions(-)

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Shunya Shishido (Gerrit)

unread,

Jun 22, 2022, 4:26:20 AM6/22/22

to blink-...@chromium.org, rayanka...@chromium.org, Rayan Kanso, Tsuyoshi Horo, Yoshisato Yanagisawa, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Rayan Kanso, Tsuyoshi Horo, Yoshisato Yanagisawa.

View Change

1 comment:

Patchset:
- Patch Set #2:
  rayankans: Can you give a review for content_index_service_impl.cc and content_index.cc? Thank you!

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Rayan Kanso (Gerrit)

unread,

Jun 22, 2022, 5:13:12 AM6/22/22

to Shunya Shishido, blink-...@chromium.org, rayanka...@chromium.org, Tsuyoshi Horo, Yoshisato Yanagisawa, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Shunya Shishido, Tsuyoshi Horo, Yoshisato Yanagisawa.

View Change

1 comment:

Patchset:
- Patch Set #2:
  The ContentIndex spec does not restrict the API to top-level frames. Although there have been some discussions around enforcing this restriction: https://github.com/WICG/content-index/issues/18
  I don't think this change is necessary, unless there are some other reasons we want to apply this restriction?

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Shunya Shishido (Gerrit)

unread,

Jun 23, 2022, 1:26:15 AM6/23/22

to Kouhei Ueno, blink-...@chromium.org, rayanka...@chromium.org, Shivani Sharma, Rayan Kanso, Tsuyoshi Horo, Yoshisato Yanagisawa

Attention is currently required from: Kouhei Ueno, Rayan Kanso, Tsuyoshi Horo, Yoshisato Yanagisawa.

Shunya Shishido would like Kouhei Ueno to review this change.

View Change

Block Content Index APIs from fenced frames

This CL is a follow-up CL for crrev.com/c/3688774 . This CL will add
error handlings to content index APIs when it's called from fenced
frames.

Bug: 1276419
Change-Id: Ia839d602f919032201ea266bcb38b18026af385d
---
M content/browser/content_index/content_index_service_impl.cc
M third_party/blink/renderer/modules/content_index/content_index.cc
A third_party/blink/web_tests/wpt_internal/fenced_frame/content-index.https.html
A third_party/blink/web_tests/wpt_internal/fenced_frame/resources/content-index-sw.js
4 files changed, 233 insertions(+), 0 deletions(-)

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Shunya Shishido (Gerrit)

unread,

Jun 23, 2022, 1:26:22 AM6/23/22

to blink-...@chromium.org, rayanka...@chromium.org, Kouhei Ueno, Shivani Sharma, Rayan Kanso, Tsuyoshi Horo, Yoshisato Yanagisawa, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Kouhei Ueno, Rayan Kanso, Tsuyoshi Horo, Yoshisato Yanagisawa.

View Change

1 comment:

Patchset:
- Patch Set #2:
  cc-ed shivanisha as a TL for fenced frames.
  Thank you for the comment! Here is the design doc, more context is there.
  https://docs.google.com/document/d/1L17q_9t_fyakEhfxkMAiCl2hfw6CsMd_emkilciHMck/edit#
  The ContentIndex spec does not restrict the API to top-level frame.
  That's a fair point. But still I think it's better to land this CL. Because 1) as the design principle, fenced frame prevents communication between the embedder and the frame. So, if a fenced frame could know what resources are saved by users outside of the frame via `index.getAll`, that would be a problem. 2) even without this CL, currently the API (specifically `index.add`) throws an error when it's called from fenced frames. The storage for fenced frames is partitioned and it results in the storage error. I think this error is not intended anyway, so this CL will add explicit error handling and tests to make the current behavior clear. If we should support the API in FF, we need some extra work to fix the current storage error, but do we really have such a use case? 3) It's safer to block the API unless we have some specific use cases to minimize security risks.

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Tsuyoshi Horo (Gerrit)

unread,

Jun 23, 2022, 4:53:20 PM6/23/22

to Shunya Shishido, blink-...@chromium.org, rayanka...@chromium.org, Kouhei Ueno, Shivani Sharma, Rayan Kanso, Yoshisato Yanagisawa, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Kouhei Ueno, Rayan Kanso, Shunya Shishido, Yoshisato Yanagisawa.

Patch set 2:Code-Review +1

View Change

1 comment:

Patchset:
- Patch Set #2:
  lgtm

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Kouhei Ueno (Gerrit)

unread,

Jun 23, 2022, 10:24:31 PM6/23/22

to Shunya Shishido, blink-...@chromium.org, rayanka...@chromium.org, Shivani Sharma, Rayan Kanso, Tsuyoshi Horo, Yoshisato Yanagisawa, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Rayan Kanso, Shunya Shishido, Yoshisato Yanagisawa.

Patch set 2:Code-Review +1

View Change

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Shunya Shishido (Gerrit)

unread,

Jun 27, 2022, 2:15:43 AM6/27/22

to blink-...@chromium.org, rayanka...@chromium.org, Kouhei Ueno, Shivani Sharma, Rayan Kanso, Tsuyoshi Horo, Yoshisato Yanagisawa, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Kouhei Ueno, Rayan Kanso, Shivani Sharma.

View Change

1 comment:

Patchset:
- Patch Set #2:
  cc-ed shivanisha as a TL for fenced frames. […]
  rayankans: Does that address your concern?

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Rayan Kanso (Gerrit)

unread,

Jun 27, 2022, 8:26:28 AM6/27/22

to Shunya Shishido, blink-...@chromium.org, rayanka...@chromium.org, Kouhei Ueno, Shivani Sharma, Tsuyoshi Horo, Yoshisato Yanagisawa, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Kouhei Ueno, Shivani Sharma, Shunya Shishido.

Patch set 2:Code-Review +1

View Change

1 comment:

Patchset:
- Patch Set #2:
  Sorry for the delay, I was OOO for the last few days.

- even without this CL, currently the API (specifically index.add ) throws an error when it's called from fenced frames.

- In that case this should be fine, otherwise this change would have required a spec change + I2S.

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Shunya Shishido (Gerrit)

unread,

Jun 27, 2022, 7:40:10 PM6/27/22

to blink-...@chromium.org, rayanka...@chromium.org, Rayan Kanso, Kouhei Ueno, Shivani Sharma, Tsuyoshi Horo, Yoshisato Yanagisawa, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Kouhei Ueno, Shivani Sharma, Shunya Shishido.

Patch set 2:Commit-Queue +2

View Change

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Chromium LUCI CQ (Gerrit)

unread,

Jun 27, 2022, 8:42:00 PM6/27/22

to Shunya Shishido, blink-...@chromium.org, rayanka...@chromium.org, Rayan Kanso, Kouhei Ueno, Shivani Sharma, Tsuyoshi Horo, Yoshisato Yanagisawa, chromium...@chromium.org

Chromium LUCI CQ submitted this change.

View Change

Approvals:
  Kouhei Ueno: Looks good to me
  Tsuyoshi Horo: Looks good to me
  Yoshisato Yanagisawa: Looks good to me
  Rayan Kanso: Looks good to me
  Shunya Shishido: Commit

Block Content Index APIs from fenced frames

This CL is a follow-up CL for crrev.com/c/3688774 . This CL will add

error handlings to content index APIs when it's called from fenced
frames.

Bug: 1276419
Change-Id: Ia839d602f919032201ea266bcb38b18026af385d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715416
Reviewed-by: Kouhei Ueno <kou...@chromium.org>
Commit-Queue: Shunya Shishido <sisid...@chromium.org>
Reviewed-by: Yoshisato Yanagisawa <yyana...@chromium.org>
Reviewed-by: Rayan Kanso <raya...@chromium.org>
Reviewed-by: Tsuyoshi Horo <ho...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1018484}


---
M content/browser/content_index/content_index_service_impl.cc
M third_party/blink/renderer/modules/content_index/content_index.cc
A third_party/blink/web_tests/wpt_internal/fenced_frame/content-index.https.html
A third_party/blink/web_tests/wpt_internal/fenced_frame/resources/content-index-sw.js

4 files changed, 240 insertions(+), 0 deletions(-)

To view, visit change 3715416. To unsubscribe, or for help writing mail filters, visit settings.

Reply all

Reply to author

Forward