- Display list of active workers in a sidebar on the scripts panel. - Provide an option to inject a "fake" workers implementation that allows debugging workers.
Created attachment 49792 [details] Display list of active workers in scripts panel, allow workers debugging by injecting fake workers implementation
Attachment 49792 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/inspector/InspectorController.cpp:1189: Missing spaces around != [whitespace/operators] [3] WebCore/workers/Worker.cpp:48: Alphabetical sorting problem. [build/include_order] [4] WebCore/dom/Document.h:453: Use 0 instead of NULL. [readability/null] [4] WebCore/inspector/InspectorWorkerResource.h:39: Alphabetical sorting problem. [build/include_order] [4] WebCore/inspector/InspectorWorkerResource.h:42: Code inside a namespace should not be indented. [whitespace/indent] [4] WebCore/workers/SharedWorker.cpp:41: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 6 in 30 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 49795 [details] Display list of active workers in scripts panel, allow workers debugging by injecting fake workers implementation (style fixed)
Attachment 49795 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/dom/Document.h:453: Use 0 instead of NULL. [readability/null] [4] WebCore/workers/SharedWorker.cpp:38: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 30 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 49795 [details] Display list of active workers in scripts panel, allow workers debugging by injecting fake workers implementation (style fixed) Overall looks good, a bunch of nits below. > + * Copyright (C) 2009, 2010 Google Inc. All rights reserved. Make it simply 2010. > * > * Redistribution and use in source and binary forms, with or without > * modification, are permitted provided that the following conditions are > @@ -213,6 +213,12 @@ JSValue JSInjectedScriptHost::reportDidD > return jsUndefined(); > } > > +pair<long, ScriptObject> InjectedScriptHost::injectScript(const String& source, ScriptState *scriptState) > +{ > + long id = m_nextInjectedScriptId++; > + return std::make_pair(id, createInjectedScript(source, this, scriptState, id)); > +} > + This should not be custom (make createInjectedScript static member if necessary). > +#if ENABLE(WORKERS) > +void InjectedScriptHost::didCreateWorker(long id, const String &url, bool isSharedWorker) const String& url > +#if ENABLE(WORKERS) > + void didCreateWorker(long id, const String &url, bool isSharedWorker); ditto > > + long generateId(); generateId is a bad name since it will conflict with injected script id. nextWorkerId. > +void InspectorController::clearWorkers() > +{ > + if (m_frontend) { > + WorkersMap::iterator workersEnd = m_workers.end(); > + for (WorkersMap::iterator it = m_workers.begin(); it != workersEnd; ++it) > + m_frontend->willDestroyWorker(*it->second); > + } > + m_workers.clear(); > +} I don't think you need this method - panel's reset will do tear down on the front-end side. > + template<typename T> void innerDidCreateWorker(HashMap<T, RefPtr<InspectorWorkerResource> >* map, T id, const String& url, bool isSharedWorker); > + template<typename T> void innerWillDestroyWorker(HashMap<T, RefPtr<InspectorWorkerResource> >* map, T id); I think you wanted to get rid of this. > + WebInspector.panels.scripts.sidebarPanes.workers.addWorker.apply(WebInspector.panels.scripts.sidebarPanes.workers, arguments); > +} > + > +WebInspector.willDestroyWorker = function() > +{ > + WebInspector.panels.scripts.sidebarPanes.workers.removeWorker.apply(WebInspector.panels.scripts.sidebarPanes.workers, arguments); You should cache WebInspector.panels.scripts.sidebarPanes.workers. > + > +.sidebar-pane-subtitle input[type=checkbox] { We are already slow. Could you use explicit class for this instead? > EventTargetData m_eventTargetData; > + long m_id; > + > + static long s_lastWorkerId; Either m_workerId or s_lastId > @@ -1,3 +1,23 @@ > +2010-03-02 Andrey Kosyakov <caseq@chromium.org> > + > + Reviewed by NOBODY (OOPS!). > + > + Added addScriptToEvaluateOnLoad() / removeAllScriptsToEvaluateOnLoad() > + (part of fake workers injection support) > + https://bugs.webkit.org/show_bug.cgi?id=35568 > + > + * src/js/InspectorControllerImpl.js: > + (devtools.InspectorBackendImpl): > + > +2010-03-02 Andrey Kosyakov <caseq@chromium.org> > + > + Reviewed by NOBODY (OOPS!). > + > + Need a short description and bug URL (OOPS!) > + > + * src/js/InspectorControllerImpl.js: > + (devtools.InspectorBackendImpl): > + > 2010-03-01 Jakob Petsovits <jpetsovits@rim.com> You have 2 entries here. It would it great to have a test for addScriptToEvaluateOnLoad.
Created attachment 49829 [details] Display list of active workers in scripts panel, allow workers debugging by injecting fake workers implementation (nits picked, test added)
Attachment 49829 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/dom/Document.h:453: Use 0 instead of NULL. [readability/null] [4] Total errors found: 1 in 33 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 49830 [details] Screenshot of workers sidebar pane
Are the list of workers in the sidebar clickable to view the source in the Resources panel?
(In reply to comment #9) > Are the list of workers in the sidebar clickable to view the source in the > Resources panel? Yes. Links will probably work only in case instrumentation is on and resources are loaded using XHRs. The change that would provide inspector resources for worker scripts is to follow.
Created attachment 49907 [details] 49829: Display list of active workers in scripts panel, allow workers debugging by injecting fake workers implementation Fixed relative URL resolution bug in fake worker's implementation of importScript() Fixed display of worker list upon front-end attach in Safari
Attachment 49907 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/dom/Document.h:453: Use 0 instead of NULL. [readability/null] [4] Total errors found: 1 in 33 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 49920 [details] Display list of active workers in scripts panel, allow workers debugging by injecting fake workers implementation (fixed importScript sourceURL for Safari compatibility)
Created attachment 50003 [details] Display list of active workers in scripts panel, allow workers debugging by injecting fake workers implementation (rebased to ToT)
Attachment 50003 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/dom/Document.h:453: Use 0 instead of NULL. [readability/null] [4] Total errors found: 1 in 29 files If any of these errors are false positives, please file a bug against check-webkit-style.
Committing to http://svn.webkit.org/repository/webkit/trunk ... C WebCore/workers/SharedWorker.cpp => WebCore/inspector/InspectorWorkerResource.h C WebCore/inspector/front-end/Script.js => WebCore/inspector/front-end/Checkbox.js M WebCore/ChangeLog M WebCore/English.lproj/localizedStrings.js M WebCore/WebCore.gypi M WebCore/WebCore.vcproj/WebCore.vcproj M WebCore/bindings/js/JSInjectedScriptHostCustom.cpp M WebCore/bindings/v8/custom/V8InjectedScriptHostCustom.cpp M WebCore/dom/Document.cpp M WebCore/dom/Document.h M WebCore/dom/ScriptExecutionContext.h M WebCore/inspector/InjectedScriptHost.cpp M WebCore/inspector/InjectedScriptHost.h M WebCore/inspector/InjectedScriptHost.idl M WebCore/inspector/InspectorController.cpp M WebCore/inspector/InspectorController.h M WebCore/inspector/InspectorFrontend.cpp M WebCore/inspector/InspectorFrontend.h M WebCore/inspector/front-end/InjectedFakeWorker.js M WebCore/inspector/front-end/Script.js M WebCore/inspector/front-end/ScriptsPanel.js M WebCore/inspector/front-end/WebKit.qrc A WebCore/inspector/front-end/WorkersSidebarPane.js M WebCore/inspector/front-end/inspector.css M WebCore/inspector/front-end/inspector.html M WebCore/workers/AbstractWorker.cpp M WebCore/workers/AbstractWorker.h M WebCore/workers/SharedWorker.cpp M WebCore/workers/Worker.cpp M WebKit/chromium/src/js/InspectorControllerImpl.js Committed r55522
I had to roll this back since it seems to regress memory footprint according to chromium page cyclers.
Comment on attachment 49829 [details] Display list of active workers in scripts panel, allow workers debugging by injecting fake workers implementation (nits picked, test added) Cleared Pavel Feldman's review+ from obsolete attachment 49829 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment on attachment 49907 [details] 49829: Display list of active workers in scripts panel, allow workers debugging by injecting fake workers implementation Cleared Pavel Feldman's review+ from obsolete attachment 49907 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment on attachment 49920 [details] Display list of active workers in scripts panel, allow workers debugging by injecting fake workers implementation (fixed importScript sourceURL for Safari compatibility) Cleared Pavel Feldman's review+ from obsolete attachment 49920 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment on attachment 50003 [details] Display list of active workers in scripts panel, allow workers debugging by injecting fake workers implementation (rebased to ToT) Marking r- since this was rolled out.
Created attachment 50324 [details] Display list of active workers in scripts panel, allow workers debugging by injecting fake workers implementation (fixed memory footprint problem on page cycler)
Attachment 50324 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/dom/Document.h:453: Use 0 instead of NULL. [readability/null] [4] Total errors found: 1 in 31 files If any of these errors are false positives, please file a bug against check-webkit-style.
r=me
Committing to http://svn.webkit.org/repository/webkit/trunk ... C WebCore/workers/SharedWorker.cpp => WebCore/inspector/InspectorWorkerResource.h M WebCore/ChangeLog M WebCore/English.lproj/localizedStrings.js M WebCore/WebCore.gypi M WebCore/WebCore.vcproj/WebCore.vcproj M WebCore/bindings/js/JSInjectedScriptHostCustom.cpp M WebCore/bindings/v8/custom/V8InjectedScriptHostCustom.cpp M WebCore/dom/Document.cpp M WebCore/dom/Document.h M WebCore/dom/ScriptExecutionContext.h M WebCore/inspector/InjectedScriptHost.cpp M WebCore/inspector/InjectedScriptHost.h M WebCore/inspector/InjectedScriptHost.idl M WebCore/inspector/InspectorController.cpp M WebCore/inspector/InspectorController.h M WebCore/inspector/InspectorFrontend.cpp M WebCore/inspector/InspectorFrontend.h A WebCore/inspector/front-end/Checkbox.js M WebCore/inspector/front-end/InjectedFakeWorker.js M WebCore/inspector/front-end/ScriptsPanel.js M WebCore/inspector/front-end/StylesSidebarPane.js M WebCore/inspector/front-end/WebKit.qrc A WebCore/inspector/front-end/WorkersSidebarPane.js M WebCore/inspector/front-end/inspector.css M WebCore/inspector/front-end/inspector.html M WebCore/workers/AbstractWorker.cpp M WebCore/workers/AbstractWorker.h M WebCore/workers/SharedWorker.cpp M WebCore/workers/Worker.cpp M WebKit/chromium/ChangeLog M WebKit/chromium/src/js/InspectorControllerImpl.js Committed r55771
Comment on attachment 50324 [details] Display list of active workers in scripts panel, allow workers debugging by injecting fake workers implementation (fixed memory footprint problem on page cycler) Clearing r? since this bug is closed.