Log to UMA why we would load a third-party frame (issue 2437193002 by dgrogan@chromium.org)

[dgr...@chromium.org]

Reviewers: dcheng
CL: https://codereview.chromium.org/2437193002/

Message:
Looks like I shouldn't have been copying stack-allocated SimRequest objects.

Do you know why this only failed on win64 dbg? All other compiler configurations
optimize away the copy that returning from createMainResource() creates?

Description:
Log to UMA why we would load a third-party frame

This will give an estimate of the % of iframes that we can defer
loading.

We can defer offscreen frames unless they are probably used for
cross-origin communication.

Reland of https://codereview.chromium.org/2389023002

BUG=635105

Affected files (+271, -30 lines):
M third_party/WebKit/Source/core/dom/Document.h
M third_party/WebKit/Source/core/dom/Document.cpp
M third_party/WebKit/Source/core/frame/FrameView.cpp
M third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp
M third_party/WebKit/Source/web/BUILD.gn
A third_party/WebKit/Source/web/tests/DeferredLoadingTest.cpp
M tools/metrics/histograms/histograms.xml

[dch...@chromium.org]

On 2016/10/21 17:21:59, dgrogan wrote:
> Looks like I shouldn't have been copying stack-allocated SimRequest objects.
>
> Do you know why this only failed on win64 dbg? All other compiler
configurations
> optimize away the copy that returning from createMainResource() creates?

I guess so! LGTM

https://codereview.chromium.org/2437193002/

[commit-bot@chromium.org via chromiumcodereview-hr.appspot.com]

CQ is trying da patch. Follow status at

https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2437193002/20001


https://codereview.chromium.org/2437193002/

[dgr...@chromium.org]

TBR=isherman because it's a reland, histograms are identical to the
previously-approved patch

https://codereview.chromium.org/2437193002/

[dch...@chromium.org]

On 2016/10/21 17:35:59, dgrogan wrote:
> TBR=isherman because it's a reland, histograms are identical to the
> previously-approved patch

Actually, why does eliding this copy matter? What did the failures look like?

https://codereview.chromium.org/2437193002/

[dgr...@chromium.org]

This is the stack trace:
blink::KURL::operator= [0x076823E1+17]
blink::ResourceResponse::operator= [0x075A3823+19]
blink::WebURLResponse::operator= [0x072C6063+339]
blink::SimRequest::didReceiveResponse [0x008EBB89+41]
blink::SimNetwork::didReceiveResponse [0x008EA717+343]
blink::WebURLLoaderMock::ServeAsynchronousRequest [0x01597D6C+332]
blink::WebURLLoaderMockFactoryImpl::serveAsynchronousRequests [0x015961C7+471]
blink::SimNetwork::servePendingRequests [0x008EB612+50]
blink::SimRequest::start [0x008EBD6F+31]
blink::SimRequest::complete [0x008EBAEF+15]
blink::DeferredLoadingTest_Visible_Test::TestBody [0x0065360A+74]

I think it's because SimRequest's constructor statically adds itself to
SimNetwork:
SimNetwork::current().addRequest(*this);

So the first object that's created in createMainResource() adds itself and then
goes out of scope. But the SimNetwork isn't notified on destruction and later
tries to do stuff with the destroyed object.

https://codereview.chromium.org/2437193002/

[commit-bot@chromium.org via chromiumcodereview-hr.appspot.com]

Try jobs failed on following builders:
linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/321609)

https://codereview.chromium.org/2437193002/

[commit-bot@chromium.org via chromiumcodereview-hr.appspot.com]

[commit-bot@chromium.org via chromiumcodereview-hr.appspot.com]

Try jobs failed on following builders:
linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,

http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/321694)

https://codereview.chromium.org/2437193002/

[commit-bot@chromium.org via chromiumcodereview-hr.appspot.com]

[commit-bot@chromium.org via chromiumcodereview-hr.appspot.com]

Committed patchset #2 (id:20001)

https://codereview.chromium.org/2437193002/

[commit-bot@chromium.org via chromiumcodereview-hr.appspot.com]

Patchset 2 (id:??) landed as
https://crrev.com/c301da12f702e16e64faf04524f89bde02ad9588
Cr-Commit-Position: refs/heads/master@{#426979}

https://codereview.chromium.org/2437193002/