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

Issue 2242743002: Make custom elements work in HTML imports (Closed)

Created:
4 years, 4 months ago by kochi
Modified:
4 years, 3 months ago
CC:
blink-reviews, blink-reviews-dom_chromium.org, chromium-reviews, dglazkov+blink, dominicc+watchlist_chromium.org, eae+blinkwatch, kojii, rwlbuis, sof, tfarina, webcomponents-bugzilla_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Make custom elements work in HTML imports Continuation from https://codereview.chromium.org/2132343002/ In HTML imports, custom elements should be created or upgraded. BUG=594918 Committed: https://crrev.com/2443446ab5e812272779bb80a82d680fb4d92126 Cr-Commit-Position: refs/heads/master@{#414263}

Patch Set 1 #

Patch Set 2 : fix #

Patch Set 3 : +1 nesting #

Patch Set 4 : add more tests #

Patch Set 5 : rebase wip #

Total comments: 42

Patch Set 6 : reparent #

Patch Set 7 : wip fixed detached document handling #

Patch Set 8 : tests should pass #

Patch Set 9 : update test #

Total comments: 15

Patch Set 10 : reparent #

Patch Set 11 : WIP cpp update for UpgradeSorter #

Patch Set 12 : resolve most comments #

Total comments: 15

Patch Set 13 : wip #

Patch Set 14 : wip (CustomElementsRegistry.cpp change is gone) #

Total comments: 7

Patch Set 15 : Add TODO and stylistic change for create-element.html #

Patch Set 16 : Update layout tests for more consistency. #

Patch Set 17 : Tentative: back out CEDefitions.cpp change and add failure TestExpectations #

Patch Set 18 : Add detached import test case. #

Patch Set 19 : Clean up CustomElementUpgradeSorter code. #

Patch Set 20 : clean up tests, remove redundant cases #

Total comments: 8

Patch Set 21 : Addresses tkent's comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+314 lines, -16 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 chunk +2 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/custom-elements/imports/async-imports.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +21 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/custom-elements/imports/async-nested-imports.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +30 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/custom-elements/imports/circular-imports.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +3 lines, -1 line 0 comments Download
A third_party/WebKit/LayoutTests/custom-elements/imports/create-element-in-import.html View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/custom-elements/imports/detached-import.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +21 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/custom-elements/imports/inner-html-in-import.html View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/custom-elements/imports/nested-imports.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +2 lines, -1 line 0 comments Download
A third_party/WebKit/LayoutTests/custom-elements/imports/resources/async-component.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +8 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/custom-elements/imports/resources/async-nested-component.html View 1 2 3 4 5 6 7 1 chunk +5 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/custom-elements/imports/resources/circular-level1.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/custom-elements/imports/resources/circular-level2.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/custom-elements/imports/resources/create-element.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +38 lines, -0 lines 0 comments Download
A + third_party/WebKit/LayoutTests/custom-elements/imports/resources/import-custom.html View 1 chunk +1 line, -1 line 0 comments Download
A third_party/WebKit/LayoutTests/custom-elements/imports/resources/inner-html.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +28 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/custom-elements/imports/resources/nested-level1.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/custom-elements/imports/resources/nested-level2.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/custom-elements/imports/resources/nested-level3.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/custom-elements/imports/resources/upgrade.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +13 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/custom-elements/imports/sync-create-element-order.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +45 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/custom-elements/imports/upgrade-order.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 chunk +30 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/custom-elements/spec/resources/custom-elements-helpers.js View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Document.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/Document.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/ExecutionContext.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/custom/CustomElement.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +6 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 3 chunks +36 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorterTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 55 (35 generated)
dominicc (has gone to gerrit)
Some comments inline. I did not get through the whole thing but I started seeing ...
4 years, 4 months ago (2016-08-19 04:54:11 UTC) #8
kochi
I think tests are ready for next round of reviews. I'm working on C++ side ...
4 years, 4 months ago (2016-08-19 10:51:48 UTC) #9
dominicc (has gone to gerrit)
Some comments inline. Some of these comments are repetitive from last time so I have ...
4 years, 4 months ago (2016-08-22 01:46:36 UTC) #10
kochi
Resolved most of comments, though there are still some remaining. Could you take a look? ...
4 years, 4 months ago (2016-08-22 07:37:35 UTC) #11
dominicc (has gone to gerrit)
Great! This is starting to come together. https://codereview.chromium.org/2242743002/diff/300001/third_party/WebKit/LayoutTests/custom-elements/spec/resources/custom-elements-helpers.js File third_party/WebKit/LayoutTests/custom-elements/spec/resources/custom-elements-helpers.js (right): https://codereview.chromium.org/2242743002/diff/300001/third_party/WebKit/LayoutTests/custom-elements/spec/resources/custom-elements-helpers.js#newcode47 third_party/WebKit/LayoutTests/custom-elements/spec/resources/custom-elements-helpers.js:47: function assert_is_upgraded(element, ...
4 years, 4 months ago (2016-08-22 08:07:36 UTC) #13
dominicc (has gone to gerrit)
https://codereview.chromium.org/2242743002/diff/300001/third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp File third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp (right): https://codereview.chromium.org/2242743002/diff/300001/third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp#newcode62 third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp:62: for (Node* n = element, *parent = n->parentOrShadowHostNode(); parent;) ...
4 years, 4 months ago (2016-08-22 08:10:55 UTC) #14
kochi
https://codereview.chromium.org/2242743002/diff/300001/third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.cpp File third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.cpp (right): https://codereview.chromium.org/2242743002/diff/300001/third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.cpp#newcode58 third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.cpp:58: if (&element->document() != &document On 2016/08/22 08:07:36, dominicc wrote: ...
4 years, 4 months ago (2016-08-22 12:02:00 UTC) #17
dominicc (has gone to gerrit)
A couple more comments inline. https://codereview.chromium.org/2242743002/diff/290023/third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.cpp File third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.cpp (right): https://codereview.chromium.org/2242743002/diff/290023/third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.cpp#newcode58 third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.cpp:58: if (&element->document() != &document ...
4 years, 4 months ago (2016-08-22 12:05:58 UTC) #18
kochi
https://codereview.chromium.org/2242743002/diff/290023/third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.cpp File third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.cpp (right): https://codereview.chromium.org/2242743002/diff/290023/third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.cpp#newcode58 third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.cpp:58: if (&element->document() != &document On 2016/08/22 12:05:58, dominicc (OOF ...
4 years, 4 months ago (2016-08-22 15:56:30 UTC) #21
kochi
https://codereview.chromium.org/2242743002/diff/290023/third_party/WebKit/Source/core/dom/custom/CustomElement.cpp File third_party/WebKit/Source/core/dom/custom/CustomElement.cpp (right): https://codereview.chromium.org/2242743002/diff/290023/third_party/WebKit/Source/core/dom/custom/CustomElement.cpp#newcode30 third_party/WebKit/Source/core/dom/custom/CustomElement.cpp:30: if (LocalDOMWindow* window = document.executingWindow()) This was necessary to ...
4 years, 4 months ago (2016-08-22 16:10:21 UTC) #22
kochi
Koji-san, Could you take a look at this CL?
4 years, 4 months ago (2016-08-24 06:22:45 UTC) #30
kojii
lgtm
4 years, 4 months ago (2016-08-24 09:10:38 UTC) #33
kochi
tkent-san, could you take a look? https://codereview.chromium.org/2242743002/diff/290023/third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp File third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp (right): https://codereview.chromium.org/2242743002/diff/290023/third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp#newcode54 third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp:54: // |parent| can ...
4 years, 4 months ago (2016-08-24 11:26:03 UTC) #35
tkent
lgtm https://codereview.chromium.org/2242743002/diff/490001/third_party/WebKit/LayoutTests/custom-elements/imports/upgrade-order.html File third_party/WebKit/LayoutTests/custom-elements/imports/upgrade-order.html (right): https://codereview.chromium.org/2242743002/diff/490001/third_party/WebKit/LayoutTests/custom-elements/imports/upgrade-order.html#newcode21 third_party/WebKit/LayoutTests/custom-elements/imports/upgrade-order.html:21: super(); wrong indentation. https://codereview.chromium.org/2242743002/diff/490001/third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp File third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp (right): https://codereview.chromium.org/2242743002/diff/490001/third_party/WebKit/Source/core/dom/custom/CustomElementUpgradeSorter.cpp#newcode23 ...
4 years, 4 months ago (2016-08-24 12:07:20 UTC) #38
kochi
https://codereview.chromium.org/2242743002/diff/490001/third_party/WebKit/LayoutTests/custom-elements/imports/upgrade-order.html File third_party/WebKit/LayoutTests/custom-elements/imports/upgrade-order.html (right): https://codereview.chromium.org/2242743002/diff/490001/third_party/WebKit/LayoutTests/custom-elements/imports/upgrade-order.html#newcode21 third_party/WebKit/LayoutTests/custom-elements/imports/upgrade-order.html:21: super(); On 2016/08/24 12:07:20, tkent wrote: > wrong indentation. ...
4 years, 4 months ago (2016-08-24 15:36:52 UTC) #43
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/2242743002/510001
4 years, 4 months ago (2016-08-24 15:37:57 UTC) #47
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/128912)
4 years, 4 months ago (2016-08-24 17:16:45 UTC) #49
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/2242743002/510001
4 years, 3 months ago (2016-08-24 23:14:41 UTC) #51
commit-bot: I haz the power
Committed patchset #21 (id:510001)
4 years, 3 months ago (2016-08-25 02:22:59 UTC) #53
commit-bot: I haz the power
4 years, 3 months ago (2016-08-25 02:25:43 UTC) #55
Message was sent while issue was closed.
Patchset 21 (id:??) landed as
https://crrev.com/2443446ab5e812272779bb80a82d680fb4d92126
Cr-Commit-Position: refs/heads/master@{#414263}

Powered by Google App Engine
This is Rietveld 408576698