Bug 225060 - Web Inspector: CSS Grid - measure usage of grid overlay options
Summary: Web Inspector: CSS Grid - measure usage of grid overlay options
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: Razvan Caliman
URL:
Keywords: InRadar
Depends on:
Blocks: 233298
  Show dependency treegraph
 
Reported: 2021-04-26 10:35 PDT by Razvan Caliman
Modified: 2021-11-17 18:01 PST (History)
5 users (show)

See Also:


Attachments
Patch (12.91 KB, patch)
2021-04-26 11:45 PDT, Razvan Caliman
no flags Details | Formatted Diff | Diff
Patch (12.83 KB, patch)
2021-04-28 06:43 PDT, Razvan Caliman
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Razvan Caliman 2021-04-26 10:35:47 PDT
<rdar://75843294>
Comment 1 Razvan Caliman 2021-04-26 11:45:16 PDT
Created attachment 427072 [details]
Patch
Comment 2 Blaze Burg 2021-04-27 13:57:05 PDT
Comment on attachment 427072 [details]
Patch

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

r=me, nice work!

> Source/WebInspectorUI/UserInterface/Controllers/GridOverlayConfigurationDiagnosticEventRecorder.js:50
> +        window.addEventListener("focus", this, options);

Eventually we should consolidate our various places in the code that listen for user interaction and have a class manage it. For now, it is fine to continue in this direction until we encounter perf / user input lag.

> Source/WebInspectorUI/UserInterface/Controllers/GridOverlayConfigurationDiagnosticEventRecorder.js:118
> +        if (this._eventSamplingTimerIdentifier) {

Nit: call _stopEventSamplingTimer() rather than duplicating.

> Source/WebInspectorUI/UserInterface/Controllers/GridOverlayConfigurationDiagnosticEventRecorder.js:123
> +        this._eventSamplingTimerIdentifier = setTimeout(this._sampleCurrentOverlayConfiguration.bind(this), GridOverlayConfigurationDiagnosticEventRecorder.eventSamplingInterval);

Nit: WI.GridOverlayConfigurationDiagnosticEventRecorder

> Source/WebInspectorUI/UserInterface/Controllers/GridOverlayConfigurationDiagnosticEventRecorder.js:126
> +    _stopEventSamplingTimer()

Nit: _stopEventSamplingTimerIfNeeded

> Source/WebInspectorUI/UserInterface/Controllers/GridOverlayConfigurationDiagnosticEventRecorder.js:142
> +            if (WI.settings.debugAutoLogDiagnosticEvents.valueRespectingDebugUIAvailability)

Neat.

> Source/WebInspectorUI/UserInterface/Controllers/GridOverlayConfigurationDiagnosticEventRecorder.js:155
> +        // Encode the configuration of overaly options as a sum of increasing powers of 10 for each overlay option that is enabled (zero if disabled), convert to string and pad with zero if necessary.

Nit: overlay
Comment 3 Razvan Caliman 2021-04-28 06:43:09 PDT
Created attachment 427256 [details]
Patch

Carry over R+; Address code review comments
Comment 4 EWS 2021-04-28 08:36:22 PDT
Committed r276714 (237118@main): <https://commits.webkit.org/237118@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 427256 [details].