Bug 124837 – Rename InlineIterator::m_obj and make it private

• Component: New Bugs
• Status: RESOLVED FIXED

[Gyuyoung Kim]

InlineIterator has been exported m_obj as public though there is a getter function. Fixed it. Additionally, setObject() is added as well.

[Gyuyoung Kim]

Created attachment 217790 [details]
Patch

[Andreas Kling]

Comment on attachment 217790 [details]
Patch

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

> Source/WebCore/rendering/InlineIterator.h:70
>      RenderObject* object() const { return m_obj; }
> +    void setObject(RenderObject* object) { m_obj = object; }

I would call these renderer() and setRenderer().

> Source/WebCore/rendering/InlineIterator.h:96
> +    RenderObject* m_obj;

And this would be "m_renderer"

[Gyuyoung Kim]

Created attachment 217926 [details]
Patch

[Gyuyoung Kim]

(In reply to comment #2)
> (From update of attachment 217790 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=217790&action=review
> 
> > Source/WebCore/rendering/InlineIterator.h:70
> >      RenderObject* object() const { return m_obj; }
> > +    void setObject(RenderObject* object) { m_obj = object; }
> 
> I would call these renderer() and setRenderer().
> 
> > Source/WebCore/rendering/InlineIterator.h:96
> > +    RenderObject* m_obj;
> 
> And this would be "m_renderer"

Seems good to me as well. Changed.

[Gyuyoung Kim]

Kling, could you take a look again ?

[Antti Koivisto]

Comment on attachment 217926 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Set m_obj member variable as private

The title is not informative. Maybe something like "Rename InlineIterator::m_obj and make it private"

> Source/WebCore/rendering/InlineIterator.h:69
> +    RenderObject* renderer() const { return m_renderer; }

It would be good to make this return a reference and ASSERT(m_renderer). Any valid null checks in client would be replaced with atEnd() tests. For future patch though.

[Gyuyoung Kim]

(In reply to comment #6)
 
> > Source/WebCore/rendering/InlineIterator.h:69
> > +    RenderObject* renderer() const { return m_renderer; }
> 
> It would be good to make this return a reference and ASSERT(m_renderer). Any valid null checks in client would be replaced with atEnd() tests. For future patch though.

Antti, I'd like to support this comment on new bug. I will file a bug for this. Thx.

[Gyuyoung Kim]

Created attachment 218025 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 218025 [details]
Patch

Clearing flags on attachment: 218025

Committed r159860: <http://trac.webkit.org/changeset/159860>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.