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

Issue 18469002: Introduce ScriptPromiseResolver. (Closed)

Created:
7 years, 5 months ago by yhirano
Modified:
7 years, 5 months ago
Reviewers:
abarth-chromium, marja
CC:
blink-reviews, dglazkov+blink, eae+blinkwatch, adamk+blink_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Introduce ScriptPromiseResolver. We are implementing DOM/promises[1]. This CL introduces ScriptPromiseResolver which enables C++ code to generate a Promise and a PromiseResolver and to fullfill / reject the PromiseResolver. [1]: http://dom.spec.whatwg.org/#promises R=abarth BUG=243345 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=153745

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Total comments: 14

Patch Set 4 : #

Patch Set 5 : #

Total comments: 13

Patch Set 6 : #

Patch Set 7 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+494 lines, -0 lines) Patch
M Source/WebKit/chromium/WebKitUnitTests.gyp View 1 2 3 4 5 2 chunks +2 lines, -0 lines 0 comments Download
M Source/bindings/bindings.gypi View 1 2 3 4 5 2 chunks +5 lines, -0 lines 0 comments Download
A Source/bindings/v8/ScriptPromiseResolver.h View 1 2 3 4 5 1 chunk +134 lines, -0 lines 0 comments Download
A Source/bindings/v8/ScriptPromiseResolver.cpp View 1 2 3 4 5 6 1 chunk +155 lines, -0 lines 0 comments Download
A Source/bindings/v8/ScriptPromiseResolverTest.cpp View 1 2 3 4 5 1 chunk +198 lines, -0 lines 0 comments Download

Messages

Total messages: 15 (0 generated)
yhirano
PTAL? I am wondering about the following points: - Can I use v8 API in ...
7 years, 5 months ago (2013-07-02 04:11:44 UTC) #1
abarth-chromium
https://codereview.chromium.org/18469002/diff/6/Source/core/dom/PromiseHolder.cpp File Source/core/dom/PromiseHolder.cpp (right): https://codereview.chromium.org/18469002/diff/6/Source/core/dom/PromiseHolder.cpp#newcode37 Source/core/dom/PromiseHolder.cpp:37: #include "bindings/v8/custom/V8PromiseCustom.h" It looks like this class should be ...
7 years, 5 months ago (2013-07-03 17:49:59 UTC) #2
abarth-chromium
On 2013/07/02 04:11:44, yhirano wrote: > - Should I disable PromiseHolder if RuntimeEnabledFeatures::promiseEnabled() > returns ...
7 years, 5 months ago (2013-07-03 17:50:25 UTC) #3
yhirano
> https://codereview.chromium.org/18469002/diff/6/Source/core/dom/PromiseHolder.cpp#newcode37 > Source/core/dom/PromiseHolder.cpp:37: #include > "bindings/v8/custom/V8PromiseCustom.h" > It looks like this class should be ...
7 years, 5 months ago (2013-07-04 01:46:36 UTC) #4
abarth-chromium
On 2013/07/04 01:46:36, yhirano wrote: > Can you tell me where I should place the ...
7 years, 5 months ago (2013-07-04 01:49:43 UTC) #5
yhirano
https://codereview.chromium.org/18469002/diff/6/Source/core/dom/PromiseHolder.cpp File Source/core/dom/PromiseHolder.cpp (right): https://codereview.chromium.org/18469002/diff/6/Source/core/dom/PromiseHolder.cpp#newcode37 Source/core/dom/PromiseHolder.cpp:37: #include "bindings/v8/custom/V8PromiseCustom.h" On 2013/07/03 17:49:59, abarth wrote: > It ...
7 years, 5 months ago (2013-07-04 06:38:11 UTC) #6
abarth-chromium
On 2013/07/04 06:38:11, yhirano wrote: > https://codereview.chromium.org/18469002/diff/6/Source/core/dom/PromiseHolder.cpp#newcode94 > Source/core/dom/PromiseHolder.cpp:94: } > On 2013/07/03 17:49:59, abarth ...
7 years, 5 months ago (2013-07-08 18:20:34 UTC) #7
abarth-chromium
LGTM https://codereview.chromium.org/18469002/diff/23001/Source/bindings/v8/ScriptPromiseResolver.cpp File Source/bindings/v8/ScriptPromiseResolver.cpp (right): https://codereview.chromium.org/18469002/diff/23001/Source/bindings/v8/ScriptPromiseResolver.cpp#newcode47 Source/bindings/v8/ScriptPromiseResolver.cpp:47: { Should we ASSERT(RuntimeEnabledFeatures::promiseEnabled()) ? https://codereview.chromium.org/18469002/diff/23001/Source/bindings/v8/ScriptPromiseResolver.h File Source/bindings/v8/ScriptPromiseResolver.h ...
7 years, 5 months ago (2013-07-08 18:40:09 UTC) #8
yhirano
https://codereview.chromium.org/18469002/diff/23001/Source/bindings/v8/ScriptPromiseResolver.cpp File Source/bindings/v8/ScriptPromiseResolver.cpp (right): https://codereview.chromium.org/18469002/diff/23001/Source/bindings/v8/ScriptPromiseResolver.cpp#newcode47 Source/bindings/v8/ScriptPromiseResolver.cpp:47: { On 2013/07/08 18:40:09, abarth wrote: > Should we ...
7 years, 5 months ago (2013-07-09 02:14:56 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/18469002/32001
7 years, 5 months ago (2013-07-09 02:15:13 UTC) #10
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 5 months ago (2013-07-09 02:32:04 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yhirano@chromium.org/18469002/48001
7 years, 5 months ago (2013-07-09 04:16:48 UTC) #12
commit-bot: I haz the power
Change committed as 153745
7 years, 5 months ago (2013-07-09 05:29:08 UTC) #13
marja
Your stuff is slightly conflicting with our Handlepocalypse work; this introduces new calls to ScopedPersistent::get ...
7 years, 5 months ago (2013-07-16 11:14:51 UTC) #14
abarth-chromium
7 years, 5 months ago (2013-07-16 18:52:00 UTC) #15
Message was sent while issue was closed.
On 2013/07/16 11:14:51, marja wrote:
> Your stuff is slightly conflicting with our Handlepocalypse work; this
> introduces new calls to ScopedPersistent::get which I'm trying to remove. I'll
> fix this.

Can you rename ScopedPersistent::get to something like
ScopedPersistent::deprecatedGet so that folks know not to add more callers?

Powered by Google App Engine
This is Rietveld 408576698