[Webkit-unassigned] [Bug 57717] Convert use of raw pointers to RefPtr in using Cairo

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=57717


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #89407|review?                     |review-
               Flag|                            |




--- Comment #23 from Martin Robinson <mrobinson at webkit.org>  2011-04-13 11:58:03 PST ---
(From update of attachment 89407)
View in context: https://bugs.webkit.org/attachment.cgi?id=89407&action=review

I apologize that this change has balooned from its original intent. I understand if you wish to separate out the parts of the change into separate bugs.

> Source/WebCore/platform/graphics/cairo/CairoPath.cpp:18
> +/*
> +    Copyright (C) 2011 Collabora Ltd.
> +
> +    This library is free software; you can redistribute it and/or
> +    modify it under the terms of the GNU Library General Public
> +    License as published by the Free Software Foundation; either
> +    version 2 of the License, or (at your option) any later version.
> +
> +    This library is distributed in the hope that it will be useful,
> +    but WITHOUT ANY WARRANTY; without even the implied warranty of
> +    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +    Library General Public License for more details.
> +
> +    You should have received a copy of the GNU Library General Public License
> +    aint with this library; see the file COPYING.LIB.  If not, write to
> +    the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
> +    Boston, MA 02110-1301, USA.
> +*/

I think it's good that you are moving this stuff into an actual implementation file since it's becoming heftier. Please, please take this opportunity to rename this to PlatformPathCairo though. Having CairoPath.{cpp,h} and PathCairo.cpp is incredibly confusing!

Please format this license blocks with asterisks down the left side to match the other files in this directory.

> Source/WebCore/platform/graphics/cairo/CairoPath.cpp:25
> +cairo_surface_t* CairoPath::m_pathSurface = cairo_image_surface_create(CAIRO_FORMAT_A8, 1, 1);

Now that this is in a CPP file you can dispense with the static class member altogether and just do:

static cairo_surface_t* getPathSurface()
{
    static pathSurface =  cairo_image_surface_create(CAIRO_FORMAT_A8, 1, 1);
    return pathSurface;
}

> Source/WebCore/platform/graphics/cairo/CairoPath.h:35
>      CairoPath()
>      {
> -        static cairo_surface_t* pathSurface = cairo_image_surface_create(CAIRO_FORMAT_A8, 1, 1);
> -        m_cr = cairo_create(pathSurface);
> +        m_cr = adoptRef(cairo_create(m_pathSurface));
>      }

Please move the implementation of the constructor into the CPP file so you can remove the cairo.h include above. The assignment of m_cr should be an initializer.

CairoPath()
    : m_cr(adoptRef(cairo_create(m_pathSurface))
{
}

> Source/WebCore/platform/graphics/cairo/CairoPath.h:40
>      ~CairoPath()
>      {
> -        cairo_destroy(m_cr);
>      }
>  

This can be one line now and perhaps even omitted.

> Source/WebCore/platform/graphics/cairo/CairoPath.h:45
> +    static cairo_surface_t* m_pathSurface;

You can remove this. See above.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.