Media: Delete Pause-To-Buffer.

This code was broken in M52, but nobody has complained, so remove it.

Pause-To-Buffer is a consistent source of surprise, and mistakes leading
to accidental bandwidth consumption. The new download button is a much
better interface (at least in cases where it appears).

Review-Url: https://codereview.chromium.org/2689323002
Cr-Commit-Position: refs/heads/master@{#460563}

media/blink/multibuffer_data_source.cc

@@ -122,7 +122,6 @@ MultibufferDataSource::MultibufferDataSource(
       frame_(frame),
       stop_signal_received_(false),
       media_has_played_(false),
-      buffering_strategy_(BUFFERING_STRATEGY_NORMAL),
       single_origin_(true),
       cancel_on_defer_(false),
       preload_(AUTO),
@@ -249,13 +248,6 @@ void MultibufferDataSource::SetPreload(Preload preload) {
   UpdateBufferSizes();
 }
 
-void MultibufferDataSource::SetBufferingStrategy(
-    BufferingStrategy buffering_strategy) {
-  DCHECK(render_task_runner_->BelongsToCurrentThread());
-  buffering_strategy_ = buffering_strategy;
-  UpdateBufferSizes();
-}
-
 bool MultibufferDataSource::HasSingleOrigin() {
   DCHECK(render_task_runner_->BelongsToCurrentThread());
   // Before initialization completes there is no risk of leaking data. Callers
@@ -580,18 +572,6 @@ void MultibufferDataSource::UpdateBufferSizes() {
   if (!reader_)
     return;
 
-  if (!assume_fully_buffered()) {
-    // If the playback has started and the strategy is aggressive, then try to
-    // load as much as possible, assuming that the file is cacheable. (If not,
-    // why bother?)
-    bool aggressive = (buffering_strategy_ == BUFFERING_STRATEGY_AGGRESSIVE);
-    if (media_has_played_ && aggressive && url_data_ &&
-        url_data_->range_supported() && url_data_->cacheable()) {
-      reader_->SetPreload(1LL << 40, 1LL << 40);  // 1 Tb
-      return;
-    }
-  }
-
   // Use a default bit rate if unknown and clamp to prevent overflow.
   int64_t bitrate = clamp<int64_t>(bitrate_, 0, kMaxBitrate);
   if (bitrate == 0)

media/blink/multibuffer_data_source.h

@@ -53,14 +53,6 @@ class MEDIA_BLINK_EXPORT MultibufferDataSource : public DataSource {
     AUTO,
   };
 
-  // Enum values must match the values in
-  // blink::WebMediaPlayer::BufferingStrategy and there will be assertions at
-  // compile time if they do not match.
-  enum BufferingStrategy {
-    BUFFERING_STRATEGY_NORMAL,
-    BUFFERING_STRATEGY_AGGRESSIVE,
-  };
-
   // |url| and |cors_mode| are passed to the object. Buffered byte range changes
   // will be reported to |host|. |downloading_cb| will be called whenever the
   // downloading/paused state of the source changes.
@@ -84,10 +76,6 @@ class MEDIA_BLINK_EXPORT MultibufferDataSource : public DataSource {
   // Adjusts the buffering algorithm based on the given preload value.
   void SetPreload(Preload preload);
 
-  // Adjusts the buffering algorithm based on the given buffering strategy
-  // value.
-  void SetBufferingStrategy(BufferingStrategy buffering_strategy);
-
   // Returns true if the media resource has a single origin, false otherwise.
   // Only valid to call after Initialize() has completed.
   //
@@ -226,9 +214,6 @@ class MEDIA_BLINK_EXPORT MultibufferDataSource : public DataSource {
   // least once.
   bool media_has_played_;
 
-  // Buffering strategy set by SetBufferingStrategy.
-  BufferingStrategy buffering_strategy_;
-
   // As we follow redirects, we set this variable to false if redirects
   // go between different origins.
   bool single_origin_;

media/blink/multibuffer_data_source_unittest.cc

@@ -1109,10 +1109,6 @@ TEST_F(MultibufferDataSourceTest, LocalResource_DeferStrategy) {
   data_source_->MediaIsPlaying();
   CheckCapacityDefer();
 
-  data_source_->SetBufferingStrategy(
-      MultibufferDataSource::BUFFERING_STRATEGY_AGGRESSIVE);
-  CheckCapacityDefer();
-
   Stop();
 }
 
@@ -1127,10 +1123,6 @@ TEST_F(MultibufferDataSourceTest, LocalResource_PreloadMetadata_DeferStrategy) {
   data_source_->MediaIsPlaying();
   CheckCapacityDefer();
 
-  data_source_->SetBufferingStrategy(
-      MultibufferDataSource::BUFFERING_STRATEGY_AGGRESSIVE);
-  CheckCapacityDefer();
-
   Stop();
 }
 
@@ -1145,10 +1137,6 @@ TEST_F(MultibufferDataSourceTest, ExternalResource_Reponse200_DeferStrategy) {
   data_source_->MediaIsPlaying();
   CheckCapacityDefer();
 
-  data_source_->SetBufferingStrategy(
-      MultibufferDataSource::BUFFERING_STRATEGY_AGGRESSIVE);
-  CheckCapacityDefer();
-
   Stop();
 }
 
@@ -1165,10 +1153,6 @@ TEST_F(MultibufferDataSourceTest,
   data_source_->MediaIsPlaying();
   CheckCapacityDefer();
 
-  data_source_->SetBufferingStrategy(
-      MultibufferDataSource::BUFFERING_STRATEGY_AGGRESSIVE);
-  CheckCapacityDefer();
-
   Stop();
 }
 
@@ -1182,21 +1166,11 @@ TEST_F(MultibufferDataSourceTest, ExternalResource_Reponse206_DeferStrategy) {
 
   data_source_->MediaIsPlaying();
   CheckCapacityDefer();
-  set_might_be_reused_from_cache_in_future(true);
-  data_source_->SetBufferingStrategy(
-      MultibufferDataSource::BUFFERING_STRATEGY_AGGRESSIVE);
-  CheckNeverDefer();
 
-  data_source_->SetBufferingStrategy(
-      MultibufferDataSource::BUFFERING_STRATEGY_NORMAL);
+  set_might_be_reused_from_cache_in_future(true);
   data_source_->MediaIsPlaying();
   CheckCapacityDefer();
 
-  set_might_be_reused_from_cache_in_future(false);
-  data_source_->SetBufferingStrategy(
-      MultibufferDataSource::BUFFERING_STRATEGY_AGGRESSIVE);
-  CheckCapacityDefer();
-
   Stop();
 }
 
@@ -1214,17 +1188,10 @@ TEST_F(MultibufferDataSourceTest,
   CheckCapacityDefer();
 
   set_might_be_reused_from_cache_in_future(true);
-  data_source_->SetBufferingStrategy(
-      MultibufferDataSource::BUFFERING_STRATEGY_AGGRESSIVE);
-  CheckNeverDefer();
-
-  data_source_->SetBufferingStrategy(
-      MultibufferDataSource::BUFFERING_STRATEGY_NORMAL);
   data_source_->MediaIsPlaying();
   CheckCapacityDefer();
+
   set_might_be_reused_from_cache_in_future(false);
-  data_source_->SetBufferingStrategy(
-      MultibufferDataSource::BUFFERING_STRATEGY_AGGRESSIVE);
   CheckCapacityDefer();
 
   Stop();

media/blink/webmediaplayer_impl.cc

@@ -181,7 +181,6 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
       ready_state_(WebMediaPlayer::ReadyStateHaveNothing),
       highest_ready_state_(WebMediaPlayer::ReadyStateHaveNothing),
       preload_(MultibufferDataSource::AUTO),
-      buffering_strategy_(MultibufferDataSource::BUFFERING_STRATEGY_NORMAL),
       main_task_runner_(frame->loadingTaskRunner()),
       media_task_runner_(params.media_task_runner()),
       worker_task_runner_(params.worker_task_runner()),
@@ -419,7 +418,6 @@ void WebMediaPlayerImpl::DoLoad(LoadType load_type,
         url_index_, frame_, media_log_.get(), &buffered_data_source_host_,
         base::Bind(&WebMediaPlayerImpl::NotifyDownloading, AsWeakPtr())));
     data_source_->SetPreload(preload_);
-    data_source_->SetBufferingStrategy(buffering_strategy_);
     data_source_->Initialize(
         base::Bind(&WebMediaPlayerImpl::DataSourceInitialized, AsWeakPtr()));
   }
@@ -638,31 +636,6 @@ void WebMediaPlayerImpl::setPreload(WebMediaPlayer::Preload preload) {
     data_source_->SetPreload(preload_);
 }
 
-STATIC_ASSERT_ENUM(WebMediaPlayer::BufferingStrategy::Normal,
-                   MultibufferDataSource::BUFFERING_STRATEGY_NORMAL);
-STATIC_ASSERT_ENUM(WebMediaPlayer::BufferingStrategy::Aggressive,
-                   MultibufferDataSource::BUFFERING_STRATEGY_AGGRESSIVE);
-
-void WebMediaPlayerImpl::setBufferingStrategy(
-    WebMediaPlayer::BufferingStrategy buffering_strategy) {
-  DVLOG(1) << __func__;
-  DCHECK(main_task_runner_->BelongsToCurrentThread());
-
-#if defined(OS_ANDROID)
-  // We disallow aggressive buffering on Android since it matches the behavior
-  // of the platform media player and may have data usage penalties.
-  // TODO(dalecurtis, hubbe): We should probably stop using "pause-and-buffer"
-  // everywhere. See http://crbug.com/594669 for more details.
-  buffering_strategy_ = MultibufferDataSource::BUFFERING_STRATEGY_NORMAL;
-#else
-  buffering_strategy_ =
-      static_cast<MultibufferDataSource::BufferingStrategy>(buffering_strategy);
-#endif
-
-  if (data_source_)
-    data_source_->SetBufferingStrategy(buffering_strategy_);
-}
-
 bool WebMediaPlayerImpl::hasVideo() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
@@ -1153,13 +1126,10 @@ void WebMediaPlayerImpl::OnPipelineSuspended() {
   }
 #endif
 
-  // If we're not in an aggressive buffering state, tell the data source we have
-  // enough data so that it may release the connection.
-  if (buffering_strategy_ !=
-      MultibufferDataSource::BUFFERING_STRATEGY_AGGRESSIVE) {
-    if (data_source_)
-      data_source_->OnBufferingHaveEnough(true);
-  }
+  // Tell the data source we have enough data so that it may release the
+  // connection.
+  if (data_source_)
+    data_source_->OnBufferingHaveEnough(true);
 
   ReportMemoryUsage();
 

media/blink/webmediaplayer_impl.h

@@ -118,8 +118,6 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
                  const blink::WebSecurityOrigin& security_origin,
                  blink::WebSetSinkIdCallbacks* web_callback) override;
   void setPreload(blink::WebMediaPlayer::Preload preload) override;
-  void setBufferingStrategy(
-      blink::WebMediaPlayer::BufferingStrategy buffering_strategy) override;
   blink::WebTimeRanges buffered() const override;
   blink::WebTimeRanges seekable() const override;
 
@@ -468,10 +466,6 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
   // Preload state for when |data_source_| is created after setPreload().
   MultibufferDataSource::Preload preload_;
 
-  // Buffering strategy for when |data_source_| is created after
-  // setBufferingStrategy().
-  MultibufferDataSource::BufferingStrategy buffering_strategy_;
-
   // Task runner for posting tasks on Chrome's main thread. Also used
   // for DCHECKs so methods calls won't execute in the wrong thread.
   const scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;

third_party/WebKit/Source/core/BUILD.gn

@@ -1243,7 +1243,6 @@ source_set("unit_tests") {
     "html/HTMLSelectElementTest.cpp",
     "html/HTMLTableRowElementTest.cpp",
     "html/HTMLTextAreaElementTest.cpp",
-    "html/HTMLVideoElementTest.cpp",
     "html/ImageDataTest.cpp",
     "html/ImageDocumentTest.cpp",
     "html/LinkRelAttributeTest.cpp",

third_party/WebKit/Source/core/html/HTMLMediaElement.cpp

@@ -2314,13 +2314,6 @@ Nullable<ExceptionCode> HTMLMediaElement::play() {
 void HTMLMediaElement::playInternal() {
   BLINK_MEDIA_LOG << "playInternal(" << (void*)this << ")";
 
-  // Always return the buffering strategy to normal when not paused,
-  // regardless of the cause. (In contrast with aggressive buffering which is
-  // only enabled by pause(), not pauseInternal().)
-  if (webMediaPlayer())
-    webMediaPlayer()->setBufferingStrategy(
-        WebMediaPlayer::BufferingStrategy::Normal);
-
   // 4.8.12.8. Playing the media resource
   if (m_networkState == kNetworkEmpty)
     invokeResourceSelectionAlgorithm();
@@ -2352,12 +2345,6 @@ void HTMLMediaElement::playInternal() {
 void HTMLMediaElement::pause() {
   BLINK_MEDIA_LOG << "pause(" << (void*)this << ")";
 
-  // Only buffer aggressively on a user-initiated pause. Other types of pauses
-  // (which go directly to pauseInternal()) should not cause this behavior.
-  if (webMediaPlayer() && UserGestureIndicator::utilizeUserGesture())
-    webMediaPlayer()->setBufferingStrategy(
-        WebMediaPlayer::BufferingStrategy::Aggressive);
-
   if (m_autoplayVisibilityObserver) {
     m_autoplayVisibilityObserver->stop();
     m_autoplayVisibilityObserver = nullptr;

third_party/WebKit/Source/core/html/HTMLMediaElement.h

@@ -464,7 +464,7 @@ class CORE_EXPORT HTMLMediaElement
   // This does not check user gesture restrictions.
   void playInternal();
 
-  // This does not change the buffering strategy.
+  // This does not stop autoplay visibility observation.
   void pauseInternal();
 
   void allowVideoRendering();
@@ -744,7 +744,6 @@ class CORE_EXPORT HTMLMediaElement
   friend class HTMLMediaElementEventListenersTest;
   friend class HTMLMediaElementPersistentVideoTest;
   friend class HTMLVideoElement;
-  friend class HTMLVideoElementTest;
   friend class MediaControlsOrientationLockDelegateTest;
 
   Member<AutoplayUmaHelper> m_autoplayUmaHelper;

third_party/WebKit/Source/core/html/HTMLVideoElementTest.cpp

-// Copyright 2015 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "core/html/HTMLVideoElement.h"
-
-#include <memory>
-#include "core/dom/Document.h"
-#include "core/dom/DocumentUserGestureToken.h"
-#include "core/loader/EmptyClients.h"
-#include "core/testing/DummyPageHolder.h"
-#include "platform/UserGestureIndicator.h"
-#include "platform/network/NetworkStateNotifier.h"
-#include "platform/testing/EmptyWebMediaPlayer.h"
-#include "platform/testing/UnitTestHelpers.h"
-#include "public/platform/WebSize.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-#include "wtf/PtrUtil.h"
-
-namespace blink {
-
-namespace {
-
-class MockWebMediaPlayer : public EmptyWebMediaPlayer {
- public:
-  MOCK_METHOD1(setBufferingStrategy, void(BufferingStrategy));
-};
-
-class StubLocalFrameClient : public EmptyLocalFrameClient {
- public:
-  static StubLocalFrameClient* create() { return new StubLocalFrameClient; }
-
-  std::unique_ptr<WebMediaPlayer> createWebMediaPlayer(
-      HTMLMediaElement&,
-      const WebMediaPlayerSource&,
-      WebMediaPlayerClient*) override {
-    return WTF::wrapUnique(new MockWebMediaPlayer);
-  }
-};
-
-}  // namespace
-
-class HTMLVideoElementTest : public ::testing::Test {
- protected:
-  HTMLVideoElementTest()
-      : m_dummyPageHolder(
-            DummyPageHolder::create(IntSize(640, 360),
-                                    nullptr,
-                                    StubLocalFrameClient::create())) {
-    // TODO(sandersd): This should be done by a settings initializer.
-    networkStateNotifier().setWebConnection(WebConnectionTypeWifi, 54.0);
-    m_video = HTMLVideoElement::create(m_dummyPageHolder->document());
-  }
-
-  void setSrc(const AtomicString& url) {
-    m_video->setSrc(url);
-    testing::runPendingTasks();
-  }
-
-  MockWebMediaPlayer* webMediaPlayer() {
-    return static_cast<MockWebMediaPlayer*>(m_video->webMediaPlayer());
-  }
-
-  std::unique_ptr<DummyPageHolder> m_dummyPageHolder;
-  Persistent<HTMLVideoElement> m_video;
-};
-
-TEST_F(HTMLVideoElementTest, setBufferingStrategy_NonUserPause) {
-  setSrc("http://foo.bar/");
-  MockWebMediaPlayer* player = webMediaPlayer();
-  ASSERT_TRUE(player);
-
-  // On play, the strategy is set to normal.
-  EXPECT_CALL(*player,
-              setBufferingStrategy(WebMediaPlayer::BufferingStrategy::Normal));
-  m_video->play();
-  ::testing::Mock::VerifyAndClearExpectations(player);
-
-  // On a non-user pause, the strategy is not changed.
-  m_video->pause();
-  ::testing::Mock::VerifyAndClearExpectations(player);
-
-  // On play, the strategy is set to normal.
-  EXPECT_CALL(*player,
-              setBufferingStrategy(WebMediaPlayer::BufferingStrategy::Normal));
-  m_video->play();
-  ::testing::Mock::VerifyAndClearExpectations(player);
-}
-
-TEST_F(HTMLVideoElementTest, setBufferingStrategy_UserPause) {
-  setSrc("http://foo.bar/");
-  MockWebMediaPlayer* player = webMediaPlayer();
-  ASSERT_TRUE(player);
-
-  // On play, the strategy is set to normal.
-  EXPECT_CALL(*player,
-              setBufferingStrategy(WebMediaPlayer::BufferingStrategy::Normal));
-  m_video->play();
-  ::testing::Mock::VerifyAndClearExpectations(player);
-
-  // On a user pause, the strategy is set to aggressive.
-  EXPECT_CALL(*player, setBufferingStrategy(
-                           WebMediaPlayer::BufferingStrategy::Aggressive));
-  {
-    UserGestureIndicator gesture(
-        DocumentUserGestureToken::create(&m_video->document()));
-    m_video->pause();
-  }
-  ::testing::Mock::VerifyAndClearExpectations(player);
-
-  // On play, the strategy is set to normal.
-  EXPECT_CALL(*player,
-              setBufferingStrategy(WebMediaPlayer::BufferingStrategy::Normal));
-  m_video->play();
-  ::testing::Mock::VerifyAndClearExpectations(player);
-}
-
-}  // namespace blink

third_party/WebKit/public/platform/WebMediaPlayer.h

@@ -83,11 +83,6 @@ class WebMediaPlayer {
     PreloadAuto,
   };
 
-  enum class BufferingStrategy {
-    Normal,
-    Aggressive,
-  };
-
   enum CORSMode {
     CORSModeUnspecified,
     CORSModeAnonymous,
@@ -130,7 +125,6 @@ class WebMediaPlayer {
   virtual void requestRemotePlaybackStop() {}
   virtual void requestRemotePlaybackDisabled(bool disabled) {}
   virtual void setPreload(Preload) {}
-  virtual void setBufferingStrategy(BufferingStrategy) {}
   virtual WebTimeRanges buffered() const = 0;
   virtual WebTimeRanges seekable() const = 0;