 Chromium Code Reviews
 Chromium Code Reviews Issue 1276143002:
  Defer media loading while on cellular networks.  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/blink.git@master
    
  
    Issue 1276143002:
  Defer media loading while on cellular networks.  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/blink.git@master| Index: third_party/WebKit/Source/core/html/HTMLMediaElement.cpp | 
| diff --git a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp | 
| index 614c6ce029563ab390b6395af1f1e283eed2f81d..849508e0dd1b3f314b3c15de7a6ce1a73713f187 100644 | 
| --- a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp | 
| +++ b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp | 
| @@ -66,6 +66,7 @@ | 
| #include "core/loader/FrameLoader.h" | 
| #include "core/loader/FrameLoaderClient.h" | 
| #include "core/page/ChromeClient.h" | 
| +#include "core/page/NetworkStateNotifier.h" | 
| #include "platform/ContentType.h" | 
| #include "platform/Logging.h" | 
| #include "platform/MIMETypeFromURL.h" | 
| @@ -1936,30 +1937,37 @@ void HTMLMediaElement::setPreload(const AtomicString& preload) | 
| WebMediaPlayer::Preload HTMLMediaElement::preloadType() const | 
| { | 
| const AtomicString& preload = fastGetAttribute(preloadAttr); | 
| + | 
| + WebMediaPlayer::Preload preloadType; | 
| if (equalIgnoringCase(preload, "none")) { | 
| UseCounter::count(document(), UseCounter::HTMLMediaElementPreloadNone); | 
| - return WebMediaPlayer::PreloadNone; | 
| - } | 
| - if (equalIgnoringCase(preload, "metadata")) { | 
| + preloadType = WebMediaPlayer::PreloadNone; | 
| + } else if (equalIgnoringCase(preload, "metadata")) { | 
| UseCounter::count(document(), UseCounter::HTMLMediaElementPreloadMetadata); | 
| - return WebMediaPlayer::PreloadMetaData; | 
| - } | 
| - if (equalIgnoringCase(preload, "auto")) { | 
| + preloadType = WebMediaPlayer::PreloadMetaData; | 
| + } else if (equalIgnoringCase(preload, "auto")) { | 
| UseCounter::count(document(), UseCounter::HTMLMediaElementPreloadAuto); | 
| - return WebMediaPlayer::PreloadAuto; | 
| - } | 
| + preloadType = WebMediaPlayer::PreloadAuto; | 
| + } else { | 
| + UseCounter::count(document(), UseCounter::HTMLMediaElementPreloadDefault); | 
| + | 
| + // "The attribute's missing value default is user-agent defined, though the | 
| + // Metadata state is suggested as a compromise between reducing server load | 
| + // and providing an optimal user experience." | 
| - // "The attribute's missing value default is user-agent defined, though the | 
| - // Metadata state is suggested as a compromise between reducing server load | 
| - // and providing an optimal user experience." | 
| + // The spec does not define an invalid value default: | 
| + // https://www.w3.org/Bugs/Public/show_bug.cgi?id=28950 | 
| + | 
| + // TODO(philipj): Try to make "metadata" the default preload state: | 
| + // https://crbug.com/310450 | 
| + preloadType = WebMediaPlayer::PreloadAuto; | 
| + } | 
| - // The spec does not define an invalid value default: | 
| - // https://www.w3.org/Bugs/Public/show_bug.cgi?id=28950 | 
| + // Force preload to none for cellular connections. | 
| + if (networkStateNotifier().connectionType() == WebConnectionTypeCellular) | 
| + preloadType = WebMediaPlayer::PreloadNone; | 
| 
philipj_slow
2015/09/30 11:19:30
Making this an early return to avoid even looking
 
DaleCurtis
2015/09/30 16:18:19
I didn't do that so that we could keep the use cou
 
philipj_slow
2015/10/01 09:49:06
Hmm, good point, but if they're counted without be
 | 
| - // TODO(philipj): Try to make "metadata" the default preload state: | 
| - // https://crbug.com/310450 | 
| - UseCounter::count(document(), UseCounter::HTMLMediaElementPreloadDefault); | 
| - return WebMediaPlayer::PreloadAuto; | 
| + return preloadType; | 
| } | 
| WebMediaPlayer::Preload HTMLMediaElement::effectivePreloadType() const |