Bug 69635 – Web Inspector: allow to start WorkerContext paused

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Yury Semikhatsky]

In order to be able to debug worker context initialization inspector needs to connect to front-end and restore its state before first statement of the worker script is executed. That is why we need an API for pausing worker context execution right after the context was created.

[Yury Semikhatsky]

Created attachment 110157 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

[Pavel Feldman]

Comment on attachment 110157 [details]
Patch

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

> Source/WebCore/workers/WorkerThread.cpp:68
> +    static PassOwnPtr<WorkerThreadStartupData> create(const KURL& scriptURL, const String& userAgent, const String& sourceCode, bool startPaused)

Please make sure worker folks are fine with this additional param prior to landing. Otherwise looks good.

[Early Warning System Bot]

Comment on attachment 110157 [details]
Patch

Attachment 110157 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10009268

[Dmitry Lomov]

Looks good.

[Dmitry Titov]

Comment on attachment 110157 [details]
Patch

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

Couple of nits, not on substance:

> Source/WebCore/workers/WorkerMessagingProxy.cpp:271
> +    RefPtr<DedicatedWorkerThread> thread = DedicatedWorkerThread::create(scriptURL, userAgent, sourceCode, *this, *this, false);

If we need to call this method with explicit bool param, WebKit style usually requires an enum instead of the bool. It makes call site more readable.

> Source/WebCore/workers/WorkerThread.cpp:94
> +#if ENABLE(NOTIFICATIONS)

Could you please do it in a separate patch? It's a good catch, but unrelated.

[Gustavo Noronha (kov)]

Comment on attachment 110157 [details]
Patch

Attachment 110157 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10005296

[Yury Semikhatsky]

Created attachment 110340 [details]
Patch for landing

[Yury Semikhatsky]

(In reply to comment #6)
> (From update of attachment 110157 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=110157&action=review
> 
> Couple of nits, not on substance:
> 
> > Source/WebCore/workers/WorkerMessagingProxy.cpp:271
> > +    RefPtr<DedicatedWorkerThread> thread = DedicatedWorkerThread::create(scriptURL, userAgent, sourceCode, *this, *this, false);
> 
> If we need to call this method with explicit bool param, WebKit style usually requires an enum instead of the bool. It makes call site more readable.
> 
Good point. Done.

> > Source/WebCore/workers/WorkerThread.cpp:94
> > +#if ENABLE(NOTIFICATIONS)
> 
> Could you please do it in a separate patch? It's a good catch, but unrelated.
Done. See https://bugs.webkit.org/show_bug.cgi?id=69741

[WebKit Review Bot]

Attachment 110340 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/workers/DedicatedWorkerThread.h:43:  The parameter name "startMode" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/workers/DedicatedWorkerThread.h:52:  The parameter name "startMode" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/workers/WorkerThread.cpp:78:  The parameter name "startMode" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/workers/SharedWorkerThread.h:41:  The parameter name "startMode" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/workers/SharedWorkerThread.h:48:  The parameter name "startMode" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/workers/WorkerThread.h:70:  The parameter name "startMode" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yury Semikhatsky]

Created attachment 110341 [details]
Patch for landing

Fixed style failures

[Yury Semikhatsky]

Committed r97049: <http://trac.webkit.org/changeset/97049>