Bug 135233

Summary: Member name collision in RenderView and SelectionSubtreeRoot
Product: WebKit Reporter: Radu Stavila <stavila>
Component: CSSAssignee: Radu Stavila <stavila>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, hyatt, jfernandez, kondapallykalyan, mihnea, rego, rniwa
Priority: P2 Keywords: AdobeTracked
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch for renaming selection members - NOT FOR LANDING
none
Patch
none
Patch integrating reviewer feedback none

Description Radu Stavila 2014-07-24 02:39:29 PDT
SelectionSubtreeRoot members:

    RenderObject* m_selectionStart;
    int m_selectionStartPos;
    RenderObject* m_selectionEnd;
    int m_selectionEndPos;


RenderView members:

    RenderObject* m_selectionStart;
    RenderObject* m_selectionEnd;
    int m_selectionStartPos;
    int m_selectionEndPos;


Because RenderView inherits SelectionSubtreeRoot, this presents a problem. Furthermore, the getter/setter methods also have identical names (selectionStart(), setSelectionStart(...) etc). 

Example: in RenderView.cpp, setting the start/end selection members to some values. Then calling selectionStartEndPositions(...) which retrieves those values. Because this method is inside SelectionSubtreeRoot, it will return the version of the selection members from SelectionSubtreeRoot, not the ones you would normally expect it to. This has great potential for creating confusion and hard-to-track issues.
Comment 1 Radu Stavila 2014-07-24 03:04:21 PDT
Created attachment 235420 [details]
Patch for renaming selection members - NOT FOR LANDING

The attached patch renames the members inside RenderView but causes fast/dom/HTMLObjectElement/beforeload-set-text-crash.xhtml to crash.
Comment 2 Radu Stavila 2014-07-28 06:07:03 PDT
Update: the crash was not caused by the rename. After the initial patch lands (https://bugs.webkit.org/show_bug.cgi?id=134303) I will try to re-land this.
Comment 3 Radu Stavila 2014-07-30 04:11:46 PDT
Created attachment 235740 [details]
Patch
Comment 4 Mihnea Ovidenie 2014-07-30 04:43:34 PDT
Comment on attachment 235740 [details]
Patch

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

> Source/WebCore/rendering/RenderView.h:92
> +    RenderObject* selectionUnsplitStart() const { return m_unsplitSelectionStart; }

Any reason why the function is named selectionUnsplitStart and the member is m_unsplitSelectionStart?
Comment 5 Radu Stavila 2014-07-30 06:38:27 PDT
Created attachment 235743 [details]
Patch integrating reviewer feedback
Comment 6 Mihnea Ovidenie 2014-07-30 07:02:23 PDT
Comment on attachment 235743 [details]
Patch integrating reviewer feedback

r=me
Comment 7 WebKit Commit Bot 2014-07-30 09:23:21 PDT
Comment on attachment 235743 [details]
Patch integrating reviewer feedback

Clearing flags on attachment: 235743

Committed r171797: <http://trac.webkit.org/changeset/171797>
Comment 8 WebKit Commit Bot 2014-07-30 09:23:25 PDT
All reviewed patches have been landed.  Closing bug.