WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
203300
Web Inspector: add ITML debuggable/target type
https://bugs.webkit.org/show_bug.cgi?id=203300
Summary
Web Inspector: add ITML debuggable/target type
Devin Rousso
Reported
2019-10-23 10:11:29 PDT
This will allow the frontend to do distinguish between a regular `JSContext` and an ITML one, as well as knowing at launch what domains/commands/events will be supported by ITML, most of which are not normally supported by `JSContext`, which would also allow us to remove the "extra domains" concept.
Attachments
[Patch] WIP
(84.57 KB, patch)
2019-10-23 13:08 PDT
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Patch
(669.68 KB, patch)
2019-11-09 01:03 PST
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Patch
(686.94 KB, patch)
2020-01-15 21:15 PST
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Patch
(692.72 KB, patch)
2020-01-15 21:22 PST
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Patch
(691.92 KB, patch)
2020-01-15 23:02 PST
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Patch
(555.61 KB, patch)
2020-05-21 21:46 PDT
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Patch
(530.91 KB, patch)
2020-05-28 11:48 PDT
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Patch
(530.34 KB, patch)
2020-05-28 15:07 PDT
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Patch
(530.36 KB, patch)
2020-05-28 15:37 PDT
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Patch
(531.15 KB, patch)
2020-05-28 17:41 PDT
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Patch
(544.04 KB, patch)
2020-05-28 21:32 PDT
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Show Obsolete
(10)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2019-10-23 11:07:33 PDT
<
rdar://problem/56545896
>
Devin Rousso
Comment 2
2019-10-23 13:08:27 PDT
Created
attachment 381716
[details]
[Patch] WIP
EWS Watchlist
Comment 3
2019-10-23 13:09:06 PDT
This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`) This patch modifies the inspector protocol. Please ensure that any frontend changes appropriately use feature checks for new protocol features.
Devin Rousso
Comment 4
2019-11-09 01:03:19 PST
Created
attachment 383208
[details]
Patch
Devin Rousso
Comment 5
2020-01-15 21:15:44 PST
Created
attachment 387892
[details]
Patch Rebase
Devin Rousso
Comment 6
2020-01-15 21:22:31 PST
Created
attachment 387893
[details]
Patch
Joseph Pecoraro
Comment 7
2020-01-15 22:04:44 PST
Comment on
attachment 387893
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=387893&action=review
I did a quick once over, this looks good!
> Source/WebInspectorUI/UserInterface/Base/Main.js:3155 > - if (!WI.sharedApp.isWebDebuggable()) > + if (!InspectorBackend.hasCommand("Page.archive"))
Nice =)
> Source/WebInspectorUI/UserInterface/Protocol/Legacy/10.3/InspectorBackendCommands.js:176 > -InspectorBackend.activateDomain("Database", ["page"]); > +InspectorBackend.activateDomain("Database", ["itml", "page"]);
We don't want "itml" for the "Database" domain. These probably need to be regenerated.
> Source/WebInspectorUI/UserInterface/Views/CookieStorageContentView.js:361 > _handleTableKeyDown(event) > { > - if (event.keyCode === WI.KeyboardShortcut.Key.Backspace.keyCode || event.keyCode === WI.KeyboardShortcut.Key.Delete.keyCode) > - this._table.removeSelectedRows(); > + if (event.keyCode === WI.KeyboardShortcut.Key.Backspace.keyCode || event.keyCode === WI.KeyboardShortcut.Key.Delete.keyCode) { > + if (InspectorBackend.hasCommand("Page.deleteCookie")) > + this._table.removeSelectedRows(); > + } > }
Do this beep appropriately if there is a delete and nothing gets deleted? This code is weird (probably an existing issue if there is an issue).
> WebKit.xcworkspace/xcshareddata/xcschemes/All Source.xcscheme:60 > - BuildableName = "libANGLE-shared.dylib" > + BuildableName = "libANGLE.a"
This seems wrong.
Devin Rousso
Comment 8
2020-01-15 22:56:47 PST
Comment on
attachment 387893
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=387893&action=review
>> Source/WebInspectorUI/UserInterface/Protocol/Legacy/10.3/InspectorBackendCommands.js:176 >> +InspectorBackend.activateDomain("Database", ["itml", "page"]); > > We don't want "itml" for the "Database" domain. These probably need to be regenerated.
I think I just messed up this one. It should be `InspectorBackend.activateDomain("DOMStorage", ["itml", "page"]);` instead :(
>> Source/WebInspectorUI/UserInterface/Views/CookieStorageContentView.js:361 >> } > > Do this beep appropriately if there is a delete and nothing gets deleted? This code is weird (probably an existing issue if there is an issue).
I doubt it.
>> WebKit.xcworkspace/xcshareddata/xcschemes/All Source.xcscheme:60 >> + BuildableName = "libANGLE.a" > > This seems wrong.
How did this get added??!?!?
Devin Rousso
Comment 9
2020-01-15 23:02:29 PST
Created
attachment 387896
[details]
Patch
Joseph Pecoraro
Comment 10
2020-01-16 13:30:07 PST
Comment on
attachment 387896
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=387896&action=review
r=me!
> Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js:203 > + for (let [targetType, command] of domain._supportedCommandsForTargetType) { > + if (!supportedTargetTypes.has(targetType)) > + continue;
I think we should be able to write a test for this (namely the last line in an LayoutTests/inspector test): InspectorTest.expectTrue(InspectorBackend.hasDomain("DOM"), "Web Debuggable has DOM Domain"); InspectorTest.expectTrue(InspectorBackend.hasCommand("DOM.requestNode"), "DOM.requestNode is available"); InspectorTest.expectFalse(InspectorBackend.hasCommand("DOM.getDataBindingsForNode"), "DOM.getDataBindingsForNode is not available");
Devin Rousso
Comment 11
2020-05-21 21:46:31 PDT
Created
attachment 400025
[details]
Patch
Blaze Burg
Comment 12
2020-05-22 15:21:43 PDT
Comment on
attachment 400025
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=400025&action=review
r=me too
> Source/JavaScriptCore/API/JSContextPrivate.h:112 > + @discussion ITMLKit.
Please expand or remove this line.
Devin Rousso
Comment 13
2020-05-28 11:48:31 PDT
Created
attachment 400488
[details]
Patch i undid the removal of the "extra agents" stuff, as that may still be desired if an older client links against this new WebKit. instead, we just don't dispatch `Inspector.activateExtraDomains` unless we're a basic `JavaScript` debuggable.
Devin Rousso
Comment 14
2020-05-28 15:07:49 PDT
Created
attachment 400512
[details]
Patch
Devin Rousso
Comment 15
2020-05-28 15:37:00 PDT
Created
attachment 400516
[details]
Patch oops wrong method
Devin Rousso
Comment 16
2020-05-28 17:41:05 PDT
Created
attachment 400527
[details]
Patch
Devin Rousso
Comment 17
2020-05-28 21:32:41 PDT
Created
attachment 400545
[details]
Patch add more protocol checks
EWS
Comment 18
2020-05-29 10:34:13 PDT
Committed
r262302
: <
https://trac.webkit.org/changeset/262302
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 400545
[details]
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug