[webkit-reviews] review granted: [Bug 179147] Update SWServerJobQueue to follow the Service Worker specification more closely : [Attachment 325662] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Nov 2 10:02:50 PDT 2017


youenn fablet <youennf at gmail.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 179147: Update SWServerJobQueue to follow the Service Worker specification
more closely
https://bugs.webkit.org/show_bug.cgi?id=179147

Attachment 325662: Patch

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




--- Comment #2 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 325662
  --> https://bugs.webkit.org/attachment.cgi?id=325662
Patch

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

> Source/WebCore/workers/service/server/SWServerJobQueue.cpp:103
> +    m_jobTimer.startOneShot(0_s);

Do we need a timer? Can't we do that with a lambda?

> Source/WebCore/workers/service/server/SWServerJobQueue.cpp:108
> +    ASSERT(!m_jobQueue.isEmpty());

Is it needed? Is firstJob() not asserting somehow that the queue is not empty?
Ditto elsewhere

> Source/WebCore/workers/service/server/SWServerJobQueue.cpp:212
>  void SWServerJobQueue::finishCurrentJob()

Since we remove the notion of current job, should we also remove it from some
of the methods like this one.


More information about the webkit-reviews mailing list