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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 197213007: ScriptPromise implementation for V8 Promises (Closed)

Created:
6 years, 9 months ago by yhirano
Modified:
6 years, 9 months ago
CC:
blink-reviews, Nils Barth (inactive), kojih, arv+blink, jsbell+bindings_chromium.org, sof, kouhei+bindings_chromium.org, abarth-chromium, marja+watch_chromium.org, adamk+blink_chromium.org, haraken, Nate Chapin, watchdog-blink-watchlist_google.com, Inactive
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

ScriptPromise implementation for V8 Promises Add the implementation for V8 Promises in ScriptPromise and ScriptPromiseResolver so that: - They can handle V8 Promises correctly. - ScriptPromise creates a V8 Promise if ScriptPromiseOnV8Promise turns on. Add ScriptPromiseOnV8Promise runtime enabled flag which is always off. Add Layout tests for ScriptPromise. BUG=352552 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169516

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : #

Patch Set 5 : #

Total comments: 2

Patch Set 6 : #

Patch Set 7 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+204 lines, -85 lines) Patch
A LayoutTests/fast/js/Promise-native-create.html View 1 chunk +21 lines, -0 lines 0 comments Download
A + LayoutTests/fast/js/Promise-native-create-expected.txt View 1 chunk +4 lines, -5 lines 0 comments Download
A LayoutTests/fast/js/Promise-native-reject.html View 1 2 3 1 chunk +33 lines, -0 lines 0 comments Download
A + LayoutTests/fast/js/Promise-native-reject-expected.txt View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
A + LayoutTests/fast/js/Promise-native-resolve.html View 1 2 3 1 chunk +7 lines, -8 lines 0 comments Download
A + LayoutTests/fast/js/Promise-native-resolve-expected.txt View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M Source/bindings/v8/ScriptPromise.h View 1 2 3 1 chunk +0 lines, -6 lines 0 comments Download
M Source/bindings/v8/ScriptPromise.cpp View 1 2 3 4 5 4 chunks +41 lines, -22 lines 0 comments Download
M Source/bindings/v8/ScriptPromiseResolver.h View 1 2 3 4 4 chunks +12 lines, -10 lines 0 comments Download
M Source/bindings/v8/ScriptPromiseResolver.cpp View 1 2 3 4 5 6 4 chunks +44 lines, -19 lines 0 comments Download
M Source/bindings/v8/ScriptPromiseResolverTest.cpp View 1 2 3 5 chunks +11 lines, -11 lines 0 comments Download
M Source/core/testing/Internals.h View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/testing/Internals.cpp View 1 2 3 4 5 6 2 chunks +22 lines, -0 lines 0 comments Download
M Source/core/testing/Internals.idl View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M Source/platform/RuntimeEnabledFeatures.in View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 23 (0 generated)
yhirano
abarth: Can you take a look at this CL? jochen: FYI, do you have an ...
6 years, 9 months ago (2014-03-14 10:23:51 UTC) #1
yhirano
abarth: I'm sorry, V8 API has changed a bit[1]. Please stop reviewing. [1] https://codereview.chromium.org/196943014/
6 years, 9 months ago (2014-03-14 10:48:22 UTC) #2
rossberg1
On 14 March 2014 11:48, <yhirano@chromium.org> wrote: > abarth: I'm sorry, V8 API has changed ...
6 years, 9 months ago (2014-03-14 10:52:56 UTC) #3
yhirano
abarth: It is ready now, PTAL.
6 years, 9 months ago (2014-03-17 12:05:01 UTC) #4
jochen (gone - plz use gerrit)
lgtm https://codereview.chromium.org/197213007/diff/80001/Source/bindings/v8/ScriptPromise.cpp File Source/bindings/v8/ScriptPromise.cpp (right): https://codereview.chromium.org/197213007/diff/80001/Source/bindings/v8/ScriptPromise.cpp#newcode45 Source/bindings/v8/ScriptPromise.cpp:45: if (value.IsEmpty()) { nit. no { }
6 years, 9 months ago (2014-03-17 13:54:18 UTC) #5
abarth-chromium
Do you still want me to review? jochen is a good reviewer for this area, ...
6 years, 9 months ago (2014-03-17 16:23:38 UTC) #6
yhirano
https://codereview.chromium.org/197213007/diff/80001/Source/bindings/v8/ScriptPromise.cpp File Source/bindings/v8/ScriptPromise.cpp (right): https://codereview.chromium.org/197213007/diff/80001/Source/bindings/v8/ScriptPromise.cpp#newcode45 Source/bindings/v8/ScriptPromise.cpp:45: if (value.IsEmpty()) { On 2014/03/17 13:54:19, jochen wrote: > ...
6 years, 9 months ago (2014-03-18 02:17:45 UTC) #7
yhirano
On 2014/03/17 16:23:38, abarth wrote: > Do you still want me to review? jochen is ...
6 years, 9 months ago (2014-03-18 02:18:06 UTC) #8
yhirano
The CQ bit was checked by yhirano@chromium.org
6 years, 9 months ago (2014-03-18 02:18:11 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yhirano@chromium.org/197213007/100001
6 years, 9 months ago (2014-03-18 03:18:08 UTC) #10
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-18 03:30:56 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on mac_blink_compile_dbg tryserver.blink on mac_blink_rel
6 years, 9 months ago (2014-03-18 03:30:57 UTC) #12
yhirano
The CQ bit was checked by yhirano@chromium.org
6 years, 9 months ago (2014-03-18 15:21:04 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yhirano@chromium.org/197213007/100001
6 years, 9 months ago (2014-03-18 15:21:08 UTC) #14
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-18 15:22:43 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on mac_blink_rel
6 years, 9 months ago (2014-03-18 15:22:45 UTC) #16
yhirano
The CQ bit was checked by yhirano@chromium.org
6 years, 9 months ago (2014-03-18 17:00:23 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yhirano@chromium.org/197213007/100001
6 years, 9 months ago (2014-03-18 17:00:40 UTC) #18
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-18 17:04:36 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on linux_blink_dbg
6 years, 9 months ago (2014-03-18 17:04:37 UTC) #20
yhirano
The CQ bit was checked by yhirano@chromium.org
6 years, 9 months ago (2014-03-19 02:34:24 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yhirano@chromium.org/197213007/120001
6 years, 9 months ago (2014-03-19 02:34:34 UTC) #22
commit-bot: I haz the power
6 years, 9 months ago (2014-03-19 03:39:24 UTC) #23
Message was sent while issue was closed. 
Change committed as 169516

Powered by Google App Engine
This is Rietveld 408576698