Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(131)

Issue 2831943002: bindings: Port bindings/core/v8 away from ToImplArray APIs. (Closed)

Created:
3 years, 8 months ago by Raphael Kubo da Costa (rakuco)
Modified:
3 years, 8 months ago
Reviewers:
haraken, bashi, Yuki
CC:
chromium-reviews, shimazu+worker_chromium.org, kinuko+worker_chromium.org, blink-reviews-bindings_chromium.org, jbroman+watch_chromium.org, blink-reviews, horo+watch_chromium.org, falken+watch_chromium.org, blink-worker-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

bindings: Port bindings/core/v8 away from ToImplArray APIs. Convert callers of ToImplArray, ToImplSequence and ToMemberNativeArray to NativeValueTraits<IDLSequence<T>>::NativeValue(). This is another step towards removing most of the functions above and centralizing conversions from JS types to WebIDL sequences in a single location. BUG=690428 R=bashi@chromium.org,haraken@chromium.org,yukishiino@chromium.org Review-Url: https://codereview.chromium.org/2831943002 Cr-Commit-Position: refs/heads/master@{#466317} Committed: https://chromium.googlesource.com/chromium/src/+/2c772b93310fcf295f3de133e67095c7fe49a910

Patch Set 1 #

Patch Set 2 : Remove expectation for test that is now passing #

Total comments: 2

Patch Set 3 : Check for exception in SerializedScriptValue #

Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -50 lines) Patch
D third_party/WebKit/LayoutTests/external/wpt/webmessaging/with-ports/027-expected.txt View 1 1 chunk +0 lines, -5 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/dom/Window/window-postmessage-args.html View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/dom/Window/window-postmessage-args-expected.txt View 2 chunks +7 lines, -8 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/events/init-message-event.html View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/events/init-message-event-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/events/message-port-multi-expected.txt View 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/events/resources/message-port-multi.js View 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/workers/worker-context-multi-port-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/workers/worker-multi-port-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/workers/worker-onerror-09.html View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/workers/worker-onerror-09-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/virtual/sharedarraybuffer/fast/workers/worker-onerror-09-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/DictionaryHelperForCore.cpp View 2 chunks +5 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinitionBuilder.cpp View 1 2 3 chunks +4 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/ScriptPromiseTest.cpp View 2 chunks +4 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp View 1 2 3 chunks +9 lines, -16 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/custom/V8MessageEventCustom.cpp View 2 chunks +4 lines, -3 lines 0 comments Download

Messages

Total messages: 18 (10 generated)
Raphael Kubo da Costa (rakuco)
PTAL
3 years, 8 months ago (2017-04-20 15:58:08 UTC) #1
haraken
LGTM
3 years, 8 months ago (2017-04-20 17:26:33 UTC) #4
Yuki
LGTM with a comment. https://codereview.chromium.org/2831943002/diff/20001/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp File third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp (right): https://codereview.chromium.org/2831943002/diff/20001/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp#newcode380 third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp:380: isolate, value, exception_state); Should return ...
3 years, 8 months ago (2017-04-21 07:02:54 UTC) #7
Raphael Kubo da Costa (rakuco)
https://codereview.chromium.org/2831943002/diff/20001/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp File third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp (right): https://codereview.chromium.org/2831943002/diff/20001/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp#newcode380 third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp:380: isolate, value, exception_state); On 2017/04/21 07:02:54, Yuki wrote: > ...
3 years, 8 months ago (2017-04-21 10:30:21 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2831943002/40001
3 years, 8 months ago (2017-04-21 10:30:43 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/436310)
3 years, 8 months ago (2017-04-21 11:44:46 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2831943002/40001
3 years, 8 months ago (2017-04-21 11:57:52 UTC) #15
commit-bot: I haz the power
3 years, 8 months ago (2017-04-21 12:33:01 UTC) #18
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/2c772b93310fcf295f3de133e670...

Powered by Google App Engine
This is Rietveld 408576698