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

Issue 2472753003: Move fromJSONString to V8Binding (Closed)

Created:
4 years, 1 month ago by rwlbuis
Modified:
4 years, 1 month ago
Reviewers:
haraken
CC:
chromium-reviews, awdf+watch_chromium.org, rouslan+payments_chromium.org, Peter Beverloo, johnme+watch_chromium.org, haraken, blink-reviews-bindings_chromium.org, blink-reviews, harkness+watch_chromium.org, sebsg+paymentswatch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Move fromJSONString to V8Binding Move fromJSONString to V8Binding and start using it in a few more places. Also remove JSONValuesForV8.* since that was the only function it contained. Committed: https://crrev.com/36757cf38870dd6fa9f4e4110edf077a7a4ac8d8 Cr-Commit-Position: refs/heads/master@{#429762}

Patch Set 1 #

Patch Set 2 : V2 #

Total comments: 2

Patch Set 3 : Use proper ExceptionState in V8XMLHttpRequestCustom #

Total comments: 1

Patch Set 4 : Patch for landing #

Patch Set 5 : Really swallow exception #

Messages

Total messages: 24 (12 generated)
rwlbuis
PTAL. I tried to use it in a few more places like fetch/Body.cpp but I ...
4 years, 1 month ago (2016-11-03 13:53:52 UTC) #4
haraken
https://codereview.chromium.org/2472753003/diff/20001/third_party/WebKit/Source/bindings/core/v8/custom/V8XMLHttpRequestCustom.cpp File third_party/WebKit/Source/bindings/core/v8/custom/V8XMLHttpRequestCustom.cpp (right): https://codereview.chromium.org/2472753003/diff/20001/third_party/WebKit/Source/bindings/core/v8/custom/V8XMLHttpRequestCustom.cpp#newcode87 third_party/WebKit/Source/bindings/core/v8/custom/V8XMLHttpRequestCustom.cpp:87: TrackExceptionState exceptionState; Use a normal ExceptionState instead of TrackExceptionState. ...
4 years, 1 month ago (2016-11-03 14:01:36 UTC) #5
rwlbuis
https://codereview.chromium.org/2472753003/diff/20001/third_party/WebKit/Source/bindings/core/v8/custom/V8XMLHttpRequestCustom.cpp File third_party/WebKit/Source/bindings/core/v8/custom/V8XMLHttpRequestCustom.cpp (right): https://codereview.chromium.org/2472753003/diff/20001/third_party/WebKit/Source/bindings/core/v8/custom/V8XMLHttpRequestCustom.cpp#newcode87 third_party/WebKit/Source/bindings/core/v8/custom/V8XMLHttpRequestCustom.cpp:87: TrackExceptionState exceptionState; On 2016/11/03 14:01:35, haraken wrote: > > ...
4 years, 1 month ago (2016-11-03 14:12:59 UTC) #6
haraken
LGTM https://codereview.chromium.org/2472753003/diff/40001/third_party/WebKit/Source/bindings/core/v8/custom/V8XMLHttpRequestCustom.cpp File third_party/WebKit/Source/bindings/core/v8/custom/V8XMLHttpRequestCustom.cpp (right): https://codereview.chromium.org/2472753003/diff/40001/third_party/WebKit/Source/bindings/core/v8/custom/V8XMLHttpRequestCustom.cpp#newcode89 third_party/WebKit/Source/bindings/core/v8/custom/V8XMLHttpRequestCustom.cpp:89: info.Holder(), info.GetIsolate()); Would you put this at line ...
4 years, 1 month ago (2016-11-03 14:26:38 UTC) #7
rwlbuis
On 2016/11/03 14:26:38, haraken wrote: > https://codereview.chromium.org/2472753003/diff/40001/third_party/WebKit/Source/bindings/core/v8/custom/V8XMLHttpRequestCustom.cpp#newcode89 > third_party/WebKit/Source/bindings/core/v8/custom/V8XMLHttpRequestCustom.cpp:89: > info.Holder(), info.GetIsolate()); > > Would ...
4 years, 1 month ago (2016-11-03 14:37:58 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/2472753003/60001
4 years, 1 month ago (2016-11-03 14:40:36 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/328062)
4 years, 1 month ago (2016-11-03 16:48:39 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/2472753003/80001
4 years, 1 month ago (2016-11-03 19:23:22 UTC) #16
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/330485)
4 years, 1 month ago (2016-11-03 22:15:11 UTC) #18
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/2472753003/80001
4 years, 1 month ago (2016-11-03 22:33:46 UTC) #20
commit-bot: I haz the power
Committed patchset #5 (id:80001)
4 years, 1 month ago (2016-11-04 01:46:00 UTC) #22
commit-bot: I haz the power
4 years, 1 month ago (2016-11-04 01:47:47 UTC) #24
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/36757cf38870dd6fa9f4e4110edf077a7a4ac8d8
Cr-Commit-Position: refs/heads/master@{#429762}

Powered by Google App Engine
This is Rietveld 408576698