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

Issue 1413583002: Make ElementShadow::youngestShadowRoot() return a reference (Closed)

Created:
5 years, 2 months ago by hayato
Modified:
5 years, 2 months ago
Reviewers:
kojii, kochi
CC:
chromium-reviews, devtools-reviews_chromium.org, caseq+blink_chromium.org, blink-reviews-style_chromium.org, pfeldman+blink_chromium.org, webcomponents-bugzilla_chromium.org, blink-reviews-css, sof, eae+blinkwatch, yurys+blink_chromium.org, lushnikov+blink_chromium.org, blink-reviews-dom_chromium.org, dglazkov+blink, apavlov+blink_chromium.org, darktears, blink-reviews, sergeyv+blink_chromium.org, kozyatinskiy+blink_chromium.org, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Make ElementShadow::youngestShadowRoot() return a reference. Since ElementShadow::m_shadowRoots is never empty after ElementShadowRoot is created, Element::yougestShadowRoot() can return a reference, instead of a pointer. Note that ElementShadow::addShadowRoot() is a exception because it could be called when ElementShadow is created. BUG=531990 Committed: https://crrev.com/7eb2d6e7e66570ce60cd3e4a19c4db2369fc8d21 Cr-Commit-Position: refs/heads/master@{#354750}

Patch Set 1 #

Patch Set 2 : wip #

Patch Set 3 : clean up #

Patch Set 4 : Use m_shadowRoots.isEmpty() #

Total comments: 4

Patch Set 5 : tail -> head #

Total comments: 2

Patch Set 6 : one more tail -> head #

Messages

Total messages: 35 (11 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1413583002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1413583002/1
5 years, 2 months ago (2015-10-16 09:20:01 UTC) #2
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_gn_chromeos_rel/builds/94754) linux_chromium_rel_ng on ...
5 years, 2 months ago (2015-10-16 09:28:54 UTC) #4
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1413583002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1413583002/20001
5 years, 2 months ago (2015-10-16 10:20:47 UTC) #6
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 2 months ago (2015-10-16 11:43:08 UTC) #8
hayato
clean up
5 years, 2 months ago (2015-10-19 05:09:07 UTC) #10
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1413583002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1413583002/40001
5 years, 2 months ago (2015-10-19 05:09:13 UTC) #11
hayato
PTAL
5 years, 2 months ago (2015-10-19 05:10:07 UTC) #13
kochi
On 2015/10/19 05:10:07, hayato wrote: > PTAL The description seems confusing, as it say: "Since ...
5 years, 2 months ago (2015-10-19 05:35:39 UTC) #14
hayato
Thanks. I've updated the description. > And if it is true, how can the change ...
5 years, 2 months ago (2015-10-19 06:19:13 UTC) #15
hayato
Use m_shadowRoots.isEmpty()
5 years, 2 months ago (2015-10-19 06:54:24 UTC) #17
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1413583002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1413583002/60001
5 years, 2 months ago (2015-10-19 06:54:36 UTC) #18
hayato
https://codereview.chromium.org/1413583002/diff/60001/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp File third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp (right): https://codereview.chromium.org/1413583002/diff/60001/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp#newcode152 third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp:152: if (m_shadowRoots.isEmpty()) { Updated: Use m_shadowRoots.isEmpty() here because !m_shadowRoots.tails() ...
5 years, 2 months ago (2015-10-19 06:58:19 UTC) #19
kochi
https://codereview.chromium.org/1413583002/diff/60001/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp File third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp (right): https://codereview.chromium.org/1413583002/diff/60001/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp#newcode152 third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp:152: if (m_shadowRoots.isEmpty()) { On 2015/10/19 06:58:19, hayato wrote: > ...
5 years, 2 months ago (2015-10-19 07:02:03 UTC) #20
hayato
tail -> head
5 years, 2 months ago (2015-10-19 07:10:39 UTC) #22
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1413583002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1413583002/80001
5 years, 2 months ago (2015-10-19 07:10:50 UTC) #23
hayato
https://codereview.chromium.org/1413583002/diff/60001/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp File third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp (right): https://codereview.chromium.org/1413583002/diff/60001/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp#newcode164 third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp:164: for (ShadowRoot* root = m_shadowRoots.tail(); root; root = root->olderShadowRoot()) ...
5 years, 2 months ago (2015-10-19 07:12:11 UTC) #24
kochi
https://codereview.chromium.org/1413583002/diff/80001/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp File third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp (right): https://codereview.chromium.org/1413583002/diff/80001/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp#newcode154 third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp:154: } else if (m_shadowRoots.tail()->type() == ShadowRootType::UserAgent) { Sorry for ...
5 years, 2 months ago (2015-10-19 07:19:38 UTC) #25
hayato
one more tail -> head
5 years, 2 months ago (2015-10-19 08:10:45 UTC) #27
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1413583002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1413583002/100001
5 years, 2 months ago (2015-10-19 08:10:55 UTC) #28
hayato
Thanks. I forgot to fix that. https://codereview.chromium.org/1413583002/diff/80001/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp File third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp (right): https://codereview.chromium.org/1413583002/diff/80001/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp#newcode154 third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp:154: } else if ...
5 years, 2 months ago (2015-10-19 08:14:45 UTC) #29
kochi
lgtm
5 years, 2 months ago (2015-10-19 08:21:22 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1413583002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1413583002/100001
5 years, 2 months ago (2015-10-19 09:23:09 UTC) #33
commit-bot: I haz the power
Committed patchset #6 (id:100001)
5 years, 2 months ago (2015-10-19 10:03:21 UTC) #34
commit-bot: I haz the power
5 years, 2 months ago (2015-10-19 10:04:21 UTC) #35
Message was sent while issue was closed.
Patchset 6 (id:??) landed as
https://crrev.com/7eb2d6e7e66570ce60cd3e4a19c4db2369fc8d21
Cr-Commit-Position: refs/heads/master@{#354750}

Powered by Google App Engine
This is Rietveld 408576698