[webkit-reviews] review granted: [Bug 203526] Introduce WorkerEventLoop and use it in FetchBodyOwner::runNetworkTaskWhenPossible : [Attachment 382274] Added an assertion

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 203526: Introduce WorkerEventLoop and use it in
FetchBodyOwner::runNetworkTaskWhenPossible
https://bugs.webkit.org/show_bug.cgi?id=203526

Attachment 382274: Added an assertion

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




--- Comment #16 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 382274
  --> https://bugs.webkit.org/attachment.cgi?id=382274
Added an assertion

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

r=me with comments

> Source/WebCore/dom/Document.cpp:6132
>  {

We likely want a isMainThread() assertion here too.

> Source/WebCore/workers/WorkerEventLoop.cpp:61
> +bool WorkerEventLoop::hasPendingActivity() const

Why do we need to override this. Presumably, the default implementation already
returns false if you don’t use makePendingActivity / setPendingActivity.

> Source/WebCore/workers/WorkerEventLoop.cpp:71
> +void WorkerEventLoop::suspend(ReasonForSuspension)

We don’t suspend / resume ActiveDOMObjects on workers. Instead, we suspend the
whole thread. As a result, I don’t think the suspend / resume methods do
anything here.

> Source/WebCore/workers/WorkerEventLoop.h:48
> +    WorkerEventLoop(ScriptExecutionContext&);

Explicit