Bug 35148

Summary: Web Inspector: support debugging of workers.
Product: WebKit Reporter: Andrey Kosyakov <caseq>
Component: Web Inspector (Deprecated)Assignee: Andrey Kosyakov <caseq>
Status: RESOLVED FIXED    
Severity: Enhancement CC: ap, bweinstein, caseq, commit-queue, joepeck, keishi, pfeldman, pmuellr, rik, timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
A patch to enable early injection of script into inspected page and a fake worker implementation to be injected when debugging workers is enabled
none
A patch to enable early injection of script into inspected page and a fake worker implementation to be injected when debugging workers is enabled
pfeldman: review-
A patch to enable early injection of script into inspected page and a fake worker implementation to be injected when debugging workers is enabled (review suggestions addressed)
pfeldman: review+
A patch to enable early injection of script into inspected page and a fake worker implementation to be injected when debugging workers is enabled (nits picked)
pfeldman: review+, commit-queue: commit-queue-
A patch to enable early injection of script into inspected page and a fake worker implementation to be injected when debugging workers is enabled (rebased to correct root path) none

Description Andrey Kosyakov 2010-02-19 05:32:32 PST
Currently, scripts started as workers are not shown in inspector's script pane and hence can't be debugged.
It is suggested that we implement a workaround first, by injecting a script that overrides Worker constructor with a mock implementation that simulates Workers using a separate iframe. This requires inspector to be able to inject a script into inspected page on an early stage (i.e. before any user JS code is parsed).
Comment 1 Andrey Kosyakov 2010-02-24 02:35:18 PST
Created attachment 49369 [details]
A patch to enable early injection of script into inspected page and a fake worker implementation to be injected when debugging workers is enabled
Comment 2 Timothy Hatcher 2010-02-24 03:40:01 PST
Why no r? and obsolete?
Comment 3 Andrey Kosyakov 2010-02-24 05:14:49 PST
(In reply to comment #2)
> Why no r? and obsolete?

There are some problems with this patch, the right version is coming on.
Comment 4 Andrey Kosyakov 2010-02-24 05:18:44 PST
Created attachment 49380 [details]
A patch to enable early injection of script into inspected page and a fake worker implementation to be injected when debugging workers is enabled
Comment 5 Pavel Feldman 2010-02-24 05:26:12 PST
Comment on attachment 49380 [details]
A patch to enable early injection of script into inspected page and a fake worker implementation to be injected when debugging workers is enabled

> +            'inspector/front-end/InjectedFakeWorkers.js',

Rename to InjectedFrameWorker.js?

>  
> +void InjectedScriptHost::addScriptToEvaluateOnLoad(const String& source)
> +{
> +    m_scriptsToEvaluateOnLoad.append(source);
> +}
> +

> +void InjectedScriptHost::removeAllScriptsEvaluatedOnLoad()
> +{

Name is misleading - you don't really remove evaluated scripts. removeAllScriptsToEvaluateOnLoad() ?


> +void InjectedScriptHost::evaluateOnLoadScripts(Frame *frame)
> +{

This could be done in InspectorBackend (or InspectorController) explicitly.
Comment 6 Andrey Kosyakov 2010-02-24 06:52:18 PST
Created attachment 49388 [details]
A patch to enable early injection of script into inspected page and a fake worker implementation to be injected when debugging workers is enabled (review suggestions addressed)
Comment 7 Pavel Feldman 2010-02-24 07:55:23 PST
Comment on attachment 49388 [details]
A patch to enable early injection of script into inspected page and a fake worker implementation to be injected when debugging workers is enabled (review suggestions addressed)

A bunch of style nits, otherwise looks good. Please go through the { } alignment, blank lines and wrapped lines, then we can do cq+!
Btw, where is the injecting code itself?
Comment 8 Andrey Kosyakov 2010-02-24 08:28:53 PST
Created attachment 49397 [details]
A patch to enable early injection of script into inspected page and a fake worker implementation to be injected when debugging workers is enabled (nits picked)
Comment 9 Andrey Kosyakov 2010-02-24 08:32:19 PST
(In reply to comment #7)
> (From update of attachment 49388 [details])
> Btw, where is the injecting code itself?

It's not there yet, as it needs to be done conditionally on a UI option (i.e. "enable workers debugging") -- I'm working on it right now. I'm planning to submit it as a separate change soon.
Comment 10 WebKit Commit Bot 2010-02-24 20:51:47 PST
Comment on attachment 49397 [details]
A patch to enable early injection of script into inspected page and a fake worker implementation to be injected when debugging workers is enabled (nits picked)

Rejecting patch 49397 from commit-queue.

Failed to run "['/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', '--reviewer', 'Pavel Feldman', '--force']" exit_code: 1
Last 500 characters of output:
ipping patch.
1 out of 1 hunk ignored
can't find file to patch at input line 5
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|Index: inspector/front-end/inspector.html
|===================================================================
|--- inspector/front-end/inspector.html	(revision 55161)
|+++ inspector/front-end/inspector.html	(working copy)
--------------------------
No file to patch.  Skipping patch.
1 out of 1 hunk ignored

Full output: http://webkit-commit-queue.appspot.com/results/308353
Comment 11 Andrey Kosyakov 2010-02-25 02:59:15 PST
Created attachment 49471 [details]
A patch to enable early injection of script into inspected page and a fake worker implementation to be injected when debugging workers is enabled (rebased to correct root path)
Comment 12 Andrey Kosyakov 2010-02-25 03:00:15 PST
Comment on attachment 49397 [details]
A patch to enable early injection of script into inspected page and a fake worker implementation to be injected when debugging workers is enabled (nits picked)

Invalid base path
Comment 13 WebKit Commit Bot 2010-02-25 03:24:59 PST
Comment on attachment 49471 [details]
A patch to enable early injection of script into inspected page and a fake worker implementation to be injected when debugging workers is enabled (rebased to correct root path)

Clearing flags on attachment: 49471

Committed r55227: <http://trac.webkit.org/changeset/55227>
Comment 14 WebKit Commit Bot 2010-02-25 03:25:04 PST
All reviewed patches have been landed.  Closing bug.