Make the low priority iframe flag affect all resources in cross origin iframes

Make the low priority iframe flag affect all resources in cross origin iframes

Currently it affects all iframes, but only the main resource.

BUG=

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201843

[Nate Chapin, 2015-09-02 23:37:02]

The CQ bit was checked by japhet@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-02 23:37:13]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1328763002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1328763002/1

[commit-bot: I haz the power, 2015-09-03 00:40:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-03 00:40:41]

Dry run: This issue passed the CQ dry run.

[Nate Chapin, 2015-09-03 16:32:27]

japhet@chromium.org changed reviewers:
+ bmcquade@chromium.org, ojan@chromium.org, pmeenan@chromium.org

[Pat Meenan, 2015-09-03 16:45:22]

On 2015/09/03 16:32:28, Nate Chapin wrote:

LGTM - though there's a good chance this still won't catch a lot of ads.  Ads
tend to be loaded in friendly iFrames with access to the main content page to
allow for expanding/rich formats:
http://www.iab.net/media/file/rich_media_ajax_best_practices.pdf

[Bryan McQuade, 2015-09-03 17:03:15]

If possible, we should try to catch the resources loaded in those friendly
iframes as well. Can do that in a separate CL if that makes sense.

On Thu, Sep 3, 2015 at 12:45 PM <pmeenan@chromium.org> wrote:

> On 2015/09/03 16:32:28, Nate Chapin wrote:
>
> LGTM - though there's a good chance this still won't catch a lot of ads.
> Ads
> tend to be loaded in friendly iFrames with access to the main content page
> to
> allow for expanding/rich formats:
> http://www.iab.net/media/file/rich_media_ajax_best_practices.pdf
>
> https://codereview.chromium.org/1328763002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Nate Chapin, 2015-09-03 18:12:42]

How would we differentiate these same-origin ad iframes from other same-origin
iframes? Or would any policy have to apply to all iframes to some degree?

On 2015/09/03 17:03:15, Bryan McQuade wrote:
> If possible, we should try to catch the resources loaded in those friendly
> iframes as well. Can do that in a separate CL if that makes sense.
> 
> On Thu, Sep 3, 2015 at 12:45 PM <mailto:pmeenan@chromium.org> wrote:
> 
> > On 2015/09/03 16:32:28, Nate Chapin wrote:
> >
> > LGTM - though there's a good chance this still won't catch a lot of ads.
> > Ads
> > tend to be loaded in friendly iFrames with access to the main content page
> > to
> > allow for expanding/rich formats:
> > http://www.iab.net/media/file/rich_media_ajax_best_practices.pdf
> >
> > https://codereview.chromium.org/1328763002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:blink-reviews+unsubscribe@chromium.org.
\

[blink-reviews, 2015-09-03 18:18:13]

I'm thinking all iFrames.  Not sure how much something like gmail uses
frames but for most content sites the frames are usually the ads and social
buttons.

On Thu, Sep 3, 2015 at 2:12 PM, <japhet@chromium.org> wrote:

> How would we differentiate these same-origin ad iframes from other
> same-origin
> iframes? Or would any policy have to apply to all iframes to some degree?
>
> On 2015/09/03 17:03:15, Bryan McQuade wrote:
>
>> If possible, we should try to catch the resources loaded in those friendly
>> iframes as well. Can do that in a separate CL if that makes sense.
>>
>
> On Thu, Sep 3, 2015 at 12:45 PM <mailto:pmeenan@chromium.org> wrote:
>>
>
> > On 2015/09/03 16:32:28, Nate Chapin wrote:
>> >
>> > LGTM - though there's a good chance this still won't catch a lot of ads.
>> > Ads
>> > tend to be loaded in friendly iFrames with access to the main content
>> page
>> > to
>> > allow for expanding/rich formats:
>> > http://www.iab.net/media/file/rich_media_ajax_best_practices.pdf
>> >
>> > https://codereview.chromium.org/1328763002/
>> >
>>
>
> To unsubscribe from this group and stop receiving emails from it, send an
>>
> email
>
>> to mailto:blink-reviews+unsubscribe@chromium.org.
>>
> \
>
> https://codereview.chromium.org/1328763002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Nate Chapin, 2015-09-03 18:34:44]

Looks like gmail has ~15 iframes, ~3 of which are same origin.

At that point, are you suggesting that we just deprioritize every resource in
every iframe?

On 2015/09/03 18:18:13, blink-reviews wrote:
> I'm thinking all iFrames.  Not sure how much something like gmail uses
> frames but for most content sites the frames are usually the ads and social
> buttons.
> 
> On Thu, Sep 3, 2015 at 2:12 PM, <mailto:japhet@chromium.org> wrote:
> 
> > How would we differentiate these same-origin ad iframes from other
> > same-origin
> > iframes? Or would any policy have to apply to all iframes to some degree?
> >
> > On 2015/09/03 17:03:15, Bryan McQuade wrote:
> >
> >> If possible, we should try to catch the resources loaded in those friendly
> >> iframes as well. Can do that in a separate CL if that makes sense.
> >>
> >
> > On Thu, Sep 3, 2015 at 12:45 PM <mailto:pmeenan@chromium.org> wrote:
> >>
> >
> > > On 2015/09/03 16:32:28, Nate Chapin wrote:
> >> >
> >> > LGTM - though there's a good chance this still won't catch a lot of ads.
> >> > Ads
> >> > tend to be loaded in friendly iFrames with access to the main content
> >> page
> >> > to
> >> > allow for expanding/rich formats:
> >> > http://www.iab.net/media/file/rich_media_ajax_best_practices.pdf
> >> >
> >> > https://codereview.chromium.org/1328763002/
> >> >
> >>
> >
> > To unsubscribe from this group and stop receiving emails from it, send an
> >>
> > email
> >
> >> to mailto:blink-reviews+unsubscribe@chromium.org.
> >>
> > \
> >
> > https://codereview.chromium.org/1328763002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:blink-reviews+unsubscribe@chromium.org.

[blink-reviews, 2015-09-03 18:37:35]

Optimally we'd do it for frames that aren't in the viewport, but yes -
pretty much saying that we deprioritize all content in frames and treat
them like images (since they largely have the same content-blocking
behavior as images).

On Thu, Sep 3, 2015 at 2:34 PM, <japhet@chromium.org> wrote:

> Looks like gmail has ~15 iframes, ~3 of which are same origin.
>
> At that point, are you suggesting that we just deprioritize every resource
> in
> every iframe?
>
> On 2015/09/03 18:18:13, blink-reviews wrote:
>
>> I'm thinking all iFrames.  Not sure how much something like gmail uses
>> frames but for most content sites the frames are usually the ads and
>> social
>> buttons.
>>
>
> On Thu, Sep 3, 2015 at 2:12 PM, <mailto:japhet@chromium.org> wrote:
>>
>
> > How would we differentiate these same-origin ad iframes from other
>> > same-origin
>> > iframes? Or would any policy have to apply to all iframes to some
>> degree?
>> >
>> > On 2015/09/03 17:03:15, Bryan McQuade wrote:
>> >
>> >> If possible, we should try to catch the resources loaded in those
>> friendly
>> >> iframes as well. Can do that in a separate CL if that makes sense.
>> >>
>> >
>> > On Thu, Sep 3, 2015 at 12:45 PM <mailto:pmeenan@chromium.org> wrote:
>> >>
>> >
>> > > On 2015/09/03 16:32:28, Nate Chapin wrote:
>> >> >
>> >> > LGTM - though there's a good chance this still won't catch a lot of
>> ads.
>> >> > Ads
>> >> > tend to be loaded in friendly iFrames with access to the main content
>> >> page
>> >> > to
>> >> > allow for expanding/rich formats:
>> >> > http://www.iab.net/media/file/rich_media_ajax_best_practices.pdf
>> >> >
>> >> > https://codereview.chromium.org/1328763002/
>> >> >
>> >>
>> >
>> > To unsubscribe from this group and stop receiving emails from it, send
>> an
>> >>
>> > email
>> >
>> >> to mailto:blink-reviews+unsubscribe@chromium.org.
>> >>
>> > \
>> >
>> > https://codereview.chromium.org/1328763002/
>> >
>>
>
> To unsubscribe from this group and stop receiving emails from it, send an
>>
> email
>
>> to mailto:blink-reviews+unsubscribe@chromium.org.
>>
>
>
>
> https://codereview.chromium.org/1328763002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Nate Chapin, 2015-09-03 21:33:24]

Updated to drop priority for all resources in all iframes.

Should that policy apply after the main frame is done loading?

[blink-reviews, 2015-09-03 22:37:31]

Shouldn't matter once the main frame is done loading though it's probably
not worth any huge efforts to communicate that knowledge around.  The
loading of most iframes will block the parent's load event anyway so it
would be for any dynamic frames or resources injected after load and there
should be minimal contention in that case anyway.

On Thu, Sep 3, 2015 at 5:33 PM, <japhet@chromium.org> wrote:

> Updated to drop priority for all resources in all iframes.
>
> Should that policy apply after the main frame is done loading?
>
> https://codereview.chromium.org/1328763002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Bryan McQuade, 2015-09-04 16:40:28]

LGTM

https://codereview.chromium.org/1328763002/diff/20001/Source/core/loader/Fram...
File Source/core/loader/FrameFetchContextTest.cpp (right):

https://codereview.chromium.org/1328763002/diff/20001/Source/core/loader/Fram...
Source/core/loader/FrameFetchContextTest.cpp:64: Frame* top() const override {
return m_parent.get(); }
looks like this isn't used - can it be removed?

[Nate Chapin, 2015-09-04 21:52:32]

https://codereview.chromium.org/1328763002/diff/20001/Source/core/loader/Fram...
File Source/core/loader/FrameFetchContextTest.cpp (right):

https://codereview.chromium.org/1328763002/diff/20001/Source/core/loader/Fram...
Source/core/loader/FrameFetchContextTest.cpp:64: Frame* top() const override {
return m_parent.get(); }
On 2015/09/04 16:40:28, Bryan McQuade wrote:
> looks like this isn't used - can it be removed?

Done.

[Nate Chapin, 2015-09-04 21:52:35]

The CQ bit was checked by japhet@chromium.org

[Nate Chapin, 2015-09-04 21:52:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
pmeenan@chromium.org, bmcquade@chromium.org
Link to the patchset: https://codereview.chromium.org/1328763002/#ps40001
(title: "Rebase + remove unused top() override")

[commit-bot: I haz the power, 2015-09-04 21:52:41]

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

[commit-bot: I haz the power, 2015-09-04 21:52:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-04 21:52:44]

No L-G-T-M from a valid reviewer yet. Only full committers are accepted.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[ojan, 2015-09-05 03:08:12]

The CQ bit was checked by ojan@chromium.org

[ojan, 2015-09-05 03:08:13]

lgtm

[commit-bot: I haz the power, 2015-09-05 03:08:23]

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

[commit-bot: I haz the power, 2015-09-05 04:13:50]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201843