Bug 96277

Summary: Web Inspector: NMI: instrument NativeImageSkia
Product: WebKit Reporter: Ilya Tikhonovsky <loislo>
Component: Web Inspector (Deprecated)Assignee: Ilya Tikhonovsky <loislo>
Status: RESOLVED FIXED    
Severity: Normal CC: alph, apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, senorblanco, timothy, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch yurys: review+

Description Ilya Tikhonovsky 2012-09-10 09:08:02 PDT 
EOM
Comment 1 Ilya Tikhonovsky 2012-09-10 09:10:23 PDT 
Created attachment 163144 [details]
Patch
Comment 2 Yury Semikhatsky 2012-09-10 21:30:21 PDT 
Comment on attachment 163144 [details]
Patch

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

> Source/WebCore/platform/graphics/skia/NativeImageSkia.cpp:40
> +#include "WebCoreMemoryInstrumentation.h"

As discussed, we shouldn't add dependency on WebCore.
Comment 3 Ilya Tikhonovsky 2012-10-03 07:03:22 PDT 
Created attachment 166887 [details]
Patch
Comment 4 Yury Semikhatsky 2012-10-03 07:25:15 PDT 
Comment on attachment 166887 [details]
Patch

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

> Source/WebCore/platform/graphics/BitmapImage.cpp:586
> +void reportMemoryUsage(const NativeImageSkia* const&, MemoryObjectInfo*);

Let's move this declaration into NativeImagePtr.h next to NativeImagePtr typdef for Skia.

> Source/WebCore/platform/graphics/skia/NativeImageSkia.cpp:167
> +void reportSkBitmapMemoryUsage(MemoryClassInfo& info, const SkBitmap& image)

missing static?

> Source/WebCore/platform/graphics/skia/NativeImageSkia.cpp:178
> +    reportSkBitmapMemoryUsage(info, m_image);

Why not declare WTF::reportMemoryUsage(...SkBitmap... and call addMember here?
Comment 5 Ilya Tikhonovsky 2012-10-03 07:46:36 PDT 
Created attachment 166891 [details]
Patch
Comment 6 Ilya Tikhonovsky 2012-10-03 07:51:07 PDT 
Committed r130289: <http://trac.webkit.org/changeset/130289>