New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 641014 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 578344



Sign in to add a comment

rewrite_to_chrome_style: Difficulty with renaming |deref| -> |Deref| in WTF only (and not in Skia)

Project Member Reported by lukasza@chromium.org, Aug 25 2016

Issue description

We currently rename |deref| -> |Deref|:

    wtf/PassRefPtr.h:

        template <typename T> ALWAYS_INLINE void DerefIfNotNull(T* ptr)
        {
            if (LIKELY(ptr != 0))
                ptr->Deref();
        }

    wtf/RefCounted.h:
    
        template<typename T> class RefCounted : public RefCountedBase {
        ...
        public:
            void Deref() const
            {
                if (DerefBase())
                    delete static_cast<const T*>(this);
            }

The above looks good so far, but then we also use PassRefPtr<T> with non-WTF types:

        void GraphicsContextState::SetDrawLooper(PassRefPtr<SkDrawLooper> draw_looper)
        {
            ...
        }

Which obviously didn't get the |deref| -> |Deref| renaming:

    .../wtf/PassRefPtr.h:55:14: error: no member named 'Deref' in 'SkColorFilter'
            ptr->Deref();
            ~~~  ^
    .../wtf/PassRefPtr.h:72:35: note: in instantiation of function template specialization 'WTF::DerefIfNotNull<SkColorFilter>' requested here
        ALWAYS_INLINE ~PassRefPtr() { DerefIfNotNull(ptr_); }
                                  ^
    .../platform/graphics/GraphicsContextState.cpp:118:39: note: in instantiation of member function 'WTF::PassRefPtr<SkColorFilter>::~PassRefPtr' requested here
        fill_paint_.setColorFilter(ToSkSp(color_filter));
 
Maybe we can do something like this?

template <typename T,
  typename std::enable_if<std::is_base_of<::WTF::RefCountedBase, T>::value>::type* = nullptr>
ALWAYS_INLINE void DerefIfNotNull(T* ptr)
{
    if (LIKELY(ptr != 0))
        ptr->Deref();
}

template <typename T,
  typename std::enable_if<!std::is_base_of<::WTF::RefCountedBase, T>::value>::type* = nullptr>
ALWAYS_INLINE void DerefIfNotNull(T* ptr)
{
    if (LIKELY(ptr != 0))
        ptr->deref();
}

This looks like a terrible, desperate hack, but it works (for Deref-vs-deref at least;  I am still trying to figure out how to make it work for Ref-vs-ref).

Comment 2 by dcheng@chromium.org, Aug 25 2016

Cc: fmalita@chromium.org
I know we've been doing a lot of Skia cleanup to use smart pointers.

+fmalita, who's done a lot of this: any recommendations here? Can we just use sk_sp more for Skia stuff?

Comment 3 by danakj@chromium.org, Aug 25 2016

I was recently told to use sk_sp for skia things by junov.
Cc: bugsnash@chromium.org
+bugsnash@

There are ~241 lines matching 'RefPtr<Sk' regex under third_party/WebKit/Sources.  We could try to replace all of these with 'sk_sp<Sk', but I am slightly worried about preserving PassRefPtr semantics - maybe we should wait until PassRefPtr is deleted (tracked in  issue 494719  owned by bugsnash@).

Comment 5 by danakj@chromium.org, Aug 26 2016

> but I am slightly worried about preserving PassRefPtr semantics

What worries you in particular? It should be fine to just replace all PassRefPtrs with sk_sp's but you'll need to add some std::move()s (and maybe remove some redundant local vars that exist just to convert PassRefPtr->RefPtr).
I am worried about missing some places where std::move should be added.  I was hoping that bugsnash@ might have some kind of clever plan to insert these std::move calls.

Comment 7 by danakj@chromium.org, Aug 26 2016

> I am worried about missing some places where std::move should be added.

Ah. FWIW we converted from skia::RefPtr to sk_sp's in cc/ and ui/gfx/, and RefPtr code was written before std::move(). We just did a lot of manual review in those cases to see we used move() where appropriate.
Indeed I DO have a clever plan to insert std::move calls, see  issue 640449  which I've updated with a little more detail. 
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 2 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/69969af74bfff920f5fdcc2aa547ea4af2d477d2

commit 69969af74bfff920f5fdcc2aa547ea4af2d477d2
Author: lukasza <lukasza@chromium.org>
Date: Fri Sep 02 19:50:36 2016

Change (Pass)RefPtr<SkXxx> into sk_sp<SkXxx>.

This CL is motivated by 2 things:
1. General desire to use sk_sp<...> instead of RefPtr<...>
   (i.e. as given in a comment next to the fromSkSp/toSkSp functions)
2. Desire to allow rename_to_chrome_style tool to rename
   WTF::RefCountedBase::ref to Ref (while not renaming |ref| to |Ref|
   in Skia).

This CL contains the following broadly applied changes:
- s/\(Pass\|\)RefPtr<Sk/sk_sp<Sk/g
- Repeat the above for |const Sk| (i.e. const-qualified template parameter)
- Repeat the above for |WTF::RefPtr<Sk| (i.e. explicit namespace qualifier)
- Add #include "third_party/skia/include/core/SkRefCnt.h" if appropriate
- Replace calls to RefPtr<T>::clear() with calls to sk_sp<T>::reset()
- Replace calls to PassRefPtr<T>::release() with calls to std::move(x)
- Replace calls to adoptRef with calls to sk_sp<T>(T*) constructor
  or calls to sk_sp<T>::reset(T*) - both will adopt a ref-count
- Remove (now unnecessary) fromSkSp and toSkSp calls
- Add more std::move calls during a self review

This CL contains the following one-off changes:
- Tweak platform/CrossThreadCopier.h to mark sk_sp<SkRefCnt> as thread-safe
- Remove fromSkSp and toSkSp definitions from
  platform/graphics/skia/SkiaUtils.h

BUG= 641014 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2290903002
Cr-Commit-Position: refs/heads/master@{#416323}

[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/css/CSSFontFaceSourceTest.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/frame/ImageBitmap.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/frame/ImageBitmapTest.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/frame/LocalFrame.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/html/canvas/CanvasDrawListener.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/html/canvas/CanvasDrawListener.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/layout/ImageQualityControllerTest.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceClipper.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceClipper.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceMasker.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceMasker.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourcePattern.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourcePattern.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/paint/BoxReflectionUtils.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/paint/SVGClipPainter.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/paint/SVGFilterPainter.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/paint/SVGMaskPainter.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/svg/graphics/SVGImage.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/svg/graphics/SVGImageForContainer.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/svg/graphics/SVGImageForContainer.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/svg/graphics/SVGImageTest.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/core/svg/graphics/filters/SVGFEImage.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DState.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DState.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DTest.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DUsageTrackingTest.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/modules/imagebitmap/ImageBitmapRenderingContext.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/modules/mediacapturefromelement/OnRequestCanvasDrawListener.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/modules/mediacapturefromelement/OnRequestCanvasDrawListener.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/modules/mediacapturefromelement/TimedCanvasDrawListener.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/modules/mediacapturefromelement/TimedCanvasDrawListener.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/modules/offscreencanvas2d/OffscreenCanvasRenderingContext2D.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/CrossThreadCopier.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/DragImage.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/DragImage.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/DragImageTest.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/exported/WebImage.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/Font.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/FontCache.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/FontCache.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/FontCustomPlatformData.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/FontCustomPlatformData.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/FontPlatformData.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/FontPlatformData.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/TextBlob.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/WebFontDecoder.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/WebFontDecoder.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/android/FontCacheAndroid.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/linux/FontCacheLinux.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/linux/FontPlatformDataLinux.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/mac/FontPlatformDataMac.mm
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/skia/FontCacheSkia.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/win/FontCacheSkiaWin.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/win/FontFallbackWin.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/fonts/win/FontPlatformDataWin.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/AcceleratedStaticBitmapImage.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/AcceleratedStaticBitmapImage.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/BitmapImage.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/BitmapImage.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/BitmapImageTest.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/BoxReflection.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/Canvas2DImageBufferSurface.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridgeTest.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/ColorSpace.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/ColorSpace.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/DeferredImageDecoder.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/DeferredImageDecoder.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/DeferredImageDecoderTest.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/DeferredImageDecoderTestWoPlatform.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/DrawLooperBuilder.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/DrawLooperBuilder.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/GeneratedImage.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/GeneratedImage.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/GraphicsContext.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/GraphicsContext.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/GraphicsContextState.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/GraphicsContextState.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/GraphicsContextTest.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/GraphicsLayer.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/Image.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/Image.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/ImageBuffer.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/ImageBuffer.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/ImageBufferSurface.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/ImageBufferSurface.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/ImageLayerChromiumTest.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/ImagePattern.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/ImageSource.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/ImageSource.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/LoggingCanvas.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/PaintGeneratedImage.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/Pattern.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/Pattern.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/PicturePattern.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/PicturePattern.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/PictureSnapshot.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/PictureSnapshot.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/RecordingImageBufferSurface.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/RecordingImageBufferSurface.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/RecordingImageBufferSurfaceTest.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/StaticBitmapImage.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/StaticBitmapImage.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/UnacceleratedImageBufferSurface.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/UnacceleratedImageBufferSurface.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/filters/SkiaImageFilterBuilder.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/gpu/AcceleratedImageBufferSurface.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/gpu/AcceleratedImageBufferSurface.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/paint/DisplayItemListTest.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/paint/DrawingDisplayItem.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/paint/DrawingDisplayItem.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/paint/PaintController.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/paint/PaintControllerTest.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/paint/SkPictureBuilder.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/paint/SkPictureBuilder.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/skia/ImagePixelLocker.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/skia/ImagePixelLocker.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/graphics/skia/SkiaUtils.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/image-decoders/FastSharedBufferReaderTest.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/image-decoders/SegmentReader.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/image-decoders/SegmentReader.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/testing/TestPaintArtifact.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/text/TextRun.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/platform/transforms/SkewTransformOperation.h
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/web/linux/WebFontRendering.cpp
[modify] https://crrev.com/69969af74bfff920f5fdcc2aa547ea4af2d477d2/third_party/WebKit/Source/web/win/WebFontRendering.cpp

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/34971fa949b0957eb7c4feb575f372b2c0c6b98a

commit 34971fa949b0957eb7c4feb575f372b2c0c6b98a
Author: lukasza <lukasza@chromium.org>
Date: Wed Sep 14 20:26:44 2016

Allocate ExternalMemoryAllocator on the stack.

The change is desirable because:

- Ref-counting doesn't result in memory safety here, because
  ExternalMemoryAllocator wraps a raw pointer to short-lived
  memory.  Ref-counting cannot extend the lifetime of the
  memory this pointer points to.

- No consumers of SkBitmap::Allocator currently use ref-counting.
  If SkBitmap::Allocator was designed today, it probably wouldn't
  use SkRefCnt as a base class.

- Blink-to-Chromium style conversion will rename |ref| to |Ref| in WTF,
  but won't do it outside of Blink/WTF (e.g. in Skia), so RefPtr should
  only be used with Blink/WTF types going forward (i.e. Skia types
  should be wrapped in smart pointers provided by Skia).

More discussion is in an older code review:
https://codereview.chromium.org/1880993003/#msg10

BUG= 641014 

Review-Url: https://codereview.chromium.org/2326473005
Cr-Commit-Position: refs/heads/master@{#418657}

[modify] https://crrev.com/34971fa949b0957eb7c4feb575f372b2c0c6b98a/third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp

Is this fixed?
Status: Fixed (was: Untriaged)
This seems to be fixed indeed (or at least there are no more known cases of this issue).

Sign in to add a comment