These patches will reduce the priority of network requests for <link rel="prefetch"> and Flash plugin content. Honza, if you think this is a good idea, should I use the nsISupportsPriority interface (as used for Tracking Protection requests in bug 1141814) and/or nsIClassOfService's Throttleable flag (added in bug 1312745)? Would nsIClassOfService's Background or Speculative flags also be appropriate?

@Chris: you want Jan Bambas [:mayhemer] to do the review I guess. Honza

Comment on attachment 8834776 [details] Bug 1337672 - Part 1: Reduce the priority of prefetch network requests. https://reviewboard.mozilla.org/r/110618/#review111898

Comment on attachment 8834777 [details] Bug 1337672 - Part 2: Reduce the priority of Flash network requests. https://reviewboard.mozilla.org/r/110620/#review111900

(In reply to Jan Honza Odvarko [:Honza] from comment #3) > @Chris: you want Jan Bambas [:mayhemer] to do the review I guess. oops! Yes, thank you.

These patches will reduce the priority of network requests for <link rel="prefetch"> and Flash plugin content. Honza, if you think this is a good idea, should I use the nsISupportsPriority interface (as used for Tracking Protection requests in bug 1141814) and/or nsIClassOfService's Throttleable flag (added in bug 1312745)? Would nsIClassOfService's Background or Speculative flags also be appropriate? In my testing of my news sites linked from news.google.com, I only saw the following resources loaded using <link rel="prefetch">. None of them seem urgent. http://darkroom.baltimoresun.com/2017/01/preparing-for-the-trump-inauguration/ http://tpc.googlesyndication.com/safeframe/1-0-5/html/container.html https://a.disquscdn.com/next/embed/lounge.bundle.bc7675b3e7e1b4df8aad51f94abd7265.js https://assets.guim.co.uk/javascripts/e7ed4d0fceb8658f3ba7c1eb518ed2e1/app.js https://disqus.com/next/config.js https://ims-na1.adobelogin.com/favicon.ico?cache_bust=ddca24049ad2b8 https://securepubads.g.doubleclick.net/static/glade.js In contrast, I see Flash ad loaders on these news sites issuing nonstop requests to load new Flash ad images and XML.

I don't know enough about the effects of network prioritization to have much opinion about this. For example, are requests for the current tab always prioritized higher than background tabs? I wouldn't want requests from the current page's Flash (perhaps including video) to be prioritized lower than any load from other tabs. From an engineering/code-readability perspective, the values `nsISupportsPriority::PRIORITY_LOW + 3` are terrible and should really be new values on nsISupportsPriority, no? But overall I'm going to defer review to mayhemer, since I lack enough context.

Comment on attachment 8834777 [details] Bug 1337672 - Part 2: Reduce the priority of Flash network requests. https://reviewboard.mozilla.org/r/110620/#review112052

(In reply to Chris Peterson [:cpeterson] from comment #0) > These patches will reduce the priority of network requests for <link > rel="prefetch"> and Flash plugin content. > > Honza, if you think this is a good idea, should I use the > nsISupportsPriority interface (as used for Tracking Protection requests in > bug 1141814) Yes > and/or nsIClassOfService's Throttleable flag (added in bug > 1312745)? I more tend to say no, but this has no affect on prioritization. It only says to simply throttle download speed (by suspending the channel periodically) for those requests. At the moment it's not clear how well it's going to work, if at all, so I don't suggest using it. > Would nsIClassOfService's Background or Speculative flags also be > appropriate? You have to ask Patrick about these two.

(In reply to Benjamin Smedberg [:bsmedberg] from comment #10) > I don't know enough about the effects of network prioritization to have much > opinion about this. For example, are requests for the current tab always > prioritized higher than background tabs? No, but we are working on it! > I wouldn't want requests from the > current page's Flash (perhaps including video) to be prioritized lower than > any load from other tabs. Good argument. > > From an engineering/code-readability perspective, the values > `nsISupportsPriority::PRIORITY_LOW + 3` are terrible and should really be > new values on nsISupportsPriority, no? We have LOWEST too, I think? > > But overall I'm going to defer review to mayhemer, since I lack enough > context.

(In reply to Honza Bambas (:mayhemer) from comment #13) > (In reply to Benjamin Smedberg [:bsmedberg] from comment #10) > > I don't know enough about the effects of network prioritization to have much > > opinion about this. For example, are requests for the current tab always > > prioritized higher than background tabs? > > No, but we are working on it! The code that will favor the foreground tab's requests might need a special case to identify which Flash plugin requests come from Flash content on the foreground tab. Something to keep in mind. > > From an engineering/code-readability perspective, the values > > `nsISupportsPriority::PRIORITY_LOW + 3` are terrible and should really be > > new values on nsISupportsPriority, no? > > We have LOWEST too, I think? Yes, though there are nine priority levels available between PRIORITY_LOW (10) and PRIORITY_LOWEST (20).

Comment on attachment 8834776 [details] Bug 1337672 - Part 1: Reduce the priority of prefetch network requests. https://reviewboard.mozilla.org/r/110618/#review112494 ::: uriloader/prefetch/nsPrefetchService.cpp:164 (Diff revision 2) > > + // Reduce the priority of prefetch network requests. > + nsCOMPtr<nsISupportsPriority> priorityChannel = do_QueryInterface(mChannel); > + if (priorityChannel) { > + priorityChannel->AdjustPriority(nsISupportsPriority::PRIORITY_LOWEST); > + } so this class is actually used... :)

Comment on attachment 8834777 [details] Bug 1337672 - Part 2: Reduce the priority of Flash network requests. https://reviewboard.mozilla.org/r/110620/#review112496 So, with this patch... when I go to a porn site and want to see the video I found, you will deliberately delay it for me among other advertising and what ever stuff I'm definitely not interested in? The video (on any video providing site) with autoplay is the main content of interest. This patch IMO goes exactly against delivering that with added priority. I will r- this for now. I have to think first about how to tail this in the overall CDP picture. I think for some content we want to lower the priority (ads etc) but when the content is the one of most interest, we should add priority. Question is how to recognize this soon enough. Maybe the position and size of the player element given to some simple heuristic is enough? Or... is it worth to add a priority to Flash et al in times of HTLM5 players? ::: dom/base/nsObjectLoadingContent.cpp:2624 (Diff revision 2) > } > > + // Reduce the priority of network requests to load Flash <object> elements. > + nsCOMPtr<nsISupportsPriority> priorityChannel = do_QueryInterface(chan); > + if (priorityChannel) { > + priorityChannel->AdjustPriority(nsISupportsPriority::PRIORITY_LOW + 2); the LOW + 2 needs some comment to rationalize it. ::: dom/plugins/base/nsPluginHost.cpp:3272 (Diff revision 2) > } > + > + // Reduce the priority of network requests issued by Flash content. > + nsCOMPtr<nsISupportsPriority> priorityChannel = do_QueryInterface(channel); > + if (priorityChannel) { > + priorityChannel->AdjustPriority(nsISupportsPriority::PRIORITY_LOW + 3); as well here (probably in relation to the above)

(In reply to Honza Bambas (:mayhemer) from comment #16) > I will r- this for now. I have to think first about how to tail this in the > overall CDP picture. I think for some content we want to lower the priority > (ads etc) but when the content is the one of most interest, we should add > priority. > > Question is how to recognize this soon enough. Maybe the position and size > of the player element given to some simple heuristic is enough? I see what you mean. I will land the prefetch patch you r+'d and then close this bug for now. We can make a pretty good guess whether a Flash element is a video based on its size and aspect ratio. Chrome and Edge use similar heuristics to distinguish important from "non-essential" Flash (which they then make click-to-play). Some details about their heuristics: https://docs.google.com/document/d/1r4xFSsR4gtjBf1gOP4zHGWIFBV7WWZMgCiAHeepoHVw/ That said, we (and Chrome and Edge) plan to make most Flash content click-to-play over the next 6-12 months, so any Flash content that the user *does* load is likely to be something they want to see. So we might not need to bother adding any special logic to distinguish important from non-essential Flash. Flash video is still preferred over HTML5 video by some sites that use Adobe's DRM or analytics, such as HBO GO, Hulu, and CNN.

Pushed by cpeterson@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c8da87f54011 Part 1: Reduce the priority of prefetch network requests. r=mayhemer