Bug 130173 – [Texmap] Remove duplicated code in TextureMapperLayer

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Jae Hyun Park]

Patch to follow.

[Jae Hyun Park]

Created attachment 226568 [details]
Patch

[Martin Robinson]

Comment on attachment 226568 [details]
Patch

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

> Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:496
> -    if (m_parent) {
> -        unsigned i;
> -        for (i = 0; i < m_parent->m_children.size(); i++) {
> -            if (this == m_parent->m_children[i]) {
> -                m_parent->m_children.remove(i);
> -                break;
> -            }
> -        }
> +    if (m_parent)
> +        m_parent->m_children.remove(m_parent->m_children.find(this));

The removed code is slightly different in that it gracefully handles the situation where m_parent->m_children doesn't contain "this." Do you know if that's important?

[Jae Hyun Park]

(In reply to comment #2)
> (From update of attachment 226568 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=226568&action=review
> 
> > Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:496
> > -    if (m_parent) {
> > -        unsigned i;
> > -        for (i = 0; i < m_parent->m_children.size(); i++) {
> > -            if (this == m_parent->m_children[i]) {
> > -                m_parent->m_children.remove(i);
> > -                break;
> > -            }
> > -        }
> > +    if (m_parent)
> > +        m_parent->m_children.remove(m_parent->m_children.find(this));
> 
> The removed code is slightly different in that it gracefully handles the situation where m_parent->m_children doesn't contain "this." Do you know if that's important?

I can't think of a case where m_parent exists but m_parent->children doesn't contain "this". Do you happen to know how that might happen?

[Martin Robinson]

(In reply to comment #3)

> I can't think of a case where m_parent exists but m_parent->children doesn't contain "this". Do you happen to know how that might happen?

I'm not sure.

[Sergio Villar Senin]

(In reply to comment #4)
> (In reply to comment #3)
> 
> > I can't think of a case where m_parent exists but m_parent->children doesn't contain "this". Do you happen to know how that might happen?
> 
> I'm not sure.

Then we could try with an ASSERT() and check if it's hit.

[Jae Hyun Park]

Created attachment 226634 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 226634 [details]
Patch

Clearing flags on attachment: 226634

Committed r165665: <http://trac.webkit.org/changeset/165665>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.