[Webkit-unassigned] [Bug 140261] Null ptr crash in WebCore::LogicalSelectionOffsetCaches::ContainingBlockInfo::setBlock

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Apr 28 08:58:31 PDT 2015


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
 Attachment #251841|review?                     |review-
              Flags|                            |

--- Comment #28 from Darin Adler <darin at apple.com> ---
Comment on attachment 251841
  --> https://bugs.webkit.org/attachment.cgi?id=251841

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

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.

> Source/WebCore/rendering/RenderView.h:306
> +    ALWAYS_INLINE bool isValidObjectForNewSelection(const SelectionSubtreeRoot& root, const RenderObject* obj) const

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.

You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150428/9335f83c/attachment.html>

More information about the webkit-unassigned mailing list