Bug 151112 – Web Inspector: Timeline Recording across page navigations behaves poorly

• Component: Web Inspector
• Status: RESOLVED FIXED

[Joseph Pecoraro]

* SUMMARY
Timeline Recording across page navigations behaves poorly.

* STEPS TO REPRODUCE
1. Inspect about:blank
2. Start timeline recording
3. Navigate to bogojoker.com
  => Timeline data briefly shows (incomplete network requests) but ends up with no data at all in the timeline

[Radar WebKit Bug Importer]

<rdar://problem/23484607>

[Joseph Pecoraro]

Created attachment 265203 [details]
[PATCH] Work in Progress

Attached is a work in progress that highlights the issue. InspectorPageAgent is reseting the stopwatch when the main frame loads. If we are recording a timeline this will mean suddenly timestamps have lower values then expected.

I'm not proposing this as a patch in and of itself because I think we may want to specifically handle PageAgent.reload. If a user is in the frontend and reloads during an auto-record, I think it should treat that as another new auto-record and not a sub-load within an active recording.

Also, I wonder what happens if we just never reset the stopwatch.

[Joseph Pecoraro]

> I'm not proposing this as a patch in and of itself because I think we may
> want to specifically handle PageAgent.reload. If a user is in the frontend
> and reloads during an auto-record, I think it should treat that as another
> new auto-record and not a sub-load within an active recording.

Actually, the frontend handles this correctly. It creates a new recording and it appears to make the initial timestamp the start timestamp.


> Also, I wonder what happens if we just never reset the stopwatch.

In a quick experiment this behaves correctly. I just have to audit the values to see how they are used.

[Joseph Pecoraro]

Created attachment 265207 [details]
[PATCH] Proposed Fix

Yeah, I didn't see any issues. The Network Tab uses a Network Timeline that the frontend resets on main page navigation, so that tab is conveniently all network requests made for the current page load and timestamps relative to the main resource. The Timeline itself shows all network events across multiple page loads.

There is some cleanup that can be done on the frontend for how engine initiated actions (layout, style recalc, etc) are displayed in the Timeline OverviewView (SourceCodeTimelines) but this is a big step forward in being able to debug multi-page timeline recordings.

[Timothy Hatcher]

Comment on attachment 265207 [details]
[PATCH] Proposed Fix

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

> Source/WebCore/inspector/InspectorPageAgent.cpp:-810
> -    if (frame.isMainFrame()) {
> -        auto stopwatch = m_environment.executionStopwatch();
> -        stopwatch->reset();
> -        stopwatch->start();
> -    }

Do we need to start the stopwatch if it isn't already started? I made this change to make timestamps work in the Network tab, when Timeline recoding isn't always happening. I suspect those timestamps are always zero now.

[Blaze Burg]

Comment on attachment 265207 [details]
[PATCH] Proposed Fix

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

>> Source/WebCore/inspector/InspectorPageAgent.cpp:-810
>> -    }
> 
> Do we need to start the stopwatch if it isn't already started? I made this change to make timestamps work in the Network tab, when Timeline recoding isn't always happening. I suspect those timestamps are always zero now.

I have a potential design fix for multi-navigation recordings, will discuss in person.

[Joseph Pecoraro]

Comment on attachment 265207 [details]
[PATCH] Proposed Fix

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

>>> Source/WebCore/inspector/InspectorPageAgent.cpp:-810
>>> -    }
>> 
>> Do we need to start the stopwatch if it isn't already started? I made this change to make timestamps work in the Network tab, when Timeline recoding isn't always happening. I suspect those timestamps are always zero now.
> 
> I have a potential design fix for multi-navigation recordings, will discuss in person.

InspectorPageAgent::enable resets and starts the stopwatch. As long as the domain is enabled everything will behave as expected.

[Joseph Pecoraro]

> I have a potential design fix for multi-navigation recordings, will discuss
> in person.

Is the patch above still suitable?

[WebKit Commit Bot]

Comment on attachment 265207 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 265207

Committed r192585: <http://trac.webkit.org/changeset/192585>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.