[Webkit-unassigned] [Bug 110779] WebKit API for enabling DOM logging for certain worlds
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Feb 26 13:55:36 PST 2013
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.
More information about the webkit-unassigned
mailing list