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

Issue 2473743003: Call Element::rebuildLayoutTree from Document::updateStyle directly (Closed)

Created:
4 years, 1 month ago by nainar
Modified:
3 years, 9 months ago
Reviewers:
esprehn, rune
CC:
blink-reviews, blink-reviews-dom_chromium.org, blink-reviews-html_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, meade_UTC10, Mike Lawther (Google), rwlbuis, shans, sof, webcomponents-bugzilla_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Call Element::rebuildLayoutTree from Document::updateStyle directly This patch changes the callstack in Document::updateStyle to call Element::rebuildLayoutTree after Element::recalcStyle instead of from it. It also adds helper functions needed to perform Layout Tree Construction: 1. ContainerNode::rebuildChildrenLayoutTrees() 2. Element::rebuildShadowRootLayoutTree() 3. Element::reattachPseudoElementLayoutTree(PseudoId pseudoId) 4. ShadowRoot::rebuildLayoutTree() It also changes the setting/getting of the (Child)NeedsStyleRecalc and (Child)NeedsReattachLayoutTree bits to match this pattern. The patch also adds a LayoutTest to test for the crash caught in crbug.com/695950. BUG=595137 Review-Url: https://codereview.chromium.org/2473743003 Cr-Original-Commit-Position: refs/heads/master@{#451987} Committed: https://chromium.googlesource.com/chromium/src/+/8b6c7b7ce5a36d8eda49dd6e60c5e7c8f5c7277f Review-Url: https://codereview.chromium.org/2473743003 Cr-Commit-Position: refs/heads/master@{#454824} Committed: https://chromium.googlesource.com/chromium/src/+/f4c976fc1f4578b0c75ba304553ef2ff6134f551

Patch Set 1 #

Total comments: 3

Patch Set 2 : Done 1 #

Patch Set 3 : Push to Reitveld #

Patch Set 4 : For comments #

Patch Set 5 : Commit the renames #

Total comments: 19

Patch Set 6 : Stage 1 #

Patch Set 7 : FirstLetterPseudoElements are not being created #

Total comments: 15

Patch Set 8 : Addressing bugsnash's comments #

Patch Set 9 : Addressing bugsnash's comments #

Patch Set 10 : For review #

Total comments: 1

Patch Set 11 : Address esprehn's comments #

Total comments: 12

Patch Set 12 : Comments #

Patch Set 13 : Handover #

Total comments: 1

Patch Set 14 : Import bugsnash@ code and add comment #

Patch Set 15 : Everything works so far - failing test still #

Patch Set 16 : Everything works so far - failing test still #

Patch Set 17 : Split up didRecalcStyle into didRecalcStyle and didRebuildLayoutTree #

Total comments: 1

Patch Set 18 : Final patch - make sure to retain information needed to determine whether to call detachLayoutTree() #

Total comments: 6

Patch Set 19 : Remove redundant clear for root's NeedReattachLayoutTree flag #

Total comments: 1

Patch Set 20 : Clean up test failures #

Patch Set 21 : setNeedsReattachLayoutTree flag in ElementShadow::addShadowRoot #

Patch Set 22 : Fix failures due to information not being retained correctly for detachLayoutTree() in Text::reatta… #

Patch Set 23 : Fix failures due to information not being retained correctly for detachLayoutTree() in Text::reatta… #

Total comments: 14

Patch Set 24 : Fix unconditional ShadowRoot::rebuildLayoutTree() and retain needsStyleRecalc information for cases… #

Patch Set 25 : Fix unconditional ShadowRoot::rebuildLayoutTree() and retain needsStyleRecalc information for cases… #

Total comments: 2

Patch Set 26 : Mirror setChildNeedsStyleRecalc and markAncestorsWithNeedsReattachLayoutTree with setChildNeedsReat… #

Total comments: 22

Patch Set 27 : Clean up all the asserts #

Patch Set 28 : Clean up all the asserts #

Patch Set 29 : Clean up all the asserts #

Total comments: 5

Patch Set 30 : Clean up all the asserts #

Patch Set 31 : "StyleSharing should use info from map if available. Early store information on the HashMap if we h… #

Total comments: 4

Patch Set 32 : Fixing edge cases in code #

Total comments: 10

Patch Set 33 : Move reattachLayoutTree related code into the relevant block #

Patch Set 34 : Make needsAttach() only check if getStyleChangeType flag is NeedsReattachStyleChange and add a Layo… #

Total comments: 1

Patch Set 35 : Make needsAttach() only check if getStyleChangeType flag is NeedsReattachStyleChange and add a Layo… #

Patch Set 36 : Make needsAttach() only check if getStyleChangeType flag is NeedsReattachStyleChange and add a Layo… #

Unified diffs Side-by-side diffs Delta from patch set Stats (+154 lines, -54 lines) Patch
A third_party/WebKit/LayoutTests/fast/dom/HTMLDialogElement-crash-style-recalc-after-dialog-close.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 1 chunk +10 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/dom/HTMLDialogElement-crash-style-recalc-after-dialog-close-expected.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/resolver/SharedStyleFinder.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 3 chunks +15 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/dom/ContainerNode.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/ContainerNode.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 3 chunks +28 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Document.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 1 chunk +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Element.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 2 chunks +3 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/Element.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 4 chunks +56 lines, -31 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Node.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 3 chunks +7 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Node.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Text.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/Text.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 1 chunk +13 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/shadow/ShadowRoot.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 22 23 24 25 1 chunk +10 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/stylerecalc.md View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -2 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLFrameSetElement.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/style/ComputedStyleConstants.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 186 (103 generated)
nainar
Hi! Need some help figuring an issue out. Given this test case: <li> <p>Text</p> </li> ...
4 years, 1 month ago (2016-11-03 00:17:10 UTC) #2
rune
On 2016/11/03 00:17:10, nainar wrote: > Hi! > > Need some help figuring an issue ...
4 years, 1 month ago (2016-11-03 10:31:28 UTC) #3
nainar
On 2016/11/03 at 10:31:28, rune wrote: > On 2016/11/03 00:17:10, nainar wrote: > > Hi! ...
4 years, 1 month ago (2016-11-07 01:03:43 UTC) #4
esprehn
https://codereview.chromium.org/2473743003/diff/1/third_party/WebKit/Source/core/dom/Element.cpp File third_party/WebKit/Source/core/dom/Element.cpp (left): https://codereview.chromium.org/2473743003/diff/1/third_party/WebKit/Source/core/dom/Element.cpp#oldcode2014 third_party/WebKit/Source/core/dom/Element.cpp:2014: return ReattachNoLayoutObject; Is ReattachNoLayoutObject dead code now? https://codereview.chromium.org/2473743003/diff/1/third_party/WebKit/Source/core/dom/Element.cpp File ...
4 years, 1 month ago (2016-11-07 22:32:08 UTC) #5
nainar
Hi rune@, With regards to this patch, the meat of the code seems to be ...
4 years ago (2016-11-23 12:15:09 UTC) #7
rune
On 2016/11/23 12:15:09, nainar wrote: > Hi rune@, > > With regards to this patch, ...
4 years ago (2016-11-23 12:44:34 UTC) #8
nainar
On 2016/11/23 at 12:44:34, rune wrote: > On 2016/11/23 12:15:09, nainar wrote: > > Hi ...
4 years ago (2016-11-23 13:19:12 UTC) #9
rune
On 2016/11/23 13:19:12, nainar wrote: > On 2016/11/23 at 12:44:34, rune wrote: > > On ...
4 years ago (2016-11-24 08:30:51 UTC) #10
nainar
On 2016/11/24 at 08:30:51, rune wrote: > On 2016/11/23 13:19:12, nainar wrote: > > On ...
4 years ago (2016-11-24 08:59:11 UTC) #11
rune
On 2016/11/24 08:59:11, nainar wrote: > On 2016/11/24 at 08:30:51, rune wrote: > > On ...
4 years ago (2016-11-24 09:03:33 UTC) #12
rune
On 2016/11/24 08:59:11, nainar wrote: > If you take fast/css/first-letter-removed-added.html for example. While > everything ...
4 years ago (2016-11-24 09:07:22 UTC) #13
rune
On 2016/11/24 09:07:22, rune wrote: > On 2016/11/24 08:59:11, nainar wrote: > > > If ...
4 years ago (2016-11-24 09:22:38 UTC) #14
nainar
On 2016/11/24 at 09:22:38, rune wrote: > On 2016/11/24 09:07:22, rune wrote: > > On ...
4 years ago (2016-11-24 10:37:46 UTC) #15
rune
On 2016/11/24 10:37:46, nainar wrote: > On 2016/11/24 at 09:22:38, rune wrote: > > On ...
4 years ago (2016-11-24 10:40:38 UTC) #16
rune
On 2016/11/24 10:40:38, rune wrote: > On 2016/11/24 10:37:46, nainar wrote: > > On 2016/11/24 ...
4 years ago (2016-11-25 08:01:50 UTC) #17
nainar
On 2016/11/25 at 08:01:50, rune wrote: > On 2016/11/24 10:40:38, rune wrote: > > On ...
4 years ago (2016-11-25 08:46:36 UTC) #18
rune
On 2016/11/25 08:46:36, nainar wrote: > On 2016/11/25 at 08:01:50, rune wrote: > > On ...
4 years ago (2016-11-25 08:51:38 UTC) #19
nainar
On 2016/11/25 at 08:51:38, rune wrote: > On 2016/11/25 08:46:36, nainar wrote: > > On ...
4 years ago (2016-11-25 10:01:08 UTC) #20
rune
On 2016/11/25 10:01:08, nainar wrote: > On 2016/11/25 at 08:51:38, rune wrote: > > On ...
4 years ago (2016-11-25 10:15:27 UTC) #21
nainar
Hi rune@, I have made some changes and commented inline to add some context to ...
4 years ago (2016-11-29 06:13:39 UTC) #22
rune
I've applied the latest patch set on master from Saturday (no local patches this time). ...
4 years ago (2016-12-05 10:05:17 UTC) #25
esprehn
Per meeting discussion I think you want to leave a bunch of the needsAttach() machinery ...
4 years ago (2016-12-05 22:31:33 UTC) #26
nainar
@esprehn, Made the changes you asked for in patch 7. Changes explained inline as comments. ...
4 years ago (2016-12-07 14:48:22 UTC) #27
Bugs Nash
https://codereview.chromium.org/2473743003/diff/120001/third_party/WebKit/Source/core/dom/ContainerNode.h File third_party/WebKit/Source/core/dom/ContainerNode.h (right): https://codereview.chromium.org/2473743003/diff/120001/third_party/WebKit/Source/core/dom/ContainerNode.h#newcode243 third_party/WebKit/Source/core/dom/ContainerNode.h:243: void rebuildDescendantLayoutTrees(); This name is strange (as I commented ...
4 years ago (2016-12-07 23:21:44 UTC) #28
nainar
https://codereview.chromium.org/2473743003/diff/120001/third_party/WebKit/Source/core/dom/ContainerNode.h File third_party/WebKit/Source/core/dom/ContainerNode.h (right): https://codereview.chromium.org/2473743003/diff/120001/third_party/WebKit/Source/core/dom/ContainerNode.h#newcode243 third_party/WebKit/Source/core/dom/ContainerNode.h:243: void rebuildDescendantLayoutTrees(); On 2016/12/07 at 23:21:44, Bugs Nash wrote: ...
4 years ago (2016-12-07 23:49:10 UTC) #29
Bugs Nash
https://codereview.chromium.org/2473743003/diff/120001/third_party/WebKit/Source/core/dom/Node.cpp File third_party/WebKit/Source/core/dom/Node.cpp (right): https://codereview.chromium.org/2473743003/diff/120001/third_party/WebKit/Source/core/dom/Node.cpp#newcode916 third_party/WebKit/Source/core/dom/Node.cpp:916: setLocalNeedsReattachLayoutTree(); On 2016/12/07 at 23:49:10, nainar wrote: > On ...
4 years ago (2016-12-08 00:27:59 UTC) #30
nainar
Made all the changes and uploaded to patch 9. Thanks for the review! https://codereview.chromium.org/2473743003/diff/120001/third_party/WebKit/Source/core/dom/Node.cpp File ...
4 years ago (2016-12-08 00:45:48 UTC) #31
esprehn
Lgtm, amazing patch! This is how I feel about this patch: http://i.imgur.com/jrb2WgX.gif https://codereview.chromium.org/2473743003/diff/180001/third_party/WebKit/Source/core/dom/Text.cpp File third_party/WebKit/Source/core/dom/Text.cpp ...
4 years ago (2016-12-13 01:19:03 UTC) #36
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/2473743003/200001
4 years ago (2016-12-13 04:02:04 UTC) #39
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/354996)
4 years ago (2016-12-13 06:03:28 UTC) #41
esprehn
https://codereview.chromium.org/2473743003/diff/200001/third_party/WebKit/Source/core/dom/ContainerNode.cpp File third_party/WebKit/Source/core/dom/ContainerNode.cpp (right): https://codereview.chromium.org/2473743003/diff/200001/third_party/WebKit/Source/core/dom/ContainerNode.cpp#newcode1313 third_party/WebKit/Source/core/dom/ContainerNode.cpp:1313: clearChildNeedsStyleRecalc(); I don't think you need to clear this ...
4 years ago (2016-12-14 04:30:48 UTC) #49
nainar
Replied inline to your comments. https://codereview.chromium.org/2473743003/diff/200001/third_party/WebKit/Source/core/dom/ContainerNode.cpp File third_party/WebKit/Source/core/dom/ContainerNode.cpp (right): https://codereview.chromium.org/2473743003/diff/200001/third_party/WebKit/Source/core/dom/ContainerNode.cpp#newcode1313 third_party/WebKit/Source/core/dom/ContainerNode.cpp:1313: clearChildNeedsStyleRecalc(); Done. https://codereview.chromium.org/2473743003/diff/200001/third_party/WebKit/Source/core/dom/Element.cpp File ...
4 years ago (2016-12-14 22:21:55 UTC) #50
nainar
For the crashing test: svg/as-image/svg-nested.html. Attaching a gdb instance to content shell shows the following: ...
4 years ago (2016-12-15 01:25:33 UTC) #55
nainar
Elliott, I have rewritten the patch and fast/forms/textfield-to-password-on-focus.html is failing consistently across all platforms. This ...
3 years, 11 months ago (2017-01-25 09:09:13 UTC) #66
rune
On 2017/01/25 09:09:13, nainar wrote: > Elliott, > > I have rewritten the patch and ...
3 years, 11 months ago (2017-01-25 12:13:22 UTC) #69
rune
On 2017/01/25 12:13:22, rune wrote: > PS. I wonder if this was a problem without ...
3 years, 11 months ago (2017-01-25 12:19:50 UTC) #70
nainar
Thank you for the catch! Looking at the implementations for didStyleRecalc I think the following ...
3 years, 11 months ago (2017-01-27 07:08:16 UTC) #71
nainar
On 2017/01/27 at 07:08:16, nainar wrote: > Thank you for the catch! > > Looking ...
3 years, 10 months ago (2017-01-30 04:05:12 UTC) #76
esprehn
Hmm, didRebuildLayoutTree is the same as ::attachLayoutTree() if you call super first?
3 years, 10 months ago (2017-01-30 17:20:49 UTC) #77
nainar
Hi all, FYI https://codereview.chromium.org/2473743003/diff/320001/third_party/WebKit/Source/core/dom/Node.cpp File third_party/WebKit/Source/core/dom/Node.cpp (right): https://codereview.chromium.org/2473743003/diff/320001/third_party/WebKit/Source/core/dom/Node.cpp#newcode906 third_party/WebKit/Source/core/dom/Node.cpp:906: if (getStyleChangeType() < NeedsReattachStyleChange) Figured out ...
3 years, 10 months ago (2017-02-02 07:42:01 UTC) #78
nainar
Hi all, Patch with fix is uploaded now. PTAL?
3 years, 10 months ago (2017-02-02 08:14:11 UTC) #81
esprehn
Looking very cool, the unconditional reattach on shadow root means every style recalc through shadow ...
3 years, 10 months ago (2017-02-02 08:37:10 UTC) #82
nainar
Elliott, Answered your questions. https://codereview.chromium.org/2473743003/diff/340001/third_party/WebKit/Source/core/dom/Element.cpp File third_party/WebKit/Source/core/dom/Element.cpp (right): https://codereview.chromium.org/2473743003/diff/340001/third_party/WebKit/Source/core/dom/Element.cpp#newcode1930 third_party/WebKit/Source/core/dom/Element.cpp:1930: root->clearNeedsReattachLayoutTree(); Yup this is redundant ...
3 years, 10 months ago (2017-02-02 23:13:07 UTC) #85
esprehn
https://codereview.chromium.org/2473743003/diff/360001/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp File third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp (right): https://codereview.chromium.org/2473743003/diff/360001/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp#newcode155 third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp:155: // otherwise we will never rebuild the LayoutTree. I ...
3 years, 10 months ago (2017-02-02 23:31:03 UTC) #86
esprehn
On 2017/02/02 at 23:31:03, esprehn wrote: > https://codereview.chromium.org/2473743003/diff/360001/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp > File third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp (right): > > https://codereview.chromium.org/2473743003/diff/360001/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp#newcode155 ...
3 years, 10 months ago (2017-02-03 00:11:35 UTC) #87
nainar
On 2017/02/03 at 00:11:35, esprehn wrote: > On 2017/02/02 at 23:31:03, esprehn wrote: > > ...
3 years, 10 months ago (2017-02-03 00:44:01 UTC) #88
esprehn
https://codereview.chromium.org/2473743003/diff/440001/third_party/WebKit/Source/core/dom/ContainerNode.cpp File third_party/WebKit/Source/core/dom/ContainerNode.cpp (left): https://codereview.chromium.org/2473743003/diff/440001/third_party/WebKit/Source/core/dom/ContainerNode.cpp#oldcode771 third_party/WebKit/Source/core/dom/ContainerNode.cpp:771: childAttachedAllowedWhenAttachingChildren(this)); Lets add this back? https://codereview.chromium.org/2473743003/diff/440001/third_party/WebKit/Source/core/dom/ContainerNode.cpp#oldcode1283 third_party/WebKit/Source/core/dom/ContainerNode.cpp:1283: DCHECK(!needsStyleRecalc()); ditto ...
3 years, 10 months ago (2017-02-13 22:19:41 UTC) #101
nainar
Elliott, Made all the changes as per the meeting. Still working on a few asserts ...
3 years, 10 months ago (2017-02-14 03:42:09 UTC) #102
esprehn
What asserts do you see? https://codereview.chromium.org/2473743003/diff/480001/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp File third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp (right): https://codereview.chromium.org/2473743003/diff/480001/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp#newcode155 third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp:155: clearNeedsReattachLayoutTree(); this shouldn't be ...
3 years, 10 months ago (2017-02-15 02:01:00 UTC) #103
nainar
On 2017/02/15 at 02:01:00, esprehn wrote: > What asserts do you see? The asserts I ...
3 years, 10 months ago (2017-02-15 06:04:46 UTC) #104
nainar
On 2017/02/15 at 06:04:46, nainar wrote: > On 2017/02/15 at 02:01:00, esprehn wrote: > > ...
3 years, 10 months ago (2017-02-16 03:34:19 UTC) #105
nainar
Elliott, As per conversation on Hangouts. PTAL?
3 years, 10 months ago (2017-02-16 05:30:03 UTC) #106
esprehn
On 2017/02/16 at 05:30:03, nainar wrote: > Elliott, > > As per conversation on Hangouts. ...
3 years, 10 months ago (2017-02-16 05:37:02 UTC) #107
nainar
On 2017/02/16 at 05:37:02, esprehn wrote: > On 2017/02/16 at 05:30:03, nainar wrote: > > ...
3 years, 10 months ago (2017-02-16 05:42:01 UTC) #110
esprehn
https://codereview.chromium.org/2473743003/diff/500001/third_party/WebKit/Source/core/dom/ContainerNode.cpp File third_party/WebKit/Source/core/dom/ContainerNode.cpp (left): https://codereview.chromium.org/2473743003/diff/500001/third_party/WebKit/Source/core/dom/ContainerNode.cpp#oldcode1283 third_party/WebKit/Source/core/dom/ContainerNode.cpp:1283: DCHECK(!needsStyleRecalc()); add this back? Recomputing the descendant styles while ...
3 years, 10 months ago (2017-02-16 05:49:26 UTC) #111
nainar
https://codereview.chromium.org/2473743003/diff/500001/third_party/WebKit/Source/core/dom/ContainerNode.cpp File third_party/WebKit/Source/core/dom/ContainerNode.cpp (left): https://codereview.chromium.org/2473743003/diff/500001/third_party/WebKit/Source/core/dom/ContainerNode.cpp#oldcode1283 third_party/WebKit/Source/core/dom/ContainerNode.cpp:1283: DCHECK(!needsStyleRecalc()); Done. Needed to move the if (!needsReattachLayoutTree()) clearNeedsStyleRecalc(); ...
3 years, 10 months ago (2017-02-16 06:47:54 UTC) #114
esprehn
https://codereview.chromium.org/2473743003/diff/500001/third_party/WebKit/Source/core/dom/Element.cpp File third_party/WebKit/Source/core/dom/Element.cpp (left): https://codereview.chromium.org/2473743003/diff/500001/third_party/WebKit/Source/core/dom/Element.cpp#oldcode1887 third_party/WebKit/Source/core/dom/Element.cpp:1887: DCHECK(!parentOrShadowHostNode()->needsStyleRecalc()); On 2017/02/16 at 06:47:54, nainar wrote: > This ...
3 years, 10 months ago (2017-02-16 06:54:12 UTC) #115
nainar
https://codereview.chromium.org/2473743003/diff/500001/third_party/WebKit/Source/core/dom/Element.cpp File third_party/WebKit/Source/core/dom/Element.cpp (left): https://codereview.chromium.org/2473743003/diff/500001/third_party/WebKit/Source/core/dom/Element.cpp#oldcode1887 third_party/WebKit/Source/core/dom/Element.cpp:1887: DCHECK(!parentOrShadowHostNode()->needsStyleRecalc()); Test case: <div style="display:none"> <input type="image" id="image" list="dl1"> ...
3 years, 10 months ago (2017-02-16 07:13:20 UTC) #116
nainar
Post investigation. https://codereview.chromium.org/2473743003/diff/560001/third_party/WebKit/Source/core/dom/Element.cpp File third_party/WebKit/Source/core/dom/Element.cpp (right): https://codereview.chromium.org/2473743003/diff/560001/third_party/WebKit/Source/core/dom/Element.cpp#newcode1906 third_party/WebKit/Source/core/dom/Element.cpp:1906: if (change != Reattach) This was initially ...
3 years, 10 months ago (2017-02-16 08:05:57 UTC) #119
esprehn
This looks good great! ...but your bots are red. Looks like maybe you broke style ...
3 years, 10 months ago (2017-02-16 21:24:33 UTC) #122
nainar
On 2017/02/16 at 21:24:33, esprehn wrote: > This looks good great! ...but your bots are ...
3 years, 10 months ago (2017-02-16 22:26:09 UTC) #123
nainar
On a completely unrelated note. Future things to consider: 1. Move StyleChange information on to ...
3 years, 10 months ago (2017-02-16 23:46:37 UTC) #126
Bugs Nash
Issue 658092 has been closed by clusterfuzz, perhaps it should it be removed from this ...
3 years, 10 months ago (2017-02-17 01:02:11 UTC) #129
nainar
With the current patch I have the following failing tests: fast/css/first-letter-rtc-crash.html fast/layout/display-none-no-relayout.html - relaying out ...
3 years, 10 months ago (2017-02-17 07:46:48 UTC) #133
nainar
Also I have run the memory.top_10_mobile benchmark on all android bots and am now running ...
3 years, 10 months ago (2017-02-21 03:13:29 UTC) #136
nainar
On 2017/02/21 at 03:13:29, nainar wrote: > Also I have run the memory.top_10_mobile benchmark on ...
3 years, 10 months ago (2017-02-21 06:23:33 UTC) #137
Bugs Nash
On 2017/02/21 at 06:23:33, nainar wrote: > On 2017/02/21 at 03:13:29, nainar wrote: > > ...
3 years, 10 months ago (2017-02-22 00:51:14 UTC) #138
nainar
Uploaded in the latest patch with the following changes: bool layoutObjectWillChange = getStyleChangeType() == NeedsReattachStyleChange ...
3 years, 10 months ago (2017-02-22 01:07:58 UTC) #141
esprehn
https://codereview.chromium.org/2473743003/diff/600001/third_party/WebKit/Source/core/dom/Element.cpp File third_party/WebKit/Source/core/dom/Element.cpp (right): https://codereview.chromium.org/2473743003/diff/600001/third_party/WebKit/Source/core/dom/Element.cpp#newcode1906 third_party/WebKit/Source/core/dom/Element.cpp:1906: if (change != Reattach) Ah that makes sense. https://codereview.chromium.org/2473743003/diff/600001/third_party/WebKit/Source/core/dom/Element.cpp#newcode1980 ...
3 years, 10 months ago (2017-02-22 01:13:17 UTC) #142
nainar
Elliott, made the changes you asked for. https://codereview.chromium.org/2473743003/diff/620001/third_party/WebKit/Source/core/dom/Element.cpp File third_party/WebKit/Source/core/dom/Element.cpp (right): https://codereview.chromium.org/2473743003/diff/620001/third_party/WebKit/Source/core/dom/Element.cpp#newcode1906 third_party/WebKit/Source/core/dom/Element.cpp:1906: if (change ...
3 years, 10 months ago (2017-02-22 03:12:07 UTC) #145
esprehn
lgtm
3 years, 10 months ago (2017-02-22 09:22:09 UTC) #150
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/2473743003/640001
3 years, 10 months ago (2017-02-22 09:24:06 UTC) #152
commit-bot: I haz the power
Committed patchset #33 (id:640001) as https://chromium.googlesource.com/chromium/src/+/8b6c7b7ce5a36d8eda49dd6e60c5e7c8f5c7277f
3 years, 10 months ago (2017-02-22 10:53:30 UTC) #155
nainar
On 2017/02/22 at 10:53:30, commit-bot wrote: > Committed patchset #33 (id:640001) as https://chromium.googlesource.com/chromium/src/+/8b6c7b7ce5a36d8eda49dd6e60c5e7c8f5c7277f YEAH! \o/ ...
3 years, 10 months ago (2017-02-22 10:58:48 UTC) #156
nainar
A revert of this CL (patchset #33 id:640001) has been created in https://codereview.chromium.org/2729453002/ by nainar@chromium.org. ...
3 years, 9 months ago (2017-03-01 03:39:57 UTC) #158
nainar
Rune, Changed needsAttach() to only check if getStyleChangeType() == NeedsReattachStyleChange as all callsites to this ...
3 years, 9 months ago (2017-03-02 01:21:23 UTC) #162
rune
lgtm
3 years, 9 months ago (2017-03-02 10:37:23 UTC) #169
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/2473743003/680001
3 years, 9 months ago (2017-03-05 23:57:45 UTC) #172
commit-bot: I haz the power
Failed to apply patch for third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp: While running git apply --index -p1; error: patch failed: ...
3 years, 9 months ago (2017-03-06 01:25:42 UTC) #174
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/2473743003/700001
3 years, 9 months ago (2017-03-06 01:47:36 UTC) #179
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/377231)
3 years, 9 months ago (2017-03-06 03:19:57 UTC) #181
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/2473743003/700001
3 years, 9 months ago (2017-03-06 03:57:50 UTC) #183
commit-bot: I haz the power
3 years, 9 months ago (2017-03-06 04:06:37 UTC) #186
Message was sent while issue was closed.
Committed patchset #36 (id:700001) as
https://chromium.googlesource.com/chromium/src/+/f4c976fc1f4578b0c75ba304553e...

Powered by Google App Engine
This is Rietveld 408576698