[Webkit-unassigned] [Bug 18282] WebKit crashes with deeply nested divs

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=18282


Maciej Stachowiak <mjs at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #51008|review?                     |review-
               Flag|                            |




--- Comment #30 from Maciej Stachowiak <mjs at apple.com>  2010-03-31 01:09:18 PST ---
(From update of attachment 51008)
1) I don't think this fix should be optional. Ports should not have to choose
between a crash fix and a possible performance hit. We should have one code
path that both has adequate performance and does not crash.

2) A number of the DOM methods now do if (atMaxTreeDepth()) return 0;. It seems
to me that they should set the exception code too (I am not sure offhand which
DOM exception code is most appropriate). I suggest trying some DOM benchmarks,
such as <http://dromaeo.com/?dom> and
<http://nontroppo.org/timer/Hixie_DOM.html>.

3) Because node insertion has become O(N), it's possible that building up a
deep tree using DOM APIs alone will trigger O(N^2) behavior. Almost any time
we've had N^2 algorithms, we have run into some site that hangs as a result.

4) MAX_DOM_TREE_DEPTH should be defined as a const size_t rather than a
preprocessor macro.

5) 4096 seems like a fairly low limit. Is that the best we can do?

6) cMaxBlockDepth shouldn't be defined as MAX_DOM_TREE_DEPTH; the fact that
it's the same constant is a coincidence, not fundamental.

7) In the parser, it looks to me like you repurposed m_blocksInStack to be a
count of total nesting level, rather than block nesting level. That seems
dubious to me for a couple of reasons: 
   (a) The name is now wrong. If this change is otherwise right, the variable
should be renamed (or just use the dom tree depth constant directly).
   (b) Checks relating to block depth just go off of tag depth, but that seems
wrong to me; I suspect this breaks the intent of the block checks. For example,
non-blocks could now cause blocks to be popped.
   (c) I think it would be good to test a case of blocks and a case of mixed
blocks and inlines. It seems like your test case only covers spans, which are
an inline element

8) Like Eric, I would like to see testing of page load speed.

r- to address above comments.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.