[Webkit-unassigned] [Bug 31680] WebCore::Document::updateLayoutIgnorePendingStylesheets NULL pointer

[bugzilla-daemon at webkit.org]

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





--- Comment #16 from MORITA Hajime <morrita at google.com>  2010-03-30 04:20:26 PST ---
ap, thank you for reviewing again! I updated the patch.

> We don't usually add null checks just in case, especially in performance
> sensitive code. These can confuse people looking at the code to think that this
> can legitimately happen.
> 
> You could add an ASSERT - it would be useless for catching errors, because we
> crash soon enough anyway, but it would serve as documentation that nodes are
> supposed to have a non-null document here.
Agreed. I replaced null check with ASSERT()

> 
> I was going to suggest raising an exception instead (probably
> INVALID_ACCESS_ERR). I'm not sure if that's a good idea.
Fixed.

> But it made me think
> about nodes from different documents. Is there a check somewhere that we don't
> set the selection base and extent to nodes from another document?
For setBaseAndExtent(), there are 2 patterns of foreign document.
And both somewhat work.

Pattern 1: owner document of selection and of argument node differ
document of argument node is used. so selection of argument node's document
change.

Pattern 2: owner document of baseNode  and of extentNode differ:
extentNode is replaced by baseNode.

> +It is OK not to crash.
> 
> A better way to say this would be "PASSED if didn't crash".
Fixed.

This fix also cares Bug 36800 because both has same root cause and 
I think it is better to fix them at single commit. for change cleanness.

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