[Webkit-unassigned] [Bug 110779] WebKit API for enabling DOM logging for certain worlds

[bugzilla-daemon at webkit.org]

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


Adam Barth <abarth at webkit.org> changed:

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




--- Comment #4 from Adam Barth <abarth at webkit.org>  2013-02-26 13:57:58 PST ---
(From update of attachment 190180)
View in context: https://bugs.webkit.org/attachment.cgi?id=190180&action=review

> Source/WebCore/bindings/v8/DOMWrapperWorld.cpp:234
> +    // HashMap doesn't support key 0, so we store the log at worldID + 1
> +    coverWrappingLogs().set(worldID+1, log);

We should use an inline function to convert between the worldID and the key for this map.

> Source/WebKit/chromium/public/WebCoverWrapping.h:39
> +#if WEBKIT_USING_V8
> +namespace v8 {
> +class Value;
> +template <class T> class Handle;
> +}
> +#endif

I think you can just #include v8.h in the API these days.

> Source/WebKit/chromium/public/WebCoverWrapping.h:49
> +        virtual void logMessage(const char*, int, const v8::Handle<v8::Value>*, const char*) { }

Should we have names for these arguments.  It's not clear what these arguments mean.

> Source/WebKit/chromium/src/WebCoverWrapping.cpp:57
> +    OwnPtr<WebCoverWrapping::Log*> m_coverWrappingLog;

OwnPtr<WebCoverWrapping::Log*>  <-- Do you have an extra * here?

> Source/WebKit/chromium/src/WebCoverWrapping.cpp:62
> +    return DOMWrapperWorld::getCoverWrappingLog(worldID) ? true : false; 

There's no reason to use the "? :" operator here.  The compiler will convert it to bool for you.

> Source/WebKit/chromium/src/WebCoverWrapping.cpp:67
> +    DOMWrapperWorld::setCoverWrappingLog(worldID, new CoverWrappingLogContainer(log));

This is a "naked new".  All calls to "new" should be wrapped in either adoptPtr or adoptRef, depending on whether CoverWrappingLogContainer is RefCounted.

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