[webkit-reviews] review granted: [Bug 33668] WebCore::InsertListCommand::modifyRange infinite loop (100% CPU usage) : [Attachment 63036] new approach

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 33668: WebCore::InsertListCommand::modifyRange infinite loop (100% CPU
usage)
https://bugs.webkit.org/show_bug.cgi?id=33668

Attachment 63036: new approach
https://bugs.webkit.org/attachment.cgi?id=63036&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +	       ExceptionCode ec = 0;
> +	       if (rangeStartIsInList && newList)
> +		   currentSelection->setStart(newList->parentNode(),
newList->nodeIndex(), ec);
> +	       if (rangeEndIsInList && newList)
> +		   currentSelection->setEnd(newList->parentNode(),
newList->nodeIndex()+1, ec);

There should be spaces around the "+" operator here where we do "+ 1".

Since nodeIndex is potentially a O(n^2) operation, I suggest restructuring the
code so we compute it only once. On the other hand, if you don't plan to do
that optimization, it would be better to use the Range::setStartAfter and
Range::setEndBefore functions instead of calling nodeIndex directly here.

If you're going to ignore the exception code, there is no need to initialize it
to 0.

Are all the code changes here covered by this single test?