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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 732203004: Clean up child checks in ContainerNode. (Closed)

Created:
6 years, 1 month ago by esprehn
Modified:
6 years, 1 month ago
Reviewers:
ojan
CC:
ojan, abarth-chromium, mojo-reviews_chromium.org
Base URL:
git@github.com:domokit/mojo.git@master
Project:
mojo
Visibility:
Public.

Description

Clean up child checks in ContainerNode. We can simplify the checks given that there's fewer node types now. This does make the error messages from Range a little worse, but it's weird that Range is doing its own error checking anyway. I also took this as an opportunity to add a bunch of DOM tests. R=ojan@chromium.org Committed: https://chromium.googlesource.com/external/mojo/+/72e0dd3184b26b1b46f0e80cde46a3ca2d1ff2d6

Patch Set 1 #

Patch Set 2 : dry #

Total comments: 5

Patch Set 3 : Add back secondary hierarchy checks. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+381 lines, -204 lines) Patch
M sky/engine/core/dom/ContainerNode.h View 1 2 1 chunk +2 lines, -3 lines 0 comments Download
M sky/engine/core/dom/ContainerNode.cpp View 1 2 8 chunks +60 lines, -88 lines 0 comments Download
M sky/engine/core/dom/Document.h View 2 chunks +0 lines, -2 lines 0 comments Download
M sky/engine/core/dom/Document.cpp View 1 chunk +0 lines, -72 lines 0 comments Download
M sky/engine/core/dom/DocumentFragment.h View 1 chunk +0 lines, -1 line 0 comments Download
M sky/engine/core/dom/DocumentFragment.cpp View 1 chunk +0 lines, -11 lines 0 comments Download
M sky/engine/core/dom/Element.h View 1 chunk +0 lines, -1 line 0 comments Download
M sky/engine/core/dom/Element.cpp View 1 2 1 chunk +0 lines, -12 lines 0 comments Download
M sky/engine/core/dom/Node.h View 1 chunk +0 lines, -1 line 0 comments Download
M sky/engine/core/dom/Range.cpp View 2 chunks +0 lines, -13 lines 0 comments Download
A sky/tests/dom/appendChild.sky View 1 1 chunk +76 lines, -0 lines 0 comments Download
A sky/tests/dom/appendChild-expected.txt View 1 1 chunk +9 lines, -0 lines 0 comments Download
A sky/tests/dom/document-child-mutations.sky View 1 1 chunk +103 lines, -0 lines 0 comments Download
A sky/tests/dom/document-child-mutations-expected.txt View 1 chunk +11 lines, -0 lines 0 comments Download
A sky/tests/dom/replaceChild.sky View 1 1 chunk +90 lines, -0 lines 0 comments Download
A sky/tests/dom/replaceChild-expected.txt View 1 1 chunk +10 lines, -0 lines 0 comments Download
A sky/tests/resources/dom-utils.sky View 1 1 chunk +20 lines, -0 lines 0 comments Download

Messages

Total messages: 6 (1 generated)
ojan
Yay tests! https://codereview.chromium.org/732203004/diff/20001/sky/engine/core/dom/ContainerNode.cpp File sky/engine/core/dom/ContainerNode.cpp (left): https://codereview.chromium.org/732203004/diff/20001/sky/engine/core/dom/ContainerNode.cpp#oldcode182 sky/engine/core/dom/ContainerNode.cpp:182: // We need this extra check because ...
6 years, 1 month ago (2014-11-18 02:46:57 UTC) #2
esprehn
https://codereview.chromium.org/732203004/diff/20001/sky/engine/core/dom/ContainerNode.cpp File sky/engine/core/dom/ContainerNode.cpp (left): https://codereview.chromium.org/732203004/diff/20001/sky/engine/core/dom/ContainerNode.cpp#oldcode182 sky/engine/core/dom/ContainerNode.cpp:182: // We need this extra check because collectChildrenAndRemoveFromOldParent() can ...
6 years, 1 month ago (2014-11-18 03:01:40 UTC) #3
esprehn
want to look again?
6 years, 1 month ago (2014-11-18 23:13:55 UTC) #4
ojan
lgtm
6 years, 1 month ago (2014-11-18 23:51:40 UTC) #5
esprehn
6 years, 1 month ago (2014-11-19 00:49:52 UTC) #6
Message was sent while issue was closed. 
Committed patchset #3 (id:40001) manually as
72e0dd3184b26b1b46f0e80cde46a3ca2d1ff2d6.

Powered by Google App Engine
This is Rietveld 408576698