Bug 179147 – Update SWServerJobQueue to follow the Service Worker specification more closely

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

Update SWServerJobQueue to follow the Service Worker specification more closely.

[Chris Dumez]

Created attachment 325662 [details]
Patch

[youenn fablet]

Comment on attachment 325662 [details]
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.

[Chris Dumez]

Comment on attachment 325662 [details]
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?

How? if you're suggesting callOnMainThread([] { }), then I prefer a timer because it cannot outlive the SWServerJobQueue object.

>> 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

Not needed.

>> 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.

I did not remove the notion of current job. I only removed the m_currentJob member and the code now treats firstJob() as the current job.

[Chris Dumez]

Created attachment 325727 [details]
Patch

[Chris Dumez]

Created attachment 325729 [details]
Patch

[Chris Dumez]

Comment on attachment 325729 [details]
Patch

Clearing flags on attachment: 325729

Committed r224341: <https://trac.webkit.org/changeset/224341>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35567457>