Bug 124837 - Rename InlineIterator::m_obj and make it private
Summary: Rename InlineIterator::m_obj and make it private
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gyuyoung Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-25 04:32 PST by Gyuyoung Kim
Modified: 2013-11-28 15:22 PST (History)
5 users (show)

See Also:


Attachments
Patch (39.31 KB, patch)
2013-11-25 04:33 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (44.51 KB, patch)
2013-11-26 22:35 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (44.51 KB, patch)
2013-11-28 14:28 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2013-11-25 04:32:07 PST
InlineIterator has been exported m_obj as public though there is a getter function. Fixed it. Additionally, setObject() is added as well.
Comment 1 Gyuyoung Kim 2013-11-25 04:33:52 PST
Created attachment 217790 [details]
Patch
Comment 2 Andreas Kling 2013-11-26 15:01:20 PST
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"
Comment 3 Gyuyoung Kim 2013-11-26 22:35:37 PST
Created attachment 217926 [details]
Patch
Comment 4 Gyuyoung Kim 2013-11-26 22:36:54 PST
(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.
Comment 5 Gyuyoung Kim 2013-11-27 16:54:27 PST
Kling, could you take a look again ?
Comment 6 Antti Koivisto 2013-11-28 14:02:21 PST
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.
Comment 7 Gyuyoung Kim 2013-11-28 14:27:48 PST
(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.
Comment 8 Gyuyoung Kim 2013-11-28 14:28:47 PST
Created attachment 218025 [details]
Patch
Comment 9 WebKit Commit Bot 2013-11-28 15:22:41 PST
Comment on attachment 218025 [details]
Patch

Clearing flags on attachment: 218025

Committed r159860: <http://trac.webkit.org/changeset/159860>
Comment 10 WebKit Commit Bot 2013-11-28 15:22:44 PST
All reviewed patches have been landed.  Closing bug.