[Webkit-unassigned] [Bug 137918] Use references instead of pointers in EditingStyle

[bugzilla-daemon at webkit.org]

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

--- Comment #19 from Csaba Osztrogonác <ossy at webkit.org> ---
(In reply to https://bugs.webkit.org/show_bug.cgi?id=133796#c20)
> >> Source/WebCore/editing/EditingStyle.cpp:1557
> >> +    ASSERT(baseStyle);
> > 
> > If you're going to ASSERT this, why not make it a reference?
> Good idea for future refactoring. Both arguments to this function template
> should be references.

The original idea was to refactor the code to use references instead
of pointers for the arguments of extractPropertiesNotIn(...):
https://trac.webkit.org/browser/trunk/Source/WebCore/editing/EditingStyle.cpp?rev=175067#L1553

But I think this patch tries to do too much thing at once. I tried to review
it in details and find how to address the issues that Darin noticed, but it
is to find the proper fix or answer quickly.

So I suggest splitting up this patch to smaller pieces. The original idea
can be a good starting point. Converting the second argument (baseStyle)
from pointer to reference seems to be easy and safe at all. 

But I'm not sure if it is possible to do the same with the first argument
(styleWithRedundantProperties), because extractPropertiesNotIn(...) is called
from getPropertiesNotIn(...) directly and getPropertiesNotIn(...) is called
from many places - most of them seems to be safe (null pointer checked), except
one call: https://trac.webkit.org/browser/trunk/Source/WebCore/editing/EditingStyle.cpp?rev=175067#L651

Maybe it can be fixed with a null pointer check, but I have no idea what
return value should triStateOfStyle() have if m_mutableStyle is null.
Darin, Ryosuke? Maybe you have any idea for this issue.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141117/5a3cd298/attachment-0002.html>