WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 233101
Web Inspector: add ExtensionTabActivation diagnostic event
https://bugs.webkit.org/show_bug.cgi?id=233101
Summary
Web Inspector: add ExtensionTabActivation diagnostic event
Blaze Burg
Reported
2021-11-14 11:17:45 PST
.
Attachments
Patch v1.0
(39.22 KB, patch)
2021-11-14 17:47 PST
,
Blaze Burg
hi
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Blaze Burg
Comment 1
2021-11-14 11:20:38 PST
<
rdar://85264921
>
Blaze Burg
Comment 2
2021-11-14 17:47:52 PST
Created
attachment 444201
[details]
Patch v1.0
Devin Rousso
Comment 3
2021-11-15 13:00:13 PST
Comment on
attachment 444201
[details]
Patch v1.0 View in context:
https://bugs.webkit.org/attachment.cgi?id=444201&action=review
r=me, nice work :)
> Source/WebInspectorUI/UserInterface/Controllers/ExtensionTabActivationDiagnosticEventRecorder.js:56 > + console.assert(extension, "Extension tab should have an associated extension.");
NIT: I'd go even further and check `extension instanceof `WI.WebInspectorExtension`
> Source/WebInspectorUI/UserInterface/Controllers/ExtensionTabActivationDiagnosticEventRecorder.js:67 > + let extensionBundleIdentifier = extension.extensionBundleIdentifier; > + let extensionTabName = selectedTab.tabInfo().displayName; > + let activeExtensionTabCount = WI.sharedApp.extensionController.activeExtensionTabContentViews().length; > + this.logDiagnosticEvent(this.name, {extensionBundleIdentifier, extensionTabName, activeExtensionTabCount});
NIT: I'd inline these instead of creating new variables that're only used once ``` this.logDiagnosticEvent(this.name, { extensionBundleIdentifier: extension.extensionBundleIdentifier, extensionTabName: selectedTab.tabInfo().displayName, activeExtensionTabCount: WI.sharedApp.extensionController.activeExtensionTabContentViews().length, }); ```
> Source/WebInspectorUI/UserInterface/Controllers/WebInspectorExtensionController.js:237 > + return [...this._extensionTabContentViewForExtensionTabIDMap.values()].filter((tab) => tab.visible || !!tab.tabBarItem.parentTabBar);
I think we prefer `Array.from(this._extensionTabContentViewForExtensionTabIDMap.values())` NIT: the `!!` is unnecessary since you're only checking for falsy and not saving the result value
Blaze Burg
Comment 4
2021-11-30 14:58:36 PST
Committed
r286329
(
244688@main
): <
https://commits.webkit.org/244688@main
>
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