Autoplay: remove muted autoplay blocking from autoplay policy.

Bug: 1028264 Change-Id: I98acba2acfc95f7e4ff64c1ce942a0220e287e25 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1933202 Commit-Queue: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Reviewed-by: Jesse Doherty <jwd@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#722364}
chromium · Dec 6, 2019 · f286512 · f286512
1 parent afa5aef
commit f286512
Show file tree

Hide file tree

Showing 24 changed files with 11 additions and 136 deletions.
diff --git a/chrome/browser/profiles/renderer_updater.cc b/chrome/browser/profiles/renderer_updater.cc
@@ -29,8 +29,8 @@ namespace {
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
 
-// By default, JavaScript, images and autoplay are enabled, and blockable mixed
-// content is blocked in guest content
+// By default, JavaScript and images are enabled, and blockable mixed content is
+// blocked in guest content
 void GetGuestViewDefaultContentSettingRules(
     bool incognito,
     RendererContentSettingRules* rules) {
@@ -45,11 +45,6 @@ void GetGuestViewDefaultContentSettingRules(
       base::Value::FromUniquePtrValue(
           content_settings::ContentSettingToValue(CONTENT_SETTING_ALLOW)),
       std::string(), incognito));
-  rules->autoplay_rules.push_back(ContentSettingPatternSource(
-      ContentSettingsPattern::Wildcard(), ContentSettingsPattern::Wildcard(),
-      base::Value::FromUniquePtrValue(
-          content_settings::ContentSettingToValue(CONTENT_SETTING_ALLOW)),
-      std::string(), incognito));
   rules->client_hints_rules.push_back(ContentSettingPatternSource(
       ContentSettingsPattern::Wildcard(), ContentSettingsPattern::Wildcard(),
       base::Value::FromUniquePtrValue(

diff --git a/chrome/renderer/content_settings_agent_impl.cc b/chrome/renderer/content_settings_agent_impl.cc
@@ -454,17 +454,6 @@ bool ContentSettingsAgentImpl::AllowRunningInsecureContent(
   return allow;
 }
 
-bool ContentSettingsAgentImpl::AllowAutoplay(bool default_value) {
-  if (!content_setting_rules_)
-    return default_value;
-
-  blink::WebLocalFrame* frame = render_frame()->GetWebFrame();
-  return GetContentSettingFromRules(
-             content_setting_rules_->autoplay_rules, frame,
-             url::Origin(frame->GetDocument().GetSecurityOrigin()).GetURL()) ==
-         CONTENT_SETTING_ALLOW;
-}
-
 bool ContentSettingsAgentImpl::AllowPopupsAndRedirects(bool default_value) {
   if (!content_setting_rules_)
     return default_value;

diff --git a/chrome/renderer/content_settings_agent_impl.h b/chrome/renderer/content_settings_agent_impl.h
@@ -65,8 +65,8 @@ class ContentSettingsAgentImpl
 #endif
 
   // Sets the content setting rules which back |allowImage()|, |allowScript()|,
-  // |allowScriptFromSource()| and |allowAutoplay()|. |content_setting_rules|
-  // must outlive this |ContentSettingsAgentImpl|.
+  // |allowScriptFromSource()|. |content_setting_rules| must outlive this
+  // |ContentSettingsAgentImpl|.
   void SetContentSettingRules(
       const RendererContentSettingRules* content_setting_rules);
   const RendererContentSettingRules* GetContentSettingRules();
@@ -96,7 +96,6 @@ class ContentSettingsAgentImpl
   void DidNotAllowScript() override;
   bool AllowRunningInsecureContent(bool allowed_per_settings,
                                    const blink::WebURL& url) override;
-  bool AllowAutoplay(bool default_value) override;
   bool AllowPopupsAndRedirects(bool default_value) override;
   void PassiveInsecureContentFound(const blink::WebURL&) override;
   void PersistClientHints(

diff --git a/chrome/renderer/content_settings_agent_impl_browsertest.cc b/chrome/renderer/content_settings_agent_impl_browsertest.cc
@@ -580,42 +580,6 @@ TEST_F(ContentSettingsAgentImplBrowserTest, ContentSettingsInterstitialPages) {
   EXPECT_EQ(0, mock_agent.on_content_blocked_count());
 }
 
-TEST_F(ContentSettingsAgentImplBrowserTest, AutoplayContentSettings) {
-  MockContentSettingsAgentImpl mock_agent(view_->GetMainRenderFrame(),
-                                          registry_.get());
-
-  // Load some HTML.
-  LoadHTML("<html>Foo</html>");
-
-  // Set the default setting.
-  RendererContentSettingRules content_setting_rules;
-  ContentSettingsForOneType& autoplay_setting_rules =
-      content_setting_rules.autoplay_rules;
-  autoplay_setting_rules.push_back(ContentSettingPatternSource(
-      ContentSettingsPattern::Wildcard(), ContentSettingsPattern::Wildcard(),
-      base::Value::FromUniquePtrValue(
-          content_settings::ContentSettingToValue(CONTENT_SETTING_ALLOW)),
-      std::string(), false));
-
-  ContentSettingsAgentImpl* agent =
-      ContentSettingsAgentImpl::Get(view_->GetMainRenderFrame());
-  agent->SetContentSettingRules(&content_setting_rules);
-
-  EXPECT_TRUE(agent->AllowAutoplay(false));
-
-  // Add rule to block autoplay.
-  autoplay_setting_rules.insert(
-      autoplay_setting_rules.begin(),
-      ContentSettingPatternSource(
-          ContentSettingsPattern::Wildcard(),
-          ContentSettingsPattern::Wildcard(),
-          base::Value::FromUniquePtrValue(
-              content_settings::ContentSettingToValue(CONTENT_SETTING_BLOCK)),
-          std::string(), false));
-
-  EXPECT_FALSE(agent->AllowAutoplay(true));
-}
-
 TEST_F(ContentSettingsAgentImplBrowserTest, MixedAutoupgradesDisabledByRules) {
   MockContentSettingsAgentImpl mock_agent(view_->GetMainRenderFrame(),
                                           registry_.get());

diff --git a/components/content_settings/core/browser/content_settings_utils.cc b/components/content_settings/core/browser/content_settings_utils.cc
@@ -143,8 +143,6 @@ void GetRendererContentSettingRules(const HostContentSettingsMap* map,
 #endif
   map->GetSettingsForOneType(ContentSettingsType::JAVASCRIPT,
                              ResourceIdentifier(), &(rules->script_rules));
-  map->GetSettingsForOneType(ContentSettingsType::AUTOPLAY,
-                             ResourceIdentifier(), &(rules->autoplay_rules));
   map->GetSettingsForOneType(ContentSettingsType::CLIENT_HINTS,
                              ResourceIdentifier(),
                              &(rules->client_hints_rules));

diff --git a/components/content_settings/core/common/content_settings.cc b/components/content_settings/core/common/content_settings.cc
@@ -159,7 +159,6 @@ bool RendererContentSettingRules::IsRendererContentSetting(
     ContentSettingsType content_type) {
   return content_type == ContentSettingsType::IMAGES ||
          content_type == ContentSettingsType::JAVASCRIPT ||
-         content_type == ContentSettingsType::AUTOPLAY ||
          content_type == ContentSettingsType::CLIENT_HINTS ||
          content_type == ContentSettingsType::POPUPS ||
          content_type == ContentSettingsType::MIXEDSCRIPT;

diff --git a/components/content_settings/core/common/content_settings.h b/components/content_settings/core/common/content_settings.h
@@ -72,7 +72,6 @@ struct RendererContentSettingRules {
   ~RendererContentSettingRules();
   ContentSettingsForOneType image_rules;
   ContentSettingsForOneType script_rules;
-  ContentSettingsForOneType autoplay_rules;
   ContentSettingsForOneType client_hints_rules;
   ContentSettingsForOneType popup_redirect_rules;
   ContentSettingsForOneType mixed_content_rules;

diff --git a/components/content_settings/core/common/content_settings.mojom b/components/content_settings/core/common/content_settings.mojom
@@ -73,7 +73,6 @@ struct ContentSettingPatternSource {
 struct RendererContentSettingRules {
   array<ContentSettingPatternSource> image_rules;
   array<ContentSettingPatternSource> script_rules;
-  array<ContentSettingPatternSource> autoplay_rules;
   array<ContentSettingPatternSource> client_hints_rules;
   array<ContentSettingPatternSource> popup_redirect_rules;
   array<ContentSettingPatternSource> mixed_content_rules;

diff --git a/components/content_settings/core/common/content_settings_mojom_traits.cc b/components/content_settings/core/common/content_settings_mojom_traits.cc
@@ -99,7 +99,6 @@ bool StructTraits<content_settings::mojom::RendererContentSettingRulesDataView,
          RendererContentSettingRules* out) {
   return data.ReadImageRules(&out->image_rules) &&
          data.ReadScriptRules(&out->script_rules) &&
-         data.ReadAutoplayRules(&out->autoplay_rules) &&
          data.ReadClientHintsRules(&out->client_hints_rules) &&
          data.ReadPopupRedirectRules(&out->popup_redirect_rules) &&
          data.ReadMixedContentRules(&out->mixed_content_rules);

diff --git a/components/content_settings/core/common/content_settings_mojom_traits.h b/components/content_settings/core/common/content_settings_mojom_traits.h
@@ -130,11 +130,6 @@ struct StructTraits<
     return r.script_rules;
   }
 
-  static const std::vector<ContentSettingPatternSource>& autoplay_rules(
-      const RendererContentSettingRules& r) {
-    return r.autoplay_rules;
-  }
-
   static const std::vector<ContentSettingPatternSource>& client_hints_rules(
       const RendererContentSettingRules& r) {
     return r.client_hints_rules;

diff --git a/content/shell/test_runner/mock_content_settings_client.cc b/content/shell/test_runner/mock_content_settings_client.cc
@@ -65,10 +65,6 @@ bool MockContentSettingsClient::AllowRunningInsecureContent(
   return enabled_per_settings || flags_->running_insecure_content_allowed();
 }
 
-bool MockContentSettingsClient::AllowAutoplay(bool default_value) {
-  return flags_->autoplay_allowed();
-}
-
 void MockContentSettingsClient::SetDelegate(WebTestDelegate* delegate) {
   delegate_ = delegate;
 }

diff --git a/content/shell/test_runner/mock_content_settings_client.h b/content/shell/test_runner/mock_content_settings_client.h
@@ -38,7 +38,6 @@ class MockContentSettingsClient : public blink::WebContentSettingsClient {
   bool AllowStorage(bool local) override;
   bool AllowRunningInsecureContent(bool enabled_per_settings,
                                    const blink::WebURL& url) override;
-  bool AllowAutoplay(bool default_value) override;
   void PersistClientHints(
       const blink::WebEnabledClientHints& enabled_client_hints,
       base::TimeDelta duration,

diff --git a/content/shell/test_runner/test_runner.cc b/content/shell/test_runner/test_runner.cc
@@ -213,7 +213,6 @@ class TestRunnerBindings : public gin::Wrappable<TestRunnerBindings> {
   void SetAcceptLanguages(const std::string& accept_languages);
   void SetAllowFileAccessFromFileURLs(bool allow);
   void SetAllowRunningOfInsecureContent(bool allowed);
-  void SetAutoplayAllowed(bool allowed);
   void SetBlockThirdPartyCookies(bool block);
   void SetAudioData(const gin::ArrayBufferView& view);
   void SetBackingScaleFactor(double value, v8::Local<v8::Function> callback);
@@ -511,7 +510,6 @@ gin::ObjectTemplateBuilder TestRunnerBindings::GetObjectTemplateBuilder(
                  &TestRunnerBindings::SetAllowFileAccessFromFileURLs)
       .SetMethod("setAllowRunningOfInsecureContent",
                  &TestRunnerBindings::SetAllowRunningOfInsecureContent)
-      .SetMethod("setAutoplayAllowed", &TestRunnerBindings::SetAutoplayAllowed)
       .SetMethod("setBlockThirdPartyCookies",
                  &TestRunnerBindings::SetBlockThirdPartyCookies)
       .SetMethod("setAudioData", &TestRunnerBindings::SetAudioData)
@@ -1059,11 +1057,6 @@ void TestRunnerBindings::SetAllowRunningOfInsecureContent(bool allowed) {
     runner_->SetAllowRunningOfInsecureContent(allowed);
 }
 
-void TestRunnerBindings::SetAutoplayAllowed(bool allowed) {
-  if (runner_)
-    runner_->SetAutoplayAllowed(allowed);
-}
-
 void TestRunnerBindings::DumpPermissionClientCallbacks() {
   if (runner_)
     runner_->DumpPermissionClientCallbacks();
@@ -2349,11 +2342,6 @@ void TestRunner::SetAllowRunningOfInsecureContent(bool allowed) {
   OnWebTestRuntimeFlagsChanged();
 }
 
-void TestRunner::SetAutoplayAllowed(bool allowed) {
-  web_test_runtime_flags_.set_autoplay_allowed(allowed);
-  OnWebTestRuntimeFlagsChanged();
-}
-
 void TestRunner::DumpPermissionClientCallbacks() {
   web_test_runtime_flags_.set_dump_web_content_settings_client_callbacks(true);
   OnWebTestRuntimeFlagsChanged();

diff --git a/content/shell/test_runner/test_runner.h b/content/shell/test_runner/test_runner.h
@@ -375,7 +375,6 @@ class TestRunner : public WebTestRunner {
   void SetStorageAllowed(bool allowed);
   void SetPluginsAllowed(bool allowed);
   void SetAllowRunningOfInsecureContent(bool allowed);
-  void SetAutoplayAllowed(bool allowed);
   void DumpPermissionClientCallbacks();
 
   // Sets up a mock DocumentSubresourceFilter to disallow subsequent subresource

diff --git a/content/shell/test_runner/web_test_runtime_flags.cc b/content/shell/test_runner/web_test_runtime_flags.cc
@@ -38,7 +38,6 @@ void WebTestRuntimeFlags::Reset() {
   set_storage_allowed(true);
   set_plugins_allowed(true);
   set_running_insecure_content_allowed(false);
-  set_autoplay_allowed(true);
 
   set_dump_editting_callbacks(false);
   set_dump_frame_load_callbacks(false);

diff --git a/content/shell/test_runner/web_test_runtime_flags.h b/content/shell/test_runner/web_test_runtime_flags.h
@@ -110,7 +110,6 @@ class TEST_RUNNER_EXPORT WebTestRuntimeFlags {
   DEFINE_BOOL_WEB_TEST_RUNTIME_FLAG(plugins_allowed)
   DEFINE_BOOL_WEB_TEST_RUNTIME_FLAG(running_insecure_content_allowed)
   DEFINE_BOOL_WEB_TEST_RUNTIME_FLAG(dump_web_content_settings_client_callbacks)
-  DEFINE_BOOL_WEB_TEST_RUNTIME_FLAG(autoplay_allowed)
 
   // If true, the test runner will write a descriptive line for each editing
   // command.

diff --git a/third_party/blink/public/platform/web_content_settings_client.h b/third_party/blink/public/platform/web_content_settings_client.h
@@ -97,9 +97,6 @@ class WebContentSettingsClient {
   // interface.
   virtual bool AllowMutationEvents(bool default_value) { return default_value; }
 
-  // Controls whether autoplay is allowed for this frame.
-  virtual bool AllowAutoplay(bool default_value) { return default_value; }
-
   virtual bool AllowPopupsAndRedirects(bool default_value) {
     return default_value;
   }

diff --git a/third_party/blink/renderer/core/html/media/autoplay_policy.cc b/third_party/blink/renderer/core/html/media/autoplay_policy.cc
@@ -7,7 +7,6 @@
 #include "build/build_config.h"
 #include "third_party/blink/public/mojom/autoplay/autoplay.mojom-blink.h"
 #include "third_party/blink/public/mojom/feature_policy/feature_policy.mojom-blink.h"
-#include "third_party/blink/public/platform/web_content_settings_client.h"
 #include "third_party/blink/public/platform/web_media_player.h"
 #include "third_party/blink/public/web/web_local_frame.h"
 #include "third_party/blink/public/web/web_local_frame_client.h"
@@ -307,10 +306,9 @@ bool AutoplayPolicy::IsGestureNeededForPlayback() const {
   if (!IsLockedPendingUserGesture())
     return false;
 
-  // We want to allow muted video to autoplay if:
-  // - The element is allowed to autoplay muted;
-  // - Autoplay is enabled in settings.
-  return !(IsEligibleForAutoplayMuted() && IsAutoplayAllowedPerSettings());
+  // We want to allow muted video to autoplay if the element is allowed to
+  // autoplay muted.
+  return !IsEligibleForAutoplayMuted();
 }
 
 String AutoplayPolicy::GetPlayErrorMessage() const {
@@ -377,15 +375,6 @@ void AutoplayPolicy::MaybeSetAutoplayInitiated() {
   }
 }
 
-bool AutoplayPolicy::IsAutoplayAllowedPerSettings() const {
-  LocalFrame* frame = element_->GetDocument().GetFrame();
-  if (!frame)
-    return false;
-  if (auto* settings_client = frame->GetContentSettingsClient())
-    return settings_client->AllowAutoplay(true /* default_value */);
-  return true;
-}
-
 bool AutoplayPolicy::ShouldAutoplay() {
   if (element_->GetDocument().IsSandboxed(WebSandboxFlags::kAutomaticFeatures))
     return false;

diff --git a/third_party/blink/renderer/core/html/media/autoplay_policy.h b/third_party/blink/renderer/core/html/media/autoplay_policy.h
@@ -131,10 +131,6 @@ class CORE_EXPORT AutoplayPolicy final
   // should use, if checking to see if an action is allowed.
   bool IsLockedPendingUserGesture() const;
 
-  // Return true if and only if the settings allow autoplay of media on this
-  // frame.
-  bool IsAutoplayAllowedPerSettings() const;
-
   bool IsAutoplayingMutedInternal(bool muted) const;
   bool IsOrWillBeAutoplayingMutedInternal(bool muted) const;
 

diff --git a/third_party/blink/renderer/core/html/media/autoplay_uma_helper.cc b/third_party/blink/renderer/core/html/media/autoplay_uma_helper.cc
@@ -69,9 +69,6 @@ void AutoplayUmaHelper::OnAutoplayInitiated(AutoplaySource source) {
   DEFINE_STATIC_LOCAL(EnumerationHistogram, audio_histogram,
                       ("Media.Audio.Autoplay",
                        static_cast<int>(AutoplaySource::kNumberOfUmaSources)));
-  DEFINE_STATIC_LOCAL(
-      EnumerationHistogram, blocked_muted_video_histogram,
-      ("Media.Video.Autoplay.Muted.Blocked", kAutoplayBlockedReasonMax));
 
   // Autoplay already initiated
   if (sources_.Contains(source))
@@ -101,14 +98,6 @@ void AutoplayUmaHelper::OnAutoplayInitiated(AutoplaySource source) {
     }
   }
 
-  // Record if it will be blocked by the Autoplay setting.
-  if (IsA<HTMLVideoElement>(element_.Get()) && element_->muted() &&
-      AutoplayPolicy::DocumentShouldAutoplayMutedVideos(
-          element_->GetDocument()) &&
-      !element_->GetAutoplayPolicy().IsAutoplayAllowedPerSettings()) {
-    blocked_muted_video_histogram.Count(kAutoplayBlockedReasonSetting);
-  }
-
   element_->addEventListener(event_type_names::kPlaying, this, false);
 
   // Record UKM autoplay event.

diff --git a/third_party/blink/web_tests/media/autoplay-muted.html b/third_party/blink/web_tests/media/autoplay-muted.html
@@ -15,7 +15,6 @@
 
     internals.settings.setAutoplayPolicy('user-gesture-required');
     internals.runtimeFlags.autoplayMutedVideosEnabled = true;
-    testRunner.setAutoplayAllowed(true);
 
     function createMutedMediaElement(type) {
         var e = document.createElement(type);
@@ -97,15 +96,5 @@
             eventSender.consumeUserActivation();
         });
     }, "Test that unmuting autoplayed video with gesture doesn't pause it.");
-
-    promise_test(function(t) {
-        testRunner.setAutoplayAllowed(false);
-        return promise_rejects(
-            t,
-            new DOMException(
-                'play() can only be initiated by a user gesture.',
-                'NotAllowedError'),
-            createMutedVideoElement().play());
-    }, "Test that muted videos don't autoplay when the setting is disabled");
 </script>
 </body>
diff --git a/third_party/blink/web_tests/media/autoplay-non-whitelisted-scope.html b/third_party/blink/web_tests/media/autoplay-non-whitelisted-scope.html
@@ -16,8 +16,6 @@
 internals.settings.setWebAppScope(
     document.URL.substring(0, document.URL.lastIndexOf('/') + 1) + "foo/");
 
-testRunner.setAutoplayAllowed(true);
-
 function createMediaElement(type) {
   var e = document.createElement(type);
   e.src = type == 'audio' ? 'content/test.oga' : 'content/test.ogv';

diff --git a/third_party/blink/web_tests/media/autoplay-whitelisted-scope.html b/third_party/blink/web_tests/media/autoplay-whitelisted-scope.html
@@ -17,8 +17,6 @@
     document.URL.substring(0, document.URL.lastIndexOf('/') + 1)
     );
 
-testRunner.setAutoplayAllowed(true);
-
 function createMediaElement(type) {
   var e = document.createElement(type);
   e.src = type == 'audio' ? 'content/test.oga' : 'content/test.ogv';

diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
@@ -69034,7 +69034,10 @@ uploading your change for review.
 </histogram>
 
 <histogram name="Media.Video.Autoplay.Muted.Blocked"
-    enum="AutoplayBlockedReason" expires_after="M82">
+    enum="AutoplayBlockedReason">
+  <obsolete>
+    Deprecated as of M80, autoplaying muted videos can no longer be blocked.
+  </obsolete>
   <owner>mlamouri@chromium.org</owner>
   <owner>media-dev@chromium.org</owner>
   <summary>