Description
Andrey Kosyakov
2010-02-19 05:32:32 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
Why no r? and obsolete? (In reply to comment #2) > Why no r? and obsolete? There are some problems with this patch, the right version is coming on. 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 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. 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 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?
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)
(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 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 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 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 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> All reviewed patches have been landed. Closing bug. |