[webkit-reviews] review granted: [Bug 218141] Web Inspector: console command line API should be exposed to breakpoint conditions/actions : [Attachment 412232] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Oct 26 13:48:30 PDT 2020
Brian Burg <bburg at apple.com> has granted Devin Rousso <drousso at apple.com>'s
request for review:
Bug 218141: Web Inspector: console command line API should be exposed to
breakpoint conditions/actions
https://bugs.webkit.org/show_bug.cgi?id=218141
Attachment 412232: Patch
https://bugs.webkit.org/attachment.cgi?id=412232&action=review
--- Comment #4 from Brian Burg <bburg at apple.com> ---
Comment on attachment 412232
--> https://bugs.webkit.org/attachment.cgi?id=412232
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/JavaScriptRuntimeCompletionProv
ider.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.
More information about the webkit-reviews
mailing list