Bug 158995 - Web Inspector: first heap snapshot taken when a page is reloaded happens before the reload navigation
Summary: Web Inspector: first heap snapshot taken when a page is reloaded happens befo...
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: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-06-21 11:52 PDT by Brian Burg
Modified: 2016-06-23 11:54 PDT (History)
6 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (19.90 KB, patch)
2016-06-22 14:51 PDT, Joseph Pecoraro
bburg: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Burg 2016-06-21 11:52:47 PDT
This is not so useful. If we can tell that a reload was initiated, maybe we should skip the initial snapshot or do it on the "load" event or DOMContentLoaded event.
Comment 1 Radar WebKit Bug Importer 2016-06-21 11:53:03 PDT
<rdar://problem/26923778>
Comment 2 Joseph Pecoraro 2016-06-22 14:51:20 PDT
Created attachment 281873 [details]
[PATCH] Proposed Fix
Comment 3 Joseph Pecoraro 2016-06-22 14:53:10 PDT
As a by-product, this fixes console.profile/profileEnd while the inspector frontend is open. Previously a programmatic start would have caused the frontend to send Timeline.start() and trick the backend into thinking "enabledByFrontend" which would cause the console.profileEnd to not stop the recording.

Now, the frontend doesn't send any Agent.start on a programmatic start.
Now, the frontend doesn't send any Agent.stop on a programmatic stop.

Which reduces traffic and just makes sense.
Comment 4 Brian Burg 2016-06-22 16:49:32 PDT
Comment on attachment 281873 [details]
[PATCH] Proposed Fix

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

Having trouble reviewing in review tool as it doesn't apply cleanly any more.

> Source/WebCore/ChangeLog:13
> +        AutoCapture happens on the backend happens when it is enabled

This sentence doesn't make sense.

> Source/WebInspectorUI/UserInterface/Models/TimelineRecording.js:119
> +    start(programmatic)

Can we call this initiatedByAPI or something, anything other than 'programmatic'?
Comment 5 Brian Burg 2016-06-23 11:08:30 PDT
Comment on attachment 281873 [details]
[PATCH] Proposed Fix

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

r=me

It would be nice to have a test for this, but unlikely to be worth it right now with the reload issues and other instability.

> Source/WebCore/ChangeLog:11
> +        until after the page does its first navigation.

s/does/commits/

> Source/WebCore/ChangeLog:38
> +        Update the auto capture phase progressions.

s/progressions/transitions

>> Source/WebInspectorUI/UserInterface/Models/TimelineRecording.js:119
>> +    start(programmatic)
> 
> Can we call this initiatedByAPI or something, anything other than 'programmatic'?

EDIT: I guess we have introduced this naming into the protocol already, so never mind. Still, I think it's rather vague and we should avoid it if possible. I prefer the initiatedByUser / initiatedByAPI dichotomy, which is similar to WebKit's userGesture terminology.
Comment 6 Joseph Pecoraro 2016-06-23 11:54:14 PDT
<http://trac.webkit.org/changeset/202384>