Fix incorrect interception when O and Receiver are different [chromium/src : main]

[Jinho Bang (Gerrit)]

Attention needed from YeongHan Kim

Jinho Bang added 1 comment

File third_party/blink/renderer/bindings/scripts/bind_gen/interface.py

Line 3731, Patchset 1 (Latest): TextNode(

Jinho Bang . unresolved

We could consider using `Cxx*IfNode` families.

Open in Gerrit

Related details

Attention is currently required from:

• YeongHan Kim

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

Gerrit-Change-Number: 7096938

Gerrit-PatchSet: 1

Gerrit-Owner: YeongHan Kim <soosu...@gmail.com>

Gerrit-Reviewer: YeongHan Kim <soosu...@gmail.com>

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

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

Gerrit-CC: Jinho Bang <zi...@chromium.org>

Gerrit-CC: Kentaro Hara <har...@chromium.org>

Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>

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

Gerrit-Attention: YeongHan Kim <soosu...@gmail.com>

Gerrit-Comment-Date: Thu, 30 Oct 2025 02:33:55 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Jinho Bang (Gerrit)]

Attention needed from YeongHan Kim

Jinho Bang added 1 comment

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Jinho Bang . resolved

According to the spec, we probably should consider indexed property.

Gerrit-Comment-Date: Thu, 30 Oct 2025 02:36:12 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Yuki Shiino (Gerrit)]

Attention needed from YeongHan Kim

Yuki Shiino added 2 comments

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Yuki Shiino . resolved

Thank you for fixing this.

File third_party/blink/renderer/bindings/scripts/bind_gen/interface.py

Line 3731, Patchset 1 (Latest): TextNode(

Yuki Shiino . unresolved

nit: It'd be nice to have a spec snippet as a code comment.

Gerrit-CC: Yuki Shiino <yukis...@chromium.org>

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

Gerrit-Attention: YeongHan Kim <soosu...@gmail.com>

Gerrit-Comment-Date: Thu, 30 Oct 2025 04:59:14 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[YeongHan Kim (Gerrit)]

Attention needed from Jinho Bang and Yuki Shiino

YeongHan Kim added 3 comments

Patchset-level comments

File-level comment, Patchset 2 (Latest):

YeongHan Kim . resolved

Thanks for the review!

File third_party/blink/renderer/bindings/scripts/bind_gen/interface.py

Line 3731, Patchset 1: TextNode(

Yuki Shiino . resolved

nit: It'd be nice to have a spec snippet as a code comment.

YeongHan Kim

Done!

Line 3731, Patchset 1: TextNode(

Jinho Bang . resolved

We could consider using `Cxx*IfNode` families.

YeongHan Kim

Changed to `CxxLikelyIfNode`.

Open in Gerrit

Related details

Attention is currently required from:

• Jinho Bang
• Yuki Shiino

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

Gerrit-Change-Number: 7096938

Gerrit-PatchSet: 2

Gerrit-Owner: YeongHan Kim <soosu...@gmail.com>

Gerrit-Reviewer: YeongHan Kim <soosu...@gmail.com>

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

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

Gerrit-CC: Jinho Bang <zi...@chromium.org>

Gerrit-CC: Kentaro Hara <har...@chromium.org>

Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>

Gerrit-CC: Yuki Shiino <yukis...@chromium.org>

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

Gerrit-Attention: Jinho Bang <zi...@chromium.org>

Gerrit-Attention: Yuki Shiino <yukis...@chromium.org>

Gerrit-Comment-Date: Thu, 30 Oct 2025 16:47:02 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Jinho Bang <zi...@chromium.org>

Comment-In-Reply-To: Yuki Shiino <yukis...@chromium.org>

[YeongHan Kim (Gerrit)]

Attention needed from Jinho Bang and Yuki Shiino

YeongHan Kim added 2 comments

Patchset-level comments

File-level comment, Patchset 1:

Jinho Bang . resolved

According to the spec, we probably should consider indexed property.

YeongHan Kim

Since the indexed property has no QueryCallback, it fallback immediately.
So it should be fine.

File-level comment, Patchset 2 (Latest):

YeongHan Kim . resolved

I’m leaving an additional comment for the one I missed. Thanks!

Gerrit-Comment-Date: Fri, 31 Oct 2025 04:38:22 +0000

[Yuki Shiino (Gerrit)]

Attention needed from Igor Sheludko, Jinho Bang and YeongHan Kim

Yuki Shiino added 1 comment

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Yuki Shiino . resolved

+R=ishell@, would you take a look at this patch?
Could this be related to your work to deprecate `v8::PropertyCallbackInfo<T>::This`?

Open in Gerrit

Related details

Attention is currently required from:

• Igor Sheludko
• Jinho Bang
• YeongHan Kim

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

Gerrit-Change-Number: 7096938

Gerrit-PatchSet: 2

Gerrit-Owner: YeongHan Kim <soosu...@gmail.com>

Gerrit-Reviewer: Igor Sheludko <ish...@chromium.org>

Gerrit-Reviewer: YeongHan Kim <soosu...@gmail.com>

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

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

Gerrit-CC: Jinho Bang <zi...@chromium.org>

Gerrit-CC: Kentaro Hara <har...@chromium.org>

Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>

Gerrit-CC: Yuki Shiino <yukis...@chromium.org>

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

Gerrit-Attention: Jinho Bang <zi...@chromium.org>

Gerrit-Attention: Igor Sheludko <ish...@chromium.org>

Gerrit-Attention: YeongHan Kim <soosu...@gmail.com>

Gerrit-Comment-Date: Fri, 31 Oct 2025 06:01:35 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[YeongHan Kim (Gerrit)]

Attention needed from Igor Sheludko and Jinho Bang

YeongHan Kim added 2 comments

Patchset-level comments

File-level comment, Patchset 3 (Latest):

YeongHan Kim . resolved

I found a more appropriate approach than the previous change, so I updated it.
Thanks!

File third_party/blink/renderer/bindings/scripts/bind_gen/interface.py

Line 3731, Patchset 1: TextNode(

Jinho Bang . resolved

We could consider using `Cxx*IfNode` families.

YeongHan Kim

Changed to `CxxLikelyIfNode`.

YeongHan Kim

I changed it to `CxxUnlikelyIfNode`, as it seems more appropriate.

Open in Gerrit

Related details

Attention is currently required from:

• Igor Sheludko
• Jinho Bang

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

Gerrit-Change-Number: 7096938

Gerrit-PatchSet: 3

Gerrit-Owner: YeongHan Kim <soosu...@gmail.com>

Gerrit-Reviewer: Igor Sheludko <ish...@chromium.org>

Gerrit-Reviewer: YeongHan Kim <soosu...@gmail.com>

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

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

Gerrit-CC: Jinho Bang <zi...@chromium.org>

Gerrit-CC: Kentaro Hara <har...@chromium.org>

Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>

Gerrit-CC: Yuki Shiino <yukis...@chromium.org>

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

Gerrit-Attention: Jinho Bang <zi...@chromium.org>

Gerrit-Attention: Igor Sheludko <ish...@chromium.org>

Gerrit-Comment-Date: Fri, 31 Oct 2025 06:10:34 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Jinho Bang <zi...@chromium.org>

Comment-In-Reply-To: YeongHan Kim <soosu...@gmail.com>

[Igor Sheludko (Gerrit)]

Attention needed from Jinho Bang and YeongHan Kim

Igor Sheludko added 1 comment

File third_party/blink/renderer/bindings/scripts/bind_gen/interface.py

Line 3734, Patchset 4 (Latest):// 3.9.6. [[Set]]
// https://webidl.spec.whatwg.org/#legacy-platform-object-set
// If O and Receiver are the different object, then:
// step 2. Let ownDesc be ? LegacyPlatformObjectGetOwnProperty(O, P, true).
// step 3. Perform ? OrdinarySetWithOwnDescriptor(O, P, V, Receiver, ownDesc).
// Do not intercept. Fallback to OrdinarySetWithOwnDescriptor.

Igor Sheludko . unresolved

Thanks for pointing out to the issue. This fix is not correct and the issue should be fixed on V8 side.

The query callback is supposed to say whether the interceptor has the property and what are the property's attributes. This change however makes it lie when the storing happens through prototype (as in the failing test). This could be observable for read-only properties.

I'll take a look at this issue.

Open in Gerrit

Related details

Attention is currently required from:

• Jinho Bang
• YeongHan Kim

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

Gerrit-Change-Number: 7096938

Gerrit-PatchSet: 4

Gerrit-Owner: YeongHan Kim <soosu...@gmail.com>

Gerrit-Reviewer: Igor Sheludko <ish...@chromium.org>

Gerrit-Reviewer: YeongHan Kim <soosu...@gmail.com>

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

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

Gerrit-CC: Jinho Bang <zi...@chromium.org>

Gerrit-CC: Kentaro Hara <har...@chromium.org>

Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>

Gerrit-CC: Yuki Shiino <yukis...@chromium.org>

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

Gerrit-Attention: Jinho Bang <zi...@chromium.org>

Gerrit-Attention: YeongHan Kim <soosu...@gmail.com>

Gerrit-Comment-Date: Fri, 31 Oct 2025 11:22:37 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Igor Sheludko (Gerrit)]

Attention needed from Jinho Bang and YeongHan Kim

Igor Sheludko added 1 comment

File third_party/blink/renderer/bindings/scripts/bind_gen/interface.py

Line 3734, Patchset 4 (Latest):// 3.9.6. [[Set]]
// https://webidl.spec.whatwg.org/#legacy-platform-object-set
// If O and Receiver are the different object, then:
// step 2. Let ownDesc be ? LegacyPlatformObjectGetOwnProperty(O, P, true).
// step 3. Perform ? OrdinarySetWithOwnDescriptor(O, P, V, Receiver, ownDesc).
// Do not intercept. Fallback to OrdinarySetWithOwnDescriptor.

Igor Sheludko . unresolved

Thanks for pointing out to the issue. This fix is not correct and the issue should be fixed on V8 side.

The query callback is supposed to say whether the interceptor has the property and what are the property's attributes. This change however makes it lie when the storing happens through prototype (as in the failing test). This could be observable for read-only properties.

I'll take a look at this issue.

Igor Sheludko

See discussion here: https://crbug.com/40677381#comment19.

Gerrit-Comment-Date: Fri, 31 Oct 2025 23:56:52 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Igor Sheludko <ish...@chromium.org>