Bug 83023

Summary: Call incrementStatsCounter directly
Product: WebKit Reporter: Mark Pilgrim (Google) <pilgrim>
Component: New BugsAssignee: Mark Pilgrim (Google) <pilgrim>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, fishd, haraken, japhet, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 82948    
Attachments:
Description Flags
Patch none

Description Mark Pilgrim (Google) 2012-04-03 06:09:30 PDT
Call incrementStatsCounter directly
Comment 1 Mark Pilgrim (Google) 2012-04-03 06:10:12 PDT
Created attachment 135319 [details]
Patch
Comment 2 Kentaro Hara 2012-04-03 06:14:47 PDT
Comment on attachment 135319 [details]
Patch

OK!
Comment 3 WebKit Review Bot 2012-04-03 08:03:44 PDT
Comment on attachment 135319 [details]
Patch

Clearing flags on attachment: 135319

Committed r113034: <http://trac.webkit.org/changeset/113034>
Comment 4 WebKit Review Bot 2012-04-03 08:03:49 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Adam Barth 2012-04-03 08:45:25 PDT
Comment on attachment 135319 [details]
Patch

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

> Source/WebCore/bindings/v8/V8Proxy.h:49
> -#define INC_STATS(name) PlatformSupport::incrementStatsCounter(name)
> +#define INC_STATS(name) webkitPlatformSupport()->incrementStatsCounter(name)

Does this compile with ENABLE_DOM_STATS_COUNTERS enabled?  webkitPlatformSupport shouldn't be visible outside the WebKit layer, but this macro is used in WebCore.
Comment 6 Kentaro Hara 2012-04-03 08:50:26 PDT
Comment on attachment 135319 [details]
Patch

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

>> Source/WebCore/bindings/v8/V8Proxy.h:49
>> +#define INC_STATS(name) webkitPlatformSupport()->incrementStatsCounter(name)
> 
> Does this compile with ENABLE_DOM_STATS_COUNTERS enabled?  webkitPlatformSupport shouldn't be visible outside the WebKit layer, but this macro is used in WebCore.

Ah, good point, the indirection call is necessary. May I roll the patch out?
Comment 7 Adam Barth 2012-04-03 10:02:30 PDT
> Ah, good point, the indirection call is necessary. May I roll the patch out?

I don't think anyone commonly builds this configuration, so we might want to wait for Mark to fix it.
Comment 8 Mark Pilgrim (Google) 2012-04-03 19:55:40 PDT
(In reply to comment #7)
> > Ah, good point, the indirection call is necessary. May I roll the patch out?
> 
> I don't think anyone commonly builds this configuration, so we might want to wait for Mark to fix it.

Fix is bug 83109.