[webkit-reviews] review granted: [Bug 193359] Prevent WorkerRunLoop::runInMode from spinning in nested cases : [Attachment 359108] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jan 15 15:50:15 PST 2019


Joseph Pecoraro <joepeck at webkit.org> has granted youenn fablet
<youennf at gmail.com>'s request for review:
Bug 193359: Prevent WorkerRunLoop::runInMode from spinning in nested cases
https://bugs.webkit.org/show_bug.cgi?id=193359

Attachment 359108: Patch

https://bugs.webkit.org/attachment.cgi?id=359108&action=review




--- Comment #7 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 359108
  --> https://bugs.webkit.org/attachment.cgi?id=359108
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=359108&action=review

r=me

> Source/WebCore/ChangeLog:13
> +	   - The run loop timer to be active and need to fire immediatly.

A few grammatical issues / typos in the ChangeLog. You could try giving it a
cleanup pass.

Here: "The run loop timer is active and needs to fire immediately"

> Source/WebCore/workers/WorkerRunLoop.cpp:-118
> -	   m_runLoop.m_nestedCount++;

I liked the older style, it was easier to follow when the increments happen
because it is not clever.

> Source/WebCore/workers/WorkerRunLoop.h:92
> +	   bool isBeingDebugged() const { return m_debugCount; }

I'd rather see a condition like `m_debugCount >= 1` but I'll leave that up to
you.


More information about the webkit-reviews mailing list