[webkit-reviews] review requested: [Bug 190060] [iOS][WK1] ASSERTION FAILED: m_creationThread.ptr() == &Thread::current() in ServiceWorkerContainer::~ServiceWorkerContainer() : [Attachment 351020] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has asked  for review:
Bug 190060: [iOS][WK1] ASSERTION FAILED: m_creationThread.ptr() ==
&Thread::current() in ServiceWorkerContainer::~ServiceWorkerContainer()
https://bugs.webkit.org/show_bug.cgi?id=190060

Attachment 351020: Patch

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




--- Comment #6 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 351020
  --> https://bugs.webkit.org/attachment.cgi?id=351020
Patch

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

>>> Source/WebCore/page/NavigatorBase.cpp:155
>>> +	 return m_serviceWorkerContainer.get();
>> 
>> I am not sure this is worth the effort.
>> If we go with it, I would keep returning a ServiceWorkerContainer&.
>> We could assert
RuntimeEnabledFeatures::sharedFeatures().serviceWorkerEnabled() and create
m_serviceWorkerContainer in this method.
> 
> If we keep returning a ServiceWorkerContainer& here then we'd have to check
RuntimeEnabledFeatures::sharedFeatures().serviceWorkerEnabled() at call sites
(e.g. in GC visitors).

Also, if we initialized m_serviceWorkerContainer in this method, then we'd need
an extra member to store the navigator's scriptExecutionContext since the
ServiceWorkerContainer constructor requires it.