Bug 126038 – Web Inspector: Extract CommandLineAPI into its own InjectedScriptModule

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

The CommandLineAPI that currently lives inside of InjectedScriptSource.js is infested with knowledge and functions dealing with the DOM. "getEventListeners", "$n", "$$", etc. These APIs cannot be used when inspecting a JSContext. However the rest of InjectedScriptSource is generic. Split the CommandLineAPI code out into is own module.

[Radar WebKit Bug Importer]

<rdar://problem/15705590>

[Joseph Pecoraro]

Created attachment 219713 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Comment on attachment 219713 [details]
[PATCH] Proposed Fix

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

> Source/WebCore/inspector/PageRuntimeAgent.cpp:143
>          JSC::ExecState* scriptState = mainWorldExecState(&m_inspectedPage->mainFrame());
>          InjectedScript result = injectedScriptManager()->injectedScriptFor(scriptState);
>          if (result.hasNoValue())
> -            *errorString = "Internal error: main world execution context not found.";
> +            *errorString = ASCIILiteral("Internal error: main world execution context not found.");
> +        CommandLineAPIModule::injectIfNeeded(injectedScriptManager(), result);
>          return result;
>      }
> +
>      InjectedScript injectedScript = injectedScriptManager()->injectedScriptForId(*executionContextId);
>      if (injectedScript.hasNoValue())
> -        *errorString = "Execution context with given id not found.";
> +        *errorString = ASCIILiteral("Execution context with given id not found.");
> +    CommandLineAPIModule::injectIfNeeded(injectedScriptManager(), injectedScript);
>      return injectedScript;
>  }

Although this works, I don't think it is ideal. This means every time you evaluate something in the console (e.g. 1+1) we first evaluate and check the result of 'InjectedScript.module("CommandLineAPI")'.

I think a better approach will be to subclass InjectedScriptManager, and provide a one evaluate at the same time we create InjectedScript. So we only have to inject the CommandLineAPI once, instead of keep checking it. I'll make this change tomorrow.

[WebKit Commit Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/GNUmakefile.am', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/inspector/CommandLineAPIModule.cpp', u'Source/WebCore/inspector/CommandLineAPIModule.h', u'Source/WebCore/inspector/CommandLineAPIModuleSource.js', u'Source/WebCore/inspector/InjectedScriptCanvasModule.h', u'Source/WebCore/inspector/InjectedScriptModule.cpp', u'Source/WebCore/inspector/InjectedScriptModule.h', u'Source/WebCore/inspector/InjectedScriptSource.js', u'Source/WebCore/inspector/InspectorAllInOne.cpp', u'Source/WebCore/inspector/PageRuntimeAgent.cpp', '--commit-queue']" exit_code: 1
ERROR: Source/WebCore/inspector/PageRuntimeAgent.cpp:133:  Declaration has space between type name and * in *errorString  [whitespace/declaration] [3]
ERROR: Source/WebCore/inspector/PageRuntimeAgent.cpp:140:  Declaration has space between type name and * in *errorString  [whitespace/declaration] [3]
Total errors found: 2 in 17 files


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

[EFL EWS Bot]

Comment on attachment 219713 [details]
[PATCH] Proposed Fix

Attachment 219713 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/49338152

[Joseph Pecoraro]

Created attachment 219717 [details]
[PATCH] Part 1 - Extract CommandLineAPIModule

Same comment from Part 1 still applies here.

[WebKit Commit Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/GNUmakefile.am', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/inspector/CommandLineAPIModule.cpp', u'Source/WebCore/inspector/CommandLineAPIModule.h', u'Source/WebCore/inspector/CommandLineAPIModuleSource.js', u'Source/WebCore/inspector/InjectedScriptCanvasModule.h', u'Source/WebCore/inspector/InjectedScriptModule.cpp', u'Source/WebCore/inspector/InjectedScriptModule.h', u'Source/WebCore/inspector/InjectedScriptSource.js', u'Source/WebCore/inspector/InspectorAllInOne.cpp', u'Source/WebCore/inspector/PageRuntimeAgent.cpp', '--commit-queue']" exit_code: 1
ERROR: Source/WebCore/inspector/PageRuntimeAgent.cpp:133:  Declaration has space between type name and * in *errorString  [whitespace/declaration] [3]
ERROR: Source/WebCore/inspector/PageRuntimeAgent.cpp:140:  Declaration has space between type name and * in *errorString  [whitespace/declaration] [3]
Total errors found: 2 in 17 files


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

[Joseph Pecoraro]

Created attachment 219771 [details]
[PATCH] Part 2 - Inject CommandLineAPIModule once, instead checking over and over again

[Joseph Pecoraro]

Created attachment 219773 [details]
[PATCH] For Bots 1

[WebKit Commit Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/GNUmakefile.am', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/inspector/CommandLineAPIModule.cpp', u'Source/WebCore/inspector/CommandLineAPIModule.h', u'Source/WebCore/inspector/CommandLineAPIModuleSource.js', u'Source/WebCore/inspector/InjectedScriptCanvasModule.h', u'Source/WebCore/inspector/InjectedScriptManager.cpp', u'Source/WebCore/inspector/InjectedScriptManager.h', u'Source/WebCore/inspector/InjectedScriptModule.cpp', u'Source/WebCore/inspector/InjectedScriptModule.h', u'Source/WebCore/inspector/InjectedScriptSource.js', u'Source/WebCore/inspector/InspectorAllInOne.cpp', u'Source/WebCore/inspector/PageInjectedScriptManager.cpp', u'Source/WebCore/inspector/PageInjectedScriptManager.h', u'Source/WebCore/inspector/PageRuntimeAgent.cpp', '--commit-queue']" exit_code: 1
ERROR: Source/WebCore/inspector/PageInjectedScriptManager.h:35:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 21 files


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

[EFL EWS Bot]

Comment on attachment 219773 [details]
[PATCH] For Bots 1

Attachment 219773 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/47608249

[Build Bot]

Comment on attachment 219773 [details]
[PATCH] For Bots 1

Attachment 219773 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/46678233

[Joseph Pecoraro]

Created attachment 219775 [details]
[PATCH] For Bots 2

[WebKit Commit Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/GNUmakefile.am', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/inspector/CommandLineAPIModule.cpp', u'Source/WebCore/inspector/CommandLineAPIModule.h', u'Source/WebCore/inspector/CommandLineAPIModuleSource.js', u'Source/WebCore/inspector/InjectedScriptCanvasModule.h', u'Source/WebCore/inspector/InjectedScriptManager.cpp', u'Source/WebCore/inspector/InjectedScriptManager.h', u'Source/WebCore/inspector/InjectedScriptModule.cpp', u'Source/WebCore/inspector/InjectedScriptModule.h', u'Source/WebCore/inspector/InjectedScriptSource.js', u'Source/WebCore/inspector/InspectorAllInOne.cpp', u'Source/WebCore/inspector/PageDebuggerAgent.cpp', u'Source/WebCore/inspector/PageInjectedScriptManager.cpp', u'Source/WebCore/inspector/PageInjectedScriptManager.h', u'Source/WebCore/inspector/PageRuntimeAgent.cpp', '--commit-queue']" exit_code: 1
ERROR: Source/WebCore/inspector/PageInjectedScriptManager.h:35:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 1 in 22 files


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

[Joseph Pecoraro]

Committed r160924 <http://trac.webkit.org/changeset/160924>
Committed r160925 <http://trac.webkit.org/changeset/160925>