[webkit-reviews] review granted: [Bug 37642] Use a lower-overhead mechanism for plug-in message throttling : [Attachment 53416] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted Steve Falkenburg
<sfalken at apple.com>'s request for review:
Bug 37642: Use a lower-overhead mechanism for plug-in message throttling
https://bugs.webkit.org/show_bug.cgi?id=37642

Attachment 53416: Patch
https://bugs.webkit.org/attachment.cgi?id=53416&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
> +	   - The timer used to process the excess messages had a very low
timeout (1ms).
> +	     Other browsers use a value of 5ms for this delay.

You could say which "other browsers", for posterity's sake.

> +// Set a timer to make sure we process any queued messages at least every
16ms.
> +static const double MessageThrottleTimeInterval = 0.016;

It might be good to say (both here and in the ChangeLog) how 16ms was chosen.

> +// During a continuous stream of timers, process one every 5ms.
> +static const double MessageDirectProcessingInterval = 0.005;

I think you meant "stream of messages".

> @@ -74,11 +81,21 @@ void PluginMessageThrottlerWin::appendMe
>      if (!m_front)
>	   m_front = message;
>  
> +    // If it has been more than MessageDirectProcessingInterval between
throttled messages,
> +    // go ahead and process a message directly.
> +    double currentTime = WTF::currentTime();
> +    if (currentTime - m_lastMessageTime > MessageDirectProcessingInterval) {

> +	   processQueuedMessage();
> +	   m_lastMessageTime = currentTime;
> +	   if (!m_front)
> +	       return;
> +    }
> +

Seems like we should update m_lastMessageTime in processQueuedMessage() instead
of here.

r=me