Propogate loading behavior data from Blink to content/

Propogate loading behavior data from Blink to content/

This patch adds an enum describing various loading metadata.
These bits are set when the Document hits various codepaths,
e.g. new loading features. The metadata is then propagated
into content/ for use by content consumers. The primary use
case for this new API is to aid in metrics collection.

This is part 1 of a 2 part patch set where the
page_load_metrics system will use the metadata to separate
separate histograms in the browser process.

The second patch can be found here:
https://codereview.chromium.org/1857443002/

BUG=594159
Committed: https://crrev.com/ae811ab72369dbbbb5bb3694c1fc618672099353
Cr-Commit-Position: refs/heads/master@{#386365}

[Charlie Harrison, 2016-04-01 17:53:18]

Description was changed from

==========
Propogate experimental from blink to content/ and consumers.

This patch propogates experimental flag data through blink to
RenderFrameObserver. This is part 1 of a 2 part patch set where the
page_load_metrics system will use the experimental flags to separate
separate histograms in the browser process.

BUG=
==========

to

==========
Propogate experimental from blink to content/ and consumers.

This patch propogates experimental flag data through blink to
RenderFrameObserver. This is part 1 of a 2 part patch set where the
page_load_metrics system will use the experimental flags to separate
separate histograms in the browser process.

BUG=594159
==========

[Charlie Harrison, 2016-04-01 17:55:58]

Description was changed from

==========
Propogate experimental from blink to content/ and consumers.

This patch propogates experimental flag data through blink to
RenderFrameObserver. This is part 1 of a 2 part patch set where the
page_load_metrics system will use the experimental flags to separate
separate histograms in the browser process.

BUG=594159
==========

to

==========
Propogate experiment data from blink to content/ and consumers.

This patch propogates experimental flag data through blink to
RenderFrameObserver. This is part 1 of a 2 part patch set where the
page_load_metrics system will use the experimental flags to separate
separate histograms in the browser process.

BUG=594159
==========

[Charlie Harrison, 2016-04-01 17:59:19]

csharrison@chromium.org changed reviewers:
+ esprehn@chromium.org, kinuko@chromium.org

[Charlie Harrison, 2016-04-01 17:59:19]

esprehn@ is the architecture of where WebExperimentData lives okay (i.e.
public/platform)?

@kinuko for general review and content/public change.

[Charlie Harrison, 2016-04-01 18:05:14]

cc jkarlin, bmcquade

[kinuko, 2016-04-04 09:40:58]

As I wrote in the other private thread the current interface that uses
'experiment' in the public API make me feel a bit nervouse (because for
propagating experiment flag we use different mechanism and one could be easily
confused what s/he should do when adding a new experiment if we start to have
this interface exposed at public layer)

[esprehn, 2016-04-04 15:25:31]

Why can't we just use uma inside blink? I'm not very comfortable having a new
enum that is really just listing runtime flags inside blink...

https://codereview.chromium.org/1846143003/diff/40001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebExperimentData.h (right):

https://codereview.chromium.org/1846143003/diff/40001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebExperimentData.h:15:
DocumentWriteEvaluatorExperiment = 1 << 0,
Having every feature we experiment with represented in a public enum doesn't
seem right.

[kinuko, 2016-04-04 15:41:55]

On 2016/04/04 15:25:31, esprehn wrote:
> Why can't we just use uma inside blink?

For page loading metrics we try to collect stats in an aggregated way in browser
process so that we can log things in a less noisy way with renderer
fast-shutdown / various abortions / tabs that are once backgrounded etc

> I'm not very comfortable having a new
> enum that is really just listing runtime flags inside blink...

For this one I agree with you (not feeling very comfortable with the current
change as is)
 
>
https://codereview.chromium.org/1846143003/diff/40001/third_party/WebKit/publ...
> File third_party/WebKit/public/platform/WebExperimentData.h (right):
> 
>
https://codereview.chromium.org/1846143003/diff/40001/third_party/WebKit/publ...
> third_party/WebKit/public/platform/WebExperimentData.h:15:
> DocumentWriteEvaluatorExperiment = 1 << 0,
> Having every feature we experiment with represented in a public enum doesn't
> seem right.

[Charlie Harrison, 2016-04-04 16:42:37]

I think the idea is a bit broader than runtime flags. Perhaps (as discussed in
another thread) the naming I used was not clear.

We want to propagate various features that a page uses up to chrome where we
have more context to log metrics. These features can be experimental (e.g. did
we use the doc write evaluator), or they can be stable (e.g. did the page use
@import in external css).

This is fundamentally different that whether a user is in an experimental group,
because it allows us to compare comparable page sets from users in experimental
groups and users in control groups. This is an alternative from the approach
that the stale-while-revalidate experiment took, which was to list a sample of
domains that used stale-while-revalidate in the chrome binary.

[esprehn, 2016-04-04 16:44:47]

Why is there more context in chrome? Do you mean the browser process? This patch
is just dead code right now since nothing uses the callback, can you give an
example of what will use it?

[Charlie Harrison, 2016-04-04 16:46:58]

Description was changed from

==========
Propogate experiment data from blink to content/ and consumers.

This patch propogates experimental flag data through blink to
RenderFrameObserver. This is part 1 of a 2 part patch set where the
page_load_metrics system will use the experimental flags to separate
separate histograms in the browser process.

BUG=594159
==========

to

==========
Propogate experiment data from blink to content/ and consumers.

This patch propogates experimental flag data through blink to
RenderFrameObserver. This is part 1 of a 2 part patch set where the
page_load_metrics system will use the experimental flags to separate
separate histograms in the browser process.

The second patch can be found here:
https://codereview.chromium.org/1857443002/

BUG=594159
==========

[Charlie Harrison, 2016-04-04 16:51:34]

On 2016/04/04 16:44:47, esprehn wrote:
> Why is there more context in chrome? Do you mean the browser process? This
patch
> is just dead code right now since nothing uses the callback, can you give an
> example of what will use it?

I updated the description to list the CL which uses this API.

Yeah meant the browser process when I said chrome. We decided to put page load
timing histograms in the browser process to avoid UMA data loss on fast
shutdown. The browser process also gives more context for things like page
aborts.

For instance, with this patch you could derive a metric which tracked abort
rates for pages that use a certain feature (e.g. document.write). This is harder
in blink which often cannot introspect it's own termination.

[kinuko, 2016-04-05 11:21:02]

Assuming that what we want to do here is to tell the browser the renderer
loading code took some code path,
- I'd make it clear that it's not really about experiments / experiment flags
- I wouldn't use the term 'feature' either as it could be again confusing as it
could sound like it'a dup of RuntimeEnabledFeature
- since we're basically going to use this only in loading context making it more
scoped would make the intention clearer / less controversial in general

didObserveLoadingBehavior(LoadingBehavior behavior) or something might sound
clearer?

[Charlie Harrison, 2016-04-05 13:28:36]

On 2016/04/05 11:21:02, kinuko wrote:
> Assuming that what we want to do here is to tell the browser the renderer
> loading code took some code path,
> - I'd make it clear that it's not really about experiments / experiment flags
> - I wouldn't use the term 'feature' either as it could be again confusing as
it
> could sound like it'a dup of RuntimeEnabledFeature
> - since we're basically going to use this only in loading context making it
more
> scoped would make the intention clearer / less controversial in general
> 
> didObserveLoadingBehavior(LoadingBehavior behavior) or something might sound
> clearer?

This SGTM. That's a clearer way to present the API and is more general. Thanks
for the suggestions.

I'll upload another patch set changing names / comments to.

[kinuko, 2016-04-05 16:07:30]

On 2016/04/05 13:28:36, csharrison wrote:
> On 2016/04/05 11:21:02, kinuko wrote:
> > Assuming that what we want to do here is to tell the browser the renderer
> > loading code took some code path,
> > - I'd make it clear that it's not really about experiments / experiment
flags
> > - I wouldn't use the term 'feature' either as it could be again confusing as
> it
> > could sound like it'a dup of RuntimeEnabledFeature
> > - since we're basically going to use this only in loading context making it
> more
> > scoped would make the intention clearer / less controversial in general
> > 
> > didObserveLoadingBehavior(LoadingBehavior behavior) or something might sound
> > clearer?
> 
> This SGTM. That's a clearer way to present the API and is more general. Thanks
> for the suggestions.
> 
> I'll upload another patch set changing names / comments to.

(You'll want to hear Elliott's comments too, as this patch needs his stamp)

[Charlie Harrison, 2016-04-05 16:39:02]

On 2016/04/05 16:07:30, kinuko wrote:
> On 2016/04/05 13:28:36, csharrison wrote:
> > On 2016/04/05 11:21:02, kinuko wrote:
> > > Assuming that what we want to do here is to tell the browser the renderer
> > > loading code took some code path,
> > > - I'd make it clear that it's not really about experiments / experiment
> flags
> > > - I wouldn't use the term 'feature' either as it could be again confusing
as
> > it
> > > could sound like it'a dup of RuntimeEnabledFeature
> > > - since we're basically going to use this only in loading context making
it
> > more
> > > scoped would make the intention clearer / less controversial in general
> > > 
> > > didObserveLoadingBehavior(LoadingBehavior behavior) or something might
sound
> > > clearer?
> > 
> > This SGTM. That's a clearer way to present the API and is more general.
Thanks
> > for the suggestions.
> > 
> > I'll upload another patch set changing names / comments to.
> 
> (You'll want to hear Elliott's comments too, as this patch needs his stamp)

Yep definitely :)

[Charlie Harrison, 2016-04-05 20:16:35]

Description was changed from

==========
Propogate experiment data from blink to content/ and consumers.

This patch propogates experimental flag data through blink to
RenderFrameObserver. This is part 1 of a 2 part patch set where the
page_load_metrics system will use the experimental flags to separate
separate histograms in the browser process.

The second patch can be found here:
https://codereview.chromium.org/1857443002/

BUG=594159
==========

to

==========
Propogate loading behavior data from blink to content/.

This patch adds an enum describing various loading metadata.
These bits are set when the Document hits various codepaths,
e.g. new loading features. The metadata is then propagated
into content/ for use by content consumers. The primary use
case for this new API is to aid in metrics collection.

This is part 1 of a 2 part patch set where the
page_load_metrics system will use the metadata to separate
separate histograms in the browser process.

The second patch can be found here:
https://codereview.chromium.org/1857443002/

BUG=594159
==========

[Charlie Harrison, 2016-04-05 20:16:47]

Description was changed from

==========
Propogate loading behavior data from blink to content/.

This patch adds an enum describing various loading metadata.
These bits are set when the Document hits various codepaths,
e.g. new loading features. The metadata is then propagated
into content/ for use by content consumers. The primary use
case for this new API is to aid in metrics collection.

This is part 1 of a 2 part patch set where the
page_load_metrics system will use the metadata to separate
separate histograms in the browser process.

The second patch can be found here:
https://codereview.chromium.org/1857443002/

BUG=594159
==========

to

==========
Propogate loading behavior data from Blink to content/

This patch adds an enum describing various loading metadata.
These bits are set when the Document hits various codepaths,
e.g. new loading features. The metadata is then propagated
into content/ for use by content consumers. The primary use
case for this new API is to aid in metrics collection.

This is part 1 of a 2 part patch set where the
page_load_metrics system will use the metadata to separate
separate histograms in the browser process.

The second patch can be found here:
https://codereview.chromium.org/1857443002/

BUG=594159
==========

[Charlie Harrison, 2016-04-05 22:26:06]

esprehn@, what do you think about this change, now that we've properly narrowed
the focus to loading behavior?

https://codereview.chromium.org/1846143003/diff/80001/content/public/renderer...
File content/public/renderer/render_frame_observer.h (right):

https://codereview.chromium.org/1846143003/diff/80001/content/public/renderer...
content/public/renderer/render_frame_observer.h:117: // load. This is used for
metrics collection.
"when the renderer"

https://codereview.chromium.org/1846143003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/FrameLoaderClient.h (right):

https://codereview.chromium.org/1846143003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/FrameLoaderClient.h:145: // propogates
renderer loading data to the browser for histograms.
"browser process"

[esprehn, 2016-04-05 22:32:16]

esprehn@chromium.org changed reviewers:
+ japhet@chromium.org

[esprehn, 2016-04-05 22:32:17]

This seems okay, though I'll say long term I think you want to move all that
code down into blink that you're plumbing out to in components/. If you want to
send an IPC to the browser related to loading you should just do it in core/
using mojo.

japhet@ is this okay with you?

https://codereview.chromium.org/1846143003/diff/80001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebLoadingData.h (right):

https://codereview.chromium.org/1846143003/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebLoadingData.h:15: enum WebLoadingData {
WebLoadingData sounds like a storage type, I think you want
WebLoadingBehaviorType ?

[Charlie Harrison, 2016-04-05 22:38:31]

On 2016/04/05 22:32:17, esprehn wrote:
> This seems okay, though I'll say long term I think you want to move all that
> code down into blink that you're plumbing out to in components/. If you want
to
> send an IPC to the browser related to loading you should just do it in core/
> using mojo.
> 
> japhet@ is this okay with you?
> 
>
https://codereview.chromium.org/1846143003/diff/80001/third_party/WebKit/publ...
> File third_party/WebKit/public/platform/WebLoadingData.h (right):
> 
>
https://codereview.chromium.org/1846143003/diff/80001/third_party/WebKit/publ...
> third_party/WebKit/public/platform/WebLoadingData.h:15: enum WebLoadingData {
> WebLoadingData sounds like a storage type, I think you want
> WebLoadingBehaviorType ?

Yeah, we are planning on eventually migrating the entire IPC system used by
page_load_metrics to blink and mojo. Hopefully we'll be able to get to that soon
and remove the plumbing through //content.

[kinuko, 2016-04-06 05:54:14]

Namings look still a bit inconsistent, but otherwise looks good to me

https://codereview.chromium.org/1846143003/diff/80001/content/public/renderer...
File content/public/renderer/render_frame_observer.h (right):

https://codereview.chromium.org/1846143003/diff/80001/content/public/renderer...
content/public/renderer/render_frame_observer.h:118: virtual void
DidObserveLoadingBehavior(blink::WebLoadingData data) {}
|data| sounds like a payload, let's consistently use 'behavior' throughout the
change.

WebLoadingData -> WebLoadingBehaviorFlag

data -> behavior

https://codereview.chromium.org/1846143003/diff/80001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebLoadingData.h (right):

https://codereview.chromium.org/1846143003/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebLoadingData.h:17:
DocumentWriteEvaluatorExperiment = 1 << 0,
We prefer having the same prefix for top-level enum names, e.g.

WebLoadingBehaviorNone,
WebLoadingBehaviorDocumentWriteEvaluator,
...


('NoLoadingData' sounds like we have no payload for the loading..)

[Charlie Harrison, 2016-04-06 14:24:52]

Thanks for the suggestions!

https://codereview.chromium.org/1846143003/diff/80001/content/public/renderer...
File content/public/renderer/render_frame_observer.h (right):

https://codereview.chromium.org/1846143003/diff/80001/content/public/renderer...
content/public/renderer/render_frame_observer.h:117: // load. This is used for
metrics collection.
On 2016/04/05 22:26:06, csharrison wrote:
> "when the renderer"

Done.

https://codereview.chromium.org/1846143003/diff/80001/content/public/renderer...
content/public/renderer/render_frame_observer.h:118: virtual void
DidObserveLoadingBehavior(blink::WebLoadingData data) {}
On 2016/04/06 05:54:14, kinuko wrote:
> |data| sounds like a payload, let's consistently use 'behavior' throughout the
> change.
> 
> WebLoadingData -> WebLoadingBehaviorFlag
> 
> data -> behavior

Done.

https://codereview.chromium.org/1846143003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/FrameLoaderClient.h (right):

https://codereview.chromium.org/1846143003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/FrameLoaderClient.h:145: // propogates
renderer loading data to the browser for histograms.
On 2016/04/05 22:26:06, csharrison wrote:
> "browser process"

Done.

https://codereview.chromium.org/1846143003/diff/80001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebLoadingData.h (right):

https://codereview.chromium.org/1846143003/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebLoadingData.h:15: enum WebLoadingData {
On 2016/04/05 22:32:17, esprehn wrote:
> WebLoadingData sounds like a storage type, I think you want
> WebLoadingBehaviorType ?

Changed to WebLoadingBehaviorFlag.

https://codereview.chromium.org/1846143003/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebLoadingData.h:17:
DocumentWriteEvaluatorExperiment = 1 << 0,
On 2016/04/06 05:54:14, kinuko wrote:
> We prefer having the same prefix for top-level enum names, e.g.
> 
> WebLoadingBehaviorNone,
> WebLoadingBehaviorDocumentWriteEvaluator,
> ...
> 
> 
> ('NoLoadingData' sounds like we have no payload for the loading..)
> 
> 
> 

Done.

https://codereview.chromium.org/1846143003/diff/120001/content/public/rendere...
File content/public/renderer/render_frame_observer.h (right):

https://codereview.chromium.org/1846143003/diff/120001/content/public/rendere...
content/public/renderer/render_frame_observer.h:116: // Notification when the
renderer uses an particular code path during a page
s/an/a/

[Nate Chapin, 2016-04-07 16:13:10]

LGTM

https://codereview.chromium.org/1846143003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp (right):

https://codereview.chromium.org/1846143003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:1166:
document()->loader()->didObserveLoadingBehavior(WebLoadingBehaviorFlag::WebLoadingBehaviorDocumentWriteEvaluator);
It feels a little weird to route this through DocumentLoader, but putting on
FrameLoader or Document or inline doesn't feel any better. :/

https://codereview.chromium.org/1846143003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/loader/DocumentLoader.cpp (right):

https://codereview.chromium.org/1846143003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/loader/DocumentLoader.cpp:215: if (frame() &&
frame()->isMainFrame() && m_state >= Committed) {
ASSERT(m_state >= Committed).

If there's a Document or a DocumentParser associated with the DocumentLoader, it
is by definition committed.

[Charlie Harrison, 2016-04-07 17:42:15]

The CQ bit was checked by csharrison@chromium.org

[Charlie Harrison, 2016-04-07 17:42:16]

https://codereview.chromium.org/1846143003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp (right):

https://codereview.chromium.org/1846143003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:1166:
document()->loader()->didObserveLoadingBehavior(WebLoadingBehaviorFlag::WebLoadingBehaviorDocumentWriteEvaluator);
On 2016/04/07 at 16:13:10, Nate Chapin wrote:
> It feels a little weird to route this through DocumentLoader, but putting on
FrameLoader or Document or inline doesn't feel any better. :/

Yeah it's a little awkward. I mostly routed through DocumentLoader because
that's how we propogate load timing to content/

https://codereview.chromium.org/1846143003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/loader/DocumentLoader.cpp (right):

https://codereview.chromium.org/1846143003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/loader/DocumentLoader.cpp:215: if (frame() &&
frame()->isMainFrame() && m_state >= Committed) {
On 2016/04/07 at 16:13:10, Nate Chapin wrote:
> ASSERT(m_state >= Committed).
> 
> If there's a Document or a DocumentParser associated with the DocumentLoader,
it is by definition committed.

Done.

[Charlie Harrison, 2016-04-07 17:42:16]

The patchset sent to the CQ was uploaded after l-g-t-m from japhet@chromium.org
Link to the patchset: https://codereview.chromium.org/1846143003/#ps140001
(title: "japhet@ review")

[Charlie Harrison, 2016-04-07 17:42:25]

Thanks!

esprehn@, kinuko@, PTAL.

[commit-bot: I haz the power, 2016-04-07 17:43:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1846143003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1846143003/140001

[Charlie Harrison, 2016-04-07 17:44:08]

The CQ bit was unchecked by csharrison@chromium.org

[kinuko, 2016-04-11 08:39:25]

lgtm/2

[kinuko, 2016-04-11 08:39:34]

The CQ bit was checked by kinuko@chromium.org

[commit-bot: I haz the power, 2016-04-11 08:39:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1846143003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1846143003/140001

[commit-bot: I haz the power, 2016-04-11 11:32:38]

Description was changed from

==========
Propogate loading behavior data from Blink to content/

This patch adds an enum describing various loading metadata.
These bits are set when the Document hits various codepaths,
e.g. new loading features. The metadata is then propagated
into content/ for use by content consumers. The primary use
case for this new API is to aid in metrics collection.

This is part 1 of a 2 part patch set where the
page_load_metrics system will use the metadata to separate
separate histograms in the browser process.

The second patch can be found here:
https://codereview.chromium.org/1857443002/

BUG=594159
==========

to

==========
Propogate loading behavior data from Blink to content/

This patch adds an enum describing various loading metadata.
These bits are set when the Document hits various codepaths,
e.g. new loading features. The metadata is then propagated
into content/ for use by content consumers. The primary use
case for this new API is to aid in metrics collection.

This is part 1 of a 2 part patch set where the
page_load_metrics system will use the metadata to separate
separate histograms in the browser process.

The second patch can be found here:
https://codereview.chromium.org/1857443002/

BUG=594159
==========

[commit-bot: I haz the power, 2016-04-11 11:32:39]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-04-11 11:34:32]

Description was changed from

==========
Propogate loading behavior data from Blink to content/

This patch adds an enum describing various loading metadata.
These bits are set when the Document hits various codepaths,
e.g. new loading features. The metadata is then propagated
into content/ for use by content consumers. The primary use
case for this new API is to aid in metrics collection.

This is part 1 of a 2 part patch set where the
page_load_metrics system will use the metadata to separate
separate histograms in the browser process.

The second patch can be found here:
https://codereview.chromium.org/1857443002/

BUG=594159
==========

to

==========
Propogate loading behavior data from Blink to content/

This patch adds an enum describing various loading metadata.
These bits are set when the Document hits various codepaths,
e.g. new loading features. The metadata is then propagated
into content/ for use by content consumers. The primary use
case for this new API is to aid in metrics collection.

This is part 1 of a 2 part patch set where the
page_load_metrics system will use the metadata to separate
separate histograms in the browser process.

The second patch can be found here:
https://codereview.chromium.org/1857443002/

BUG=594159

Committed: https://crrev.com/ae811ab72369dbbbb5bb3694c1fc618672099353
Cr-Commit-Position: refs/heads/master@{#386365}
==========

[commit-bot: I haz the power, 2016-04-11 11:34:34]

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

[Łukasz Anforowicz, 2016-04-11 22:01:19]

On 2016/04/11 11:34:34, commit-bot: I haz the power wrote:
> Patchset 8 (id:??) landed as
> https://crrev.com/ae811ab72369dbbbb5bb3694c1fc618672099353
> Cr-Commit-Position: refs/heads/master@{#386365}

After this CL mixed content layout tests started to behave flakily - please see
https://crbug.com/602421