Andrey Kosyakov (Gerrit)

unread,

Jul 10, 2024, 2:19:10 PM7/10/24

to Lei Zhang, findit...@appspot.gserviceaccount.com, Daniel Cheng, Robert Flack, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, AyeAye, nwoked...@chromium.org, xinghui...@chromium.org, andysjl...@chromium.org, druber...@chromium.org, blundell+...@chromium.org, blink-...@chromium.org, dtapuska+ch...@chromium.org, mac-r...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, zackha...@chromium.org, dtapuska+...@chromium.org, pdf-r...@chromium.org, vakh+safe_br...@chromium.org

Attention needed from Daniel Cheng, Lei Zhang and Robert Flack

Andrey Kosyakov voted and added 1 comment

Votes added by Andrey Kosyakov

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Andrey Kosyakov . resolved

Stamping right away, since crashes don't look related to me either.

Open in Gerrit

Related details

Attention is currently required from:

Daniel Cheng
Lei Zhang
Robert Flack

Submit Requirements:

Code-Coverage
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Daniel Cheng (Gerrit)

unread,

Jul 10, 2024, 5:02:08 PM7/10/24

to Lei Zhang, Andrey Kosyakov, findit...@appspot.gserviceaccount.com, Daniel Cheng, Robert Flack, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, AyeAye, nwoked...@chromium.org, xinghui...@chromium.org, andysjl...@chromium.org, druber...@chromium.org, blundell+...@chromium.org, blink-...@chromium.org, dtapuska+ch...@chromium.org, mac-r...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, zackha...@chromium.org, dtapuska+...@chromium.org, pdf-r...@chromium.org, vakh+safe_br...@chromium.org

Attention needed from Lei Zhang and Robert Flack

Daniel Cheng added 1 comment

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Daniel Cheng . resolved

If we're relanding this as-is, can the CL description note the reason why it's safe to do so?

Open in Gerrit

Related details

Attention is currently required from:

Lei Zhang
Robert Flack

Submit Requirements:

Code-Coverage
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Lei Zhang (Gerrit)

unread,

Jul 10, 2024, 5:05:39 PM7/10/24

to Lei Zhang, Andrey Kosyakov, findit...@appspot.gserviceaccount.com, Daniel Cheng, Robert Flack, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, AyeAye, nwoked...@chromium.org, xinghui...@chromium.org, andysjl...@chromium.org, druber...@chromium.org, blundell+...@chromium.org, blink-...@chromium.org, dtapuska+ch...@chromium.org, mac-r...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, zackha...@chromium.org, dtapuska+...@chromium.org, pdf-r...@chromium.org, vakh+safe_br...@chromium.org

Attention needed from Daniel Cheng and Robert Flack

Lei Zhang added 1 comment

Patchset-level comments

File-level comment, Patchset 1:

Daniel Cheng . resolved

If we're relanding this as-is, can the CL description note the reason why it's safe to do so?

Lei Zhang

Done.

Open in Gerrit

Related details

Attention is currently required from:

Daniel Cheng
Robert Flack

Submit Requirements:

Code-Coverage
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Daniel Cheng (Gerrit)

unread,

Jul 10, 2024, 5:12:10 PM7/10/24

to Lei Zhang, Daniel Cheng, Andrey Kosyakov, findit...@appspot.gserviceaccount.com, Robert Flack, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, AyeAye, nwoked...@chromium.org, xinghui...@chromium.org, andysjl...@chromium.org, druber...@chromium.org, blundell+...@chromium.org, blink-...@chromium.org, dtapuska+ch...@chromium.org, mac-r...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, zackha...@chromium.org, dtapuska+...@chromium.org, pdf-r...@chromium.org, vakh+safe_br...@chromium.org

Attention needed from Lei Zhang and Robert Flack

Daniel Cheng voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Lei Zhang
Robert Flack

Submit Requirements:

Code-Coverage
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Daniel Cheng (Gerrit)

unread,

Jul 10, 2024, 5:13:20 PM7/10/24

to Lei Zhang, Daniel Cheng, Andrey Kosyakov, findit...@appspot.gserviceaccount.com, Robert Flack, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, AyeAye, nwoked...@chromium.org, xinghui...@chromium.org, andysjl...@chromium.org, druber...@chromium.org, blundell+...@chromium.org, blink-...@chromium.org, dtapuska+ch...@chromium.org, mac-r...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, zackha...@chromium.org, dtapuska+...@chromium.org, pdf-r...@chromium.org, vakh+safe_br...@chromium.org

Attention needed from Lei Zhang and Robert Flack

Daniel Cheng voted and added 1 comment

Votes added by Daniel Cheng

Owners-Override

+1

1 comment

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Daniel Cheng . resolved

Since the non-Blink changes are a straightforward consequence of the type change in the Blink public API, using OO.

satisfied_requirement

open

diffy

findit-for-me@appspot.gserviceaccount.com (Gerrit)

unread,

Jul 10, 2024, 5:58:32 PM7/10/24

to Lei Zhang, Daniel Cheng, Andrey Kosyakov, Robert Flack, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, AyeAye, nwoked...@chromium.org, xinghui...@chromium.org, andysjl...@chromium.org, druber...@chromium.org, blundell+...@chromium.org, blink-...@chromium.org, dtapuska+ch...@chromium.org, mac-r...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, zackha...@chromium.org, dtapuska+...@chromium.org, pdf-r...@chromium.org, vakh+safe_br...@chromium.org

Attention needed from Lei Zhang and Robert Flack

findit...@appspot.gserviceaccount.com voted Code-Coverage-1

This change will be blocked from submission as there are files which do not meet the coverage criteria. Following files have incremental coverage(all tests) < 70%.

//third_party/blink/renderer/core/editing/editor_key_bindings.cc

Please add tests for uncovered lines, or add Low-Coverage-Reason:<reason> in the change description to bypass. See https://bit.ly/46jhjS9 to understand when it is okay to bypass. If you think coverage is underreported, file a bug at https://bit.ly/3ENM7Pe

Code-Coverage

-1

Open in Gerrit

Related details

Attention is currently required from:

Lei Zhang
Robert Flack

Submit Requirements:

Code-Coverage
Code-Review

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

Gerrit

blocking_requirement

satisfied_requirement

open

diffy

Lei Zhang (Gerrit)

unread,

Jul 10, 2024, 7:31:55 PM7/10/24

to Lei Zhang, findit...@appspot.gserviceaccount.com, Daniel Cheng, Andrey Kosyakov, Robert Flack, Chromium LUCI CQ, chromium...@chromium.org, devtools...@chromium.org, AyeAye, nwoked...@chromium.org, xinghui...@chromium.org, andysjl...@chromium.org, druber...@chromium.org, blundell+...@chromium.org, blink-...@chromium.org, dtapuska+ch...@chromium.org, mac-r...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, zackha...@chromium.org, dtapuska+...@chromium.org, pdf-r...@chromium.org, vakh+safe_br...@chromium.org

Attention needed from Robert Flack

Lei Zhang voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

Robert Flack

Submit Requirements:

Code-Coverage
Code-Review

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

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Jul 10, 2024, 7:41:37 PM7/10/24

to Lei Zhang, findit...@appspot.gserviceaccount.com, Daniel Cheng, Andrey Kosyakov, Robert Flack, chromium...@chromium.org, devtools...@chromium.org, AyeAye, nwoked...@chromium.org, xinghui...@chromium.org, andysjl...@chromium.org, druber...@chromium.org, blundell+...@chromium.org, blink-...@chromium.org, dtapuska+ch...@chromium.org, mac-r...@chromium.org, blink-re...@chromium.org, kinuko...@chromium.org, zackha...@chromium.org, dtapuska+...@chromium.org, pdf-r...@chromium.org, vakh+safe_br...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.

Change information

Commit message:

Reland "Change blink::WebKeyboardEvent to use std::array in is members"

This is a reland of commit ddb1b5f2be65873e58f754f37d8d417c1343b356

This CL is relanding as-is, because it did not cause the red bots on
the waterfall.

Original change's description:
> Change blink::WebKeyboardEvent to use std::array in is members
>
> Change the `text` and `unmodified_text` members from C-arrays to
> std::array, per guidance on:
> https://chromium.googlesource.com/chromium/src/+/main/docs/unsafe_buffers.md#use-of-std_array
>
> Then update callers that no longer compile to either call data() or take
> a span. Also fix a few lint errors.
>
> Low-Coverage-Reason: LARGE_SCALE_REFACTOR switching to std::array
> Change-Id: Ic29e49ef8ae20702c6df561406e8fe34a66a7af7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5685241
> Reviewed-by: Andrey Kosyakov <ca...@chromium.org>
> Reviewed-by: Robert Flack <fla...@chromium.org>
> Commit-Queue: Lei Zhang <the...@chromium.org>
> Reviewed-by: Daniel Cheng <dch...@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1324966}

Low-Coverage-Reason: LARGE_SCALE_REFACTOR switching to std::array

Change-Id: Ic2db6c274b0884f6ca914933a9093767fa0b481c

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

Commit-Queue: Lei Zhang <the...@chromium.org>

Reviewed-by: Andrey Kosyakov <ca...@chromium.org>

Reviewed-by: Daniel Cheng <dch...@chromium.org>

Owners-Override: Daniel Cheng <dch...@chromium.org>

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

Files:

M chrome/browser/safe_browsing/chrome_password_reuse_detection_manager_client.cc
M components/input/native_web_keyboard_event_mac.mm
M content/browser/devtools/protocol/input_handler.cc
M content/renderer/pepper/event_conversion.cc
M pdf/pdf_view_web_plugin_unittest.cc
M third_party/blink/common/input/web_coalesced_input_event_mojom_traits.cc
M third_party/blink/public/common/input/web_keyboard_event.h
M third_party/blink/renderer/core/editing/editor_key_bindings.cc
M third_party/blink/renderer/core/input/keyboard_event_manager.cc
M ui/events/blink/web_input_event_traits.cc

Change size: M

Delta: 10 files changed, 52 insertions(+), 45 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Coverage: -1 by findit...@appspot.gserviceaccount.com
Code-Review: +1 by Andrey Kosyakov, +1 by Daniel Cheng

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

Reland "Change blink::WebKeyboardEvent to use std::array in is members" [chromium/src : main]

Andrey Kosyakov (Gerrit)

Andrey Kosyakov voted and added 1 comment

Votes added by Andrey Kosyakov

1 comment

Related details

Daniel Cheng (Gerrit)

Daniel Cheng added 1 comment

Related details

Lei Zhang (Gerrit)

Lei Zhang added 1 comment

Related details

Daniel Cheng (Gerrit)

Daniel Cheng voted Code-Review+1

Related details

Daniel Cheng (Gerrit)

Daniel Cheng voted and added 1 comment

Votes added by Daniel Cheng

1 comment

findit-for-me@appspot.gserviceaccount.com (Gerrit)

findit...@appspot.gserviceaccount.com voted Code-Coverage-1

Related details

Lei Zhang (Gerrit)

Lei Zhang voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information