<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:darin@apple.com" title="Darin Adler <darin@apple.com>"> <span class="fn">Darin Adler</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - Null ptr crash in WebCore::LogicalSelectionOffsetCaches::ContainingBlockInfo::setBlock"
href="https://bugs.webkit.org/show_bug.cgi?id=140261">bug 140261</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #251841 Flags</td>
<td>review?
</td>
<td>review-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Null ptr crash in WebCore::LogicalSelectionOffsetCaches::ContainingBlockInfo::setBlock"
href="https://bugs.webkit.org/show_bug.cgi?id=140261#c28">Comment # 28</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Null ptr crash in WebCore::LogicalSelectionOffsetCaches::ContainingBlockInfo::setBlock"
href="https://bugs.webkit.org/show_bug.cgi?id=140261">bug 140261</a>
from <span class="vcard"><a class="email" href="mailto:darin@apple.com" title="Darin Adler <darin@apple.com>"> <span class="fn">Darin Adler</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=251841&action=diff" name="attach_251841" title="patch">attachment 251841</a> <a href="attachment.cgi?id=251841&action=edit" title="patch">[details]</a></span>
patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=251841&action=review">https://bugs.webkit.org/attachment.cgi?id=251841&action=review</a>
The bug fix looks OK. But refactoring this code into a separate function does not require touching the RenderView.h header at all! review- because of that.
<span class="quote">> Source/WebCore/rendering/RenderView.h:306
> + ALWAYS_INLINE bool isValidObjectForNewSelection(const SelectionSubtreeRoot& root, const RenderObject* obj) const</span >
Why ALWAYS_INLINE? That seems like overkill.
Please don’t use the abbreviation “obj” in new code. I suggest “renderer” or “object” as a possible name. Please use a reference for the object rather than a pointer since it can’t be null.
Please put the body of this private function in the .cpp file rather than in the header. You can put inline functions in the .cpp file as long as they aren’t used in any other translation unit, and that will be true for a private function.
This function does’t seem to use any data members of RenderView so it can just be a separate function in the .cpp file. Doesn’t need to be a member at all and if it is a member should be a static member, not a const member.
static inline bool isValidObjectForNewSelection(const SelectionSubtreeRoot& root, const RenderObject& object)
{
}
Inside the .cpp file.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>