[Webkit-unassigned] [Bug 50799] Add Memory Sampler to WebKit

[bugzilla-daemon at webkit.org]

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


Geoffrey Garen <ggaren at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #77976|review?                     |review+
               Flag|                            |




--- Comment #22 from Geoffrey Garen <ggaren at apple.com>  2011-01-05 22:43:00 PST ---
(From update of attachment 77976)
View in context: https://bugs.webkit.org/attachment.cgi?id=77976&action=review

Please make sure to address these comments before landing.

> WebKit2/WebKit2Prefix.h:71
> +#if PLATFORM(MAC)
> +#define ENABLE_MEMORY_SAMPLER 1
> +#endif

I don't see this #define used anywhere. What's it for?

> WebKit2/Shared/WebMemorySampler.cpp:34
> +static WebMemorySampler *sharedMemorySampler = 0;

Should be "WebMemorySampler* ".

You can put this static inside WebMemorySampler::shared().

Since statics are implicitly initialized to 0, there's no need to explicitly assign 0 here.

> WebKit2/Shared/WebMemorySampler.cpp:137
> +    m_separator = String("\t");    

You should initialize m_separator in the WebMemorySampler constructor. It's difficult to reason about data members that are initialized at unpredictable times.

> WebKit2/Shared/WebMemorySampler.cpp:138
> +    String header = String(); 

A simpler way to say this is "String header;"

> WebKit2/Shared/WebMemorySampler.cpp:166
> +    String statString = String(); 

"String statString;"

> WebKit2/UIProcess/WebContext.cpp:208
> +        // way to get name of process from UI side?

What does this comment mean?

> WebKit2/UIProcess/WebContext.cpp:557
> +    

Our style is to exclude braces from one-line if statements.

> WebKit2/UIProcess/WebContext.cpp:580
> +    

Our style is to exclude braces from one-line if statements.

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