RESOLVED CONFIGURATION CHANGED 37688
Remove the optional shouldLazyAttach parameter from appendChild and friends
https://bugs.webkit.org/show_bug.cgi?id=37688
Summary Remove the optional shouldLazyAttach parameter from appendChild and friends
James Robinson
Reported 2010-04-15 17:57:14 PDT
Remove the optional shouldLazyAttach parameter from appendChild and friends
Attachments
Patch (26.87 KB, patch)
2010-04-15 18:17 PDT, James Robinson
no flags
Patch (29.35 KB, patch)
2010-05-05 16:01 PDT, James Robinson
no flags
James Robinson
Comment 1 2010-04-15 18:17:30 PDT
James Robinson
Comment 2 2010-04-15 18:20:10 PDT
+cc people who have added mucked with this code or expressed interest. I am planning to try to make lazy attachment the only form of attachment, with special treatment for forms/plugins/frames. Even if that doesn't pan out this is still a good step forward - less parameters + less branching = less complexity + more gooder.
Dimitri Glazkov (Google)
Comment 3 2010-04-16 09:16:04 PDT
Comment on attachment 53498 [details] Patch > diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog > index c5e227e..24e1ee6 100644 > --- a/LayoutTests/ChangeLog > +++ b/LayoutTests/ChangeLog > @@ -1,3 +1,24 @@ > +2010-04-15 James Robinson <jamesr@chromium.org> > + > + Reviewed by NOBODY (OOPS!). > + > + Update baselines affected by lazier attachment > + https://bugs.webkit.org/show_bug.cgi?id=37688 > + > + There are no user-visible changes, just a slightly different set of > + 0-sized render objects in the render tree due to style recalcs/layouts > + happening at slightly different times. > + > + * platform/mac/editing/inserting/break-blockquote-after-delete-expected.txt: > + * platform/mac/fast/forms/basic-buttons-expected.txt: > + * platform/mac/fast/forms/formmove3-expected.txt: > + * platform/mac/fast/forms/preserveFormDuringResidualStyle-expected.txt: > + * platform/mac/fast/invalid/003-expected.txt: > + * platform/mac/fast/invalid/table-residual-style-crash-expected.txt: > + * platform/mac/fast/table/014-expected.txt: > + * platform/mac/tables/mozilla/bugs/bug647-expected.txt: > + * platform/mac/tables/mozilla/other/wa_table_tr_align-expected.txt: You should go ahead and add results for Chromium as well -- or at least update test_expectations.txt. > - > - // Should assert isBlockFlow || isInlineFlow when deletion improves. See 4244964. > - ASSERT(container->renderer()); Can you just remove these asserts? What were they for in the first place?
James Robinson
Comment 4 2010-05-05 16:01:41 PDT
James Robinson
Comment 5 2010-05-05 16:06:04 PDT
I've marked the affected Chromium tests as TEXT PASS in chromium's text_expectations.txt. I will rebase as needed after landing. It is safe to remove those ASSERT()s. They date from before there was a Node::lazyAttach() and when all nodes in the DOM would get a renderer on creation. The issue cited in the comment doesn't seem to be an issue any more.
Eric Seidel (no email)
Comment 6 2010-05-08 23:31:25 PDT
I don't understand the layout test results updates. Seems this would be easier to understand if we had the minimal code change to cause said updates. Like just flipping the default from false to true? Or by changing one or two callers? Do we really understand the test output changes and are they expected?
James Robinson
Comment 7 2010-05-11 13:30:28 PDT
The layout test results changes are all noise due to leading/trailing whitespace in blocks. I'll see if I can somehow isolate those effects from the meat of this patch.
James Robinson
Comment 8 2010-05-12 18:18:17 PDT
Comment on attachment 55166 [details] Patch Clearing r?, needs a bit of work.
Ahmad Saleem
Comment 9 2022-08-06 06:31:07 PDT
Ryosuke Niwa
Comment 10 2022-08-06 14:24:52 PDT
We wrote this code so this bug is no longer relevant.
Note You need to log in before you can comment on or make changes to this bug.