Bug 179142 - Drop running Service Worker Jobs on a background thread
Summary: Drop running Service Worker Jobs on a background thread
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
Keywords: InRadar
Depends on:
Reported: 2017-11-01 15:53 PDT by Chris Dumez
Modified: 2017-11-15 12:42 PST (History)
4 users (show)

See Also:

Patch (13.22 KB, patch)
2017-11-01 15:59 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (15.45 KB, patch)
2017-11-01 16:22 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2017-11-01 15:53:34 PDT
Drop running Service Worker Jobs on a background thread. We don't really need to and this simplifies the logic a lot.
Comment 1 Chris Dumez 2017-11-01 15:59:27 PDT
Created attachment 325644 [details]
Comment 2 youenn fablet 2017-11-01 16:14:36 PDT
Comment on attachment 325644 [details]

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

> Source/WebCore/workers/service/server/SWServer.cpp:75
> +    ASSERT(isMainThread());

Do we still need these 3 asserts now? For future persistency work maybe, right now they do not add much.

> Source/WebCore/workers/service/server/SWServerJobQueue.cpp:198
>      // If jobâs job type is update, and newestWorkerâs script url does not equal jobâs script url with the exclude fragments flag set, then:

s/job's job/job's/

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

Can we have m_jobTimer being active already?
Is startOneShot handling the active state by exiting early?

> Source/WebCore/workers/service/server/SWServerRegistration.cpp:41
> +    , m_scriptURL(scriptURL)

Now that we are no longer doing some background thread things, there might be the possibility to go from IPC up to here without having ref count churn.
Would that be possible here?

> Source/WebCore/workers/service/server/SWServerRegistration.cpp:52
> +    ASSERT(isMainThread());

Probably not needed either.
Comment 3 Chris Dumez 2017-11-01 16:22:42 PDT
Created attachment 325647 [details]
Comment 4 Chris Dumez 2017-11-01 16:24:35 PDT
Comment on attachment 325647 [details]

Clearing flags on attachment: 325647

Committed r224306: <https://trac.webkit.org/changeset/224306>
Comment 5 Chris Dumez 2017-11-01 16:24:37 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Radar WebKit Bug Importer 2017-11-15 12:42:42 PST