Bug 194956 - Web Inspector: Timelines: add UI for preventing auto-stop
Summary: Web Inspector: Timelines: add UI for preventing auto-stop
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: 194793
Blocks: 195192
  Show dependency treegraph
 
Reported: 2019-02-22 11:56 PST by Devin Rousso
Modified: 2019-02-28 16:23 PST (History)
6 users (show)

See Also:


Attachments
Patch (9.08 KB, patch)
2019-02-22 12:00 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (9.14 KB, patch)
2019-02-22 12:41 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
[Image] After Patch is applied (755.51 KB, image/png)
2019-02-22 12:41 PST, Devin Rousso
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2019-02-22 11:56:02 PST
There should be some visible UI (e.g. not a keyboard shortcut) that lets the developer toggle auto-stop on/off.
Comment 1 Devin Rousso 2019-02-22 12:00:23 PST
Created attachment 362739 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2019-02-22 12:01:21 PST
<rdar://problem/48319655>
Comment 3 Radar WebKit Bug Importer 2019-02-22 12:01:21 PST
<rdar://problem/48319654>
Comment 4 Joseph Pecoraro 2019-02-22 12:10:56 PST
Comment on attachment 362739 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js:957
> +        if (WI.settings.timelinesAutoStop.value)
> +            this._resetAutoRecordingMaxTimeTimeout();

We should probably only do this if we are capturing.

> Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js:59
> +        this._autoStopCheckboxNavigationItem = new WI.CheckboxNavigationItem("auto-stop-recording", WI.UIString("Stop recording after load"), WI.settings.timelinesAutoStop.value);

The string "Stop recording after load" sounds incorrect.

It sounds like "load" event, but it isn't...

And the auto stop behavior stops for different reasons, and the "load" event isn't exactly one of them:

    • WI.TimelineManager.MaximumAutoRecordDuration = 90000; // 90 seconds
    • WI.TimelineManager.MaximumAutoRecordDurationAfterLoadEvent = 10000; // 10 seconds
    • WI.TimelineManager.DeadTimeRequiredToStopAutoRecordingEarly = 2000; // 2 seconds

So perhaps this should be:

    Stop recordings after page load    
    Stop recordings after navigation
    Automatically stop recordings once page loads
    
Each have less of a "load" event connotation?
Comment 5 Devin Rousso 2019-02-22 12:38:31 PST
Comment on attachment 362739 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js:957
>> +            this._resetAutoRecordingMaxTimeTimeout();
> 
> We should probably only do this if we are capturing.

Ah, good catch!

>> Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js:59
>> +        this._autoStopCheckboxNavigationItem = new WI.CheckboxNavigationItem("auto-stop-recording", WI.UIString("Stop recording after load"), WI.settings.timelinesAutoStop.value);
> 
> The string "Stop recording after load" sounds incorrect.
> 
> It sounds like "load" event, but it isn't...
> 
> And the auto stop behavior stops for different reasons, and the "load" event isn't exactly one of them:
> 
>     • WI.TimelineManager.MaximumAutoRecordDuration = 90000; // 90 seconds
>     • WI.TimelineManager.MaximumAutoRecordDurationAfterLoadEvent = 10000; // 10 seconds
>     • WI.TimelineManager.DeadTimeRequiredToStopAutoRecordingEarly = 2000; // 2 seconds
> 
> So perhaps this should be:
> 
>     Stop recordings after page load    
>     Stop recordings after navigation
>     Automatically stop recordings once page loads
>     
> Each have less of a "load" event connotation?

I actually was trying to refer to the "load" event, as that's what I'd expect the vast majority of users encounter (having a page that takes more than 10s (or even 90s) to load is pretty rare).

I like "Stop recording once page loads" though.
Comment 6 Devin Rousso 2019-02-22 12:41:27 PST
Created attachment 362748 [details]
Patch
Comment 7 Devin Rousso 2019-02-22 12:41:45 PST
Created attachment 362749 [details]
[Image] After Patch is applied
Comment 8 Joseph Pecoraro 2019-02-22 14:02:25 PST
Comment on attachment 362748 [details]
Patch

r=me
Comment 9 WebKit Commit Bot 2019-02-22 20:18:04 PST
Comment on attachment 362748 [details]
Patch

Clearing flags on attachment: 362748

Committed r241982: <https://trac.webkit.org/changeset/241982>
Comment 10 WebKit Commit Bot 2019-02-22 20:18:06 PST
All reviewed patches have been landed.  Closing bug.