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

Issue 2035623002: Implement the script parts of custom element upgrade steps. (Closed)

Created:
4 years, 6 months ago by dominicc (has gone to gerrit)
Modified:
4 years, 6 months ago
Reviewers:
haraken, blink-reviews-bindings, Sébastien Marchand, kojii, alph
CC:
blink-reviews, blink-reviews-bindings_chromium.org, blink-reviews-dom_chromium.org, chromium-reviews, dglazkov+blink, dominicc+watchlist_chromium.org, eae+blinkwatch, rwlbuis, sof, webcomponents-bugzilla_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@ce-upgrade-in-document-dom-merge2
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Implement the script parts of custom element upgrade steps. BUG=594918 Committed: https://crrev.com/ee58801ba1e4a49cc2eea50d970444c7eeff9593 Committed: https://crrev.com/7811435c7a6f93b2f016788e5a963324a8f0a854 Cr-Original-Commit-Position: refs/heads/master@{#397676} Cr-Commit-Position: refs/heads/master@{#397934}

Patch Set 1 #

Total comments: 5

Patch Set 2 : Feedback. #

Total comments: 2

Patch Set 3 : Do not assign in conditional to unbreak windows builder. #

Total comments: 1

Messages

Total messages: 30 (11 generated)
dominicc (has gone to gerrit)
PTAL blink-reviews-bindings: bindings Koji: everything I think we need to think about state setting more. ...
4 years, 6 months ago (2016-06-02 07:23:30 UTC) #2
haraken
The bindings look good. https://codereview.chromium.org/2035623002/diff/1/third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp File third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp (right): https://codereview.chromium.org/2035623002/diff/1/third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp#newcode444 third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp:444: v8::MaybeLocal<v8::Value> V8ScriptRunner::callAsConstructor(v8::Isolate* isolate, v8::Local<v8::Object> constructor, ...
4 years, 6 months ago (2016-06-02 09:27:34 UTC) #4
dominicc (has gone to gerrit)
Thanks for the feedback, PTAL. https://codereview.chromium.org/2035623002/diff/1/third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLElementCustom.cpp File third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLElementCustom.cpp (right): https://codereview.chromium.org/2035623002/diff/1/third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLElementCustom.cpp#newcode85 third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLElementCustom.cpp:85: v8SetReturnValue(info, wrapper); On 2016/06/02 ...
4 years, 6 months ago (2016-06-03 07:37:58 UTC) #5
haraken
bindings/ LGTM
4 years, 6 months ago (2016-06-03 07:40:12 UTC) #6
kojii
lgtm https://codereview.chromium.org/2035623002/diff/20001/third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLElementCustom.cpp File third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLElementCustom.cpp (right): https://codereview.chromium.org/2035623002/diff/20001/third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLElementCustom.cpp#newcode58 third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLElementCustom.cpp:58: element = window->document()->createElement( nit: this needs to be ...
4 years, 6 months ago (2016-06-03 07:50:42 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2035623002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2035623002/20001
4 years, 6 months ago (2016-06-03 08:37:02 UTC) #9
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 6 months ago (2016-06-03 10:39:11 UTC) #10
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/ee58801ba1e4a49cc2eea50d970444c7eeff9593 Cr-Commit-Position: refs/heads/master@{#397676}
4 years, 6 months ago (2016-06-03 10:41:00 UTC) #12
vabr (Chromium)
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/2032373002/ by vabr@chromium.org. ...
4 years, 6 months ago (2016-06-03 11:11:44 UTC) #13
Sébastien Marchand
https://codereview.chromium.org/2035623002/diff/20001/third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLElementCustom.cpp File third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLElementCustom.cpp (right): https://codereview.chromium.org/2035623002/diff/20001/third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLElementCustom.cpp#newcode64 third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLElementCustom.cpp:64: } else if ((element = definition->constructionStack().last())) { This is ...
4 years, 6 months ago (2016-06-03 13:34:33 UTC) #15
Sébastien Marchand
just saw that this has been reverted.
4 years, 6 months ago (2016-06-03 13:35:52 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2035623002/40001
4 years, 6 months ago (2016-06-04 04:42:10 UTC) #19
kojii
still lgtm
4 years, 6 months ago (2016-06-04 04:45:25 UTC) #20
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/233495)
4 years, 6 months ago (2016-06-04 09:11:08 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2035623002/40001
4 years, 6 months ago (2016-06-04 16:48:11 UTC) #24
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 6 months ago (2016-06-04 18:25:08 UTC) #25
commit-bot: I haz the power
Patchset 3 (id:??) landed as https://crrev.com/7811435c7a6f93b2f016788e5a963324a8f0a854 Cr-Commit-Position: refs/heads/master@{#397934}
4 years, 6 months ago (2016-06-04 18:26:53 UTC) #27
alph
https://codereview.chromium.org/2035623002/diff/40001/third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp File third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp (right): https://codereview.chromium.org/2035623002/diff/40001/third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp#newcode476 third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp:476: TRACE_EVENT_END0("devtools.timeline", "FunctionCall"); microtasksScope destructor must be called before recording ...
4 years, 6 months ago (2016-06-09 00:22:47 UTC) #29
dominicc (has gone to gerrit)
4 years, 6 months ago (2016-06-10 08:07:25 UTC) #30
Message was sent while issue was closed.
On 2016/06/09 at 00:22:47, alph wrote:
>
https://codereview.chromium.org/2035623002/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp (right):
> 
>
https://codereview.chromium.org/2035623002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp:476:
TRACE_EVENT_END0("devtools.timeline", "FunctionCall");
> microtasksScope destructor must be called before recording TRACE_EVENT_END.

Thank you for pointing this out. Sorry for the slow reply; just reached this in
my inbox. I see that you submitted a fix for this--thank you. Sorry for the foul
up.

Powered by Google App Engine
This is Rietveld 408576698