[Webkit-unassigned] [Bug 66145] [chromium] Assert that main thread and compositor thread are used safely

[bugzilla-daemon at webkit.org]

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


James Robinson <jamesr at chromium.org> changed:

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




--- Comment #13 from James Robinson <jamesr at chromium.org>  2011-08-16 19:13:02 PST ---
(From update of attachment 103838)
View in context: https://bugs.webkit.org/attachment.cgi?id=103838&action=review

Please fix the style issue the bot caught and comments.  I also think you should make sure that this all compiles out in release mode, since the only real side effect is ASSERT()s which are debug only.  You might have to be creative to avoid adding too many #ifndef NDEBUG lines, but it should be possible.  You don't need to guard ASSERT()s since they already compile out.

> WebCore/platform/graphics/chromium/cc/CCThread.cpp:46
> +    ASSERT(ccThreadID == 0);

should be ASSERT(!ccThreadID)

> WebCore/platform/graphics/chromium/cc/CCThread.cpp:92
> +

nit: extra newline

> WebCore/platform/graphics/chromium/cc/CCThread.cpp:97
> +

nit: extra newline

> WebCore/platform/graphics/chromium/cc/CCThread.h:36
> +// True if we're on the compositor thread.
> +bool isCCThread();

I don't think it's a good idea to put this directly in the WebCore namespace.  Can you make this a static function on CCThread, just so that it's namespaced more tightly?

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