Bug 194956

Summary: Web Inspector: Timelines: add UI for preventing auto-stop
Product: WebKit Reporter: Devin Rousso <hi>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hi, inspector-bugzilla-changes, joepeck, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 194793    
Bug Blocks: 195192    
Attachments:
Description Flags
Patch
none
Patch
none
[Image] After Patch is applied none

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.