[Webkit-unassigned] [Bug 21719] JS Profiler notification system needed

[bugzilla-daemon at webkit.org]

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


timothy at hatcher.name changed:

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




------- Comment #4 from timothy at hatcher.name  2008-10-19 06:36 PDT -------
(From update of attachment 24470)
+#import <Foundation/NSDistributedNotificationCenter.h>
+#import <Foundation/NSProcessInfo.h>
+#import <Foundation/NSString.h>
+#import <Foundation/NSUserDefaults.h>
+#import "JSStringRef.h"

All of these #imports should move to the .mm file, you only need to #import
NSObject.h in the JSProfileServer.h header. And you should add @class NSString
to the header.

You need a header guard around the contents of JSProfileServer.h.

#ifndef JSProfileServer_h
#define JSProfileServer_h

// ...

#endif // JSProfileServer_h

I am thinking you don't even need to put the ObjC @interface in the header,
since no one in the process needs to call it except in the .mm file. So just
put startSharedProfileServer() in the header. Then you can remove the #ifdef
__OBJC__ and #ifdef __cplusplus. The header will be C++ only, and the .mm with
be ObjC++.

Also startSharedProfileServer() should be put in the JSC C++ namespace.

+static JSProfileServer *sharedServer = nil;

That can move into the sharedProfileServer class method, since only it accesses
it. Also you can leave off the "= nil" part, since statics default to 0.

+    listenerCount++;
+    if (listenerCount > 0) {
+        JSStringRef profileName = JSStringCreateWithUTF8CString([serverName
UTF8String]);
+        JSStartProfiling(0, profileName);
+        JSStringRelease(profileName);
+    }

I think that should be:

+    if (++listenerCount > 1)
+        return;
+    JSRetainPtr<JSStringRef> profileName(Adopt,
JSStringCreateWithUTF8CString([serverName UTF8String]));
+    JSStartProfiling(0, profileName.get());

I don't think you want to start the profiler multiple times if another start is
requested (since you don't end it the same number of times in stopProfiling).
Also you can use JSRetainPtr since this is C++, and that will do the
JSStringRelease for you. Just include JSRetainPtr.h.

+    listenerCount--;
+    if (listenerCount <= 0) {

That can be an early return.

+    ASSERT(listenerCount);
+    if (--listenerCount > 0)
+        return;
+    JSRetainPtr<JSStringRef> profileName(Adopt,
JSStringCreateWithUTF8CString([serverName UTF8String]));
+    JSEndProfiling(0, profileName.get());

Also you should assert the listenerCount is non-zero before decrementing it.
Include <wtf/Assertions.h>.


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