Bug 170525 – Web Inspector: Only Capture Extra Network Load Metrics when there is a Web Inspector Frontend

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

Summary:
Only Capture Extra Network Load Metrics when there is a Web Inspector Frontend

To limit any potential performance impact from making another copy of all HTTP Headers, only do this work in the NetworkProcess if the WebProcess has a Web Inspector frontend open.

We can extend this to a few other Metrics properties as well.

[Joseph Pecoraro]

Created attachment 306328 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Created attachment 306329 [details]
[PATCH] For Bots (All Parts)

[Joseph Pecoraro]

This prevents copying of headers in NetworkProcess, Serializing across to the WebProcess, and potentially copying of the HTTPHeaderMap in the WebProcess. It also avoids setting a few other properties only needed for Web Inspector (and a few others are coming).

[Joseph Pecoraro]

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

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

> Source/WebKit2/NetworkProcess/NetworkLoad.h:73
> +    bool shouldCaptureExtraNetworkLoadMetrics() const final;

This needs to be inside of a USE(NETWORK_SESSION) block. Fixed locally.

[Joseph Pecoraro]

Created attachment 306333 [details]
[PATCH] For Bots (All Parts)

[Joseph Pecoraro]

Created attachment 306337 [details]
[PATCH] For Bots (All Parts)

A few more USE(NETWORK_SESSION)

[Joseph Pecoraro]

Created attachment 306342 [details]
[PATCH] For Bots (All Parts)

[Build Bot]

Comment on attachment 306342 [details]
[PATCH] For Bots (All Parts)

Attachment 306342 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3482094

New failing tests:
http/tests/cache/disk-cache/speculative-validation/http-auth.html

[Build Bot]

Created attachment 306356 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Joseph Pecoraro]

Ping reviewers.

[youenn fablet]

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

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

> Source/WebCore/inspector/InspectorInstrumentation.h:1357
> +    if (s_frontendCounter == 1)

one liner?

> Source/WebCore/inspector/InspectorInstrumentation.h:1365
> +    if (!s_frontendCounter)

one liner?

> Source/WebKit2/NetworkProcess/cocoa/NetworkSessionCocoa.mm:315
> +                requestHeaders.set(String(name), String(value));

Might want to consider sending only the modified headers if this can be easily done.

[youenn fablet]

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

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

> Source/WebCore/inspector/InspectorInstrumentation.h:1357
> +    if (s_frontendCounter == 1)

one liner?

> Source/WebCore/inspector/InspectorInstrumentation.h:1365
> +    if (!s_frontendCounter)

one liner?

> Source/WebKit2/NetworkProcess/cocoa/NetworkSessionCocoa.mm:315
> +                requestHeaders.set(String(name), String(value));

Might want to consider sending only the modified headers if this can be easily done.

[youenn fablet]

(In reply to Joseph Pecoraro from comment #10)
> Ping reviewers.

Can you rebase it?

[Joseph Pecoraro]

(In reply to youenn fablet from comment #13)
> (In reply to Joseph Pecoraro from comment #10)
> > Ping reviewers.
> 
> Can you rebase it?

It depends on a patch that hasn't landed yet (hence the "All Parts" patch). But I'll go ahead and land the earlier part and put up a new patch.

[Joseph Pecoraro]

> > Source/WebCore/inspector/InspectorInstrumentation.h:1357
> > +    if (s_frontendCounter == 1)
> 
> one liner?
> 
> > Source/WebCore/inspector/InspectorInstrumentation.h:1365
> > +    if (!s_frontendCounter)
> 
> one liner?

The compiler should optimize this, I aimed for the code to have maximum readability / clarity.

[Joseph Pecoraro]

Created attachment 306431 [details]
[PATCH] Proposed Fix

[WebKit Commit Bot]

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

Clearing flags on attachment: 306431

Committed r215065: <http://trac.webkit.org/changeset/215065>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.