Bug 130173 - [Texmap] Remove duplicated code in TextureMapperLayer
Summary: [Texmap] Remove duplicated code in TextureMapperLayer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jae Hyun Park
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-12 19:45 PDT by Jae Hyun Park
Modified: 2014-03-14 17:36 PDT (History)
8 users (show)

See Also:


Attachments
Patch (2.26 KB, patch)
2014-03-12 19:47 PDT, Jae Hyun Park
no flags Details | Formatted Diff | Diff
Patch (2.31 KB, patch)
2014-03-13 18:02 PDT, Jae Hyun Park
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jae Hyun Park 2014-03-12 19:45:24 PDT
Patch to follow.
Comment 1 Jae Hyun Park 2014-03-12 19:47:04 PDT
Created attachment 226568 [details]
Patch
Comment 2 Martin Robinson 2014-03-12 19:51:09 PDT
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?
Comment 3 Jae Hyun Park 2014-03-12 20:07:30 PDT
(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?
Comment 4 Martin Robinson 2014-03-12 20:17:53 PDT
(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.
Comment 5 Sergio Villar Senin 2014-03-13 02:12:03 PDT
(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.
Comment 6 Jae Hyun Park 2014-03-13 18:02:27 PDT
Created attachment 226634 [details]
Patch
Comment 7 WebKit Commit Bot 2014-03-14 17:36:17 PDT
Comment on attachment 226634 [details]
Patch

Clearing flags on attachment: 226634

Committed r165665: <http://trac.webkit.org/changeset/165665>
Comment 8 WebKit Commit Bot 2014-03-14 17:36:21 PDT
All reviewed patches have been landed.  Closing bug.