Bug 218141 - Web Inspector: console command line API should be exposed to breakpoint conditions/actions
Summary: Web Inspector: console command line API should be exposed to breakpoint condi...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-10-23 15:29 PDT by Devin Rousso
Modified: 2020-10-27 09:52 PDT (History)
12 users (show)

See Also:

Attachments
Patch (82.61 KB, patch)
2020-10-23 21:38 PDT, Devin Rousso 		no flags Details | Formatted Diff | Diff
Patch (82.56 KB, patch)
2020-10-23 22:29 PDT, Devin Rousso 		no flags Details | Formatted Diff | Diff
Patch (85.28 KB, patch)
2020-10-26 18:33 PDT, Devin Rousso 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Devin Rousso 2020-10-23 15:29:46 PDT 
<rdar://problem/70636727>
Comment 2 Devin Rousso 2020-10-23 21:38:28 PDT 
Created attachment 412230 [details]
Patch
Comment 3 Devin Rousso 2020-10-23 22:29:30 PDT 
Created attachment 412232 [details]
Patch

rename `Debugger::Delegate` to `Debugger::Client` to match C++ convention
Comment 4 BJ Burg 2020-10-26 13:48:30 PDT 
Comment on attachment 412232 [details]
Patch

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

r=me, cool stuff.

> Source/JavaScriptCore/debugger/Debugger.h:147
> +        virtual JSObject* scopeExtensionObject(Debugger&, JSGlobalObject*, DebuggerCallFrame&) = 0;

I prefer we provide a default implementation so that implementing any particular client call is optional, as in an ObjC delegate protocol.

> Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.h:52
> +class JS_EXPORT_PRIVATE InspectorDebuggerAgent : public InspectorAgentBase, public DebuggerBackendDispatcherHandler, public JSC::Debugger::Client, public JSC::Debugger::Observer {

Nit: please put each superclass on its own line, if you can stand it..

> Source/WebInspectorUI/UserInterface/Controllers/JavaScriptRuntimeCompletionProvider.js:283
> +                if (completionController.mode === WI.CodeMirrorCompletionController.Mode.FullConsoleCommandLineAPI || completionController.mode === WI.CodeMirrorCompletionController.Mode.EventBreakpoint || (completionController.mode === WI.CodeMirrorCompletionController.Mode.PausedConsoleCommandLineAPI && (targetData.pauseReason === WI.DebuggerManager.PauseReason.Listener || targetData.pauseReason === WI.DebuggerManager.PauseReason.EventListener))) {

Oh my gosh. Can this huge expression go into a helper local/function or something?  At the very least please add newlines.

> LayoutTests/inspector/debugger/resources/breakpoint-options-utilities.js:86
> +                breakpoint.condition = "$" + firstEvaluateResult.savedResultIndex;

Niiice.
Comment 5 Devin Rousso 2020-10-26 18:33:35 PDT 
Created attachment 412370 [details]
Patch
Comment 6 EWS 2020-10-26 23:39:14 PDT 
Committed r269023: <https://trac.webkit.org/changeset/269023>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 412370 [details].
Comment 7 Ryan Haddad 2020-10-27 09:52:12 PDT 
Rebaselined inspector/timeline/line-column.html with https://trac.webkit.org/changeset/269044/webkit

This failure was missed by EWS because the entire inspector/timeline directory is marked as flaky, so the tests weren't run.