[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 #21 from Tibor Mészáros <tmeszaros.u-szeged at partner.samsung.com> ---
Comment on attachment 241580
  --> https://bugs.webkit.org/attachment.cgi?id=241580
Patch v5

View in context: https://bugs.webkit.org/attachment.cgi?id=241580&action=review

>> Source/WebCore/editing/EditingStyle.cpp:651
>> +    RefPtr<MutableStyleProperties> difference = getPropertiesNotIn(*m_mutableStyle, styleToCompare);
> 
> What guarantees that m_mutableStyle is non-null here?

Good point, I added a nullptr check to guarantee that m_mutableStyle isn't nullptr.
This case was previously handled by if(difference->isEmpty()) check below.

>> Source/WebCore/editing/EditingStyle.cpp:1412
>> +        extractTextStyles(document, *mutableStyle, computedStyle.useFixedFontDefaultSize());
> 
> What guarantees that mutableStyle is non-null here? Can getPropertiesNotIn ever return null?

mutableStyle is the return value of getPropertiesNotIn(*style->style(), computedStyle). getPropertiesNotIn() simply pass through the return value of extractPropertiesNotIn(), which can't be nullptr.

>> Source/WebCore/editing/EditingStyle.cpp:1548
>> +    return fontWeight ? fontWeightIsBold(*fontWeight) : false;
> 
> Forgot to mention this last time. I think this is even nicer with &&:
> 
>     return fontWeight && fontWeightIsBold(*fontWeight);

Thanks, I fixed it too.

>>> Source/WebCore/editing/EditingStyle.cpp:1559
>>> +    diffTextDecorations(*mutableStyle, CSSPropertyTextDecoration, baseTextDecorationsInEffect.get());
>> 
>> What guarantees mutableStyle is not null? Can EditingStyle::style ever return null?
> 
> Theoretically (without knowing the exact logic) it can, it simply returns m_mutableStyle.get().
> 
> But assuming if it can return nullptr, it isn't safe before this change, because diffTextDecorations()
> uses it without nullptr check: RefPtr<CSSValue> textDecoration = style->getPropertyCSSValue(propertID);
> ( http://trac.webkit.org/browser/trunk/Source/WebCore/editing/EditingStyle.cpp#L1502 )

Result is copied from the non null styleWithRedundantProperties, which guarantees that result->style() alias mutableStyle is non nullptr.

-- 
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/20141125/7aafb934/attachment-0002.html>