Revert "Add Skip Ad button to Picture-in-Picture window."
This reverts commit b0281b1250cdec351658e9bf07dd5f5d3331ef91.
Reason for revert: Broke build (see https://ci.chromium.org/buildbot/chromium.chrome/Google%20Chrome%20ChromeOS/60556 )
Original change's description:
> Add Skip Ad button to Picture-in-Picture window.
>
> This CL adds 'skipad' to the Media Session actions available to web
> developers. It allows them to use it to show/hide a Skip Ad button in
> the Picture-in-Picture window. When users clicks it, the action handler
> is called.
>
> Screenshots: https://imgur.com/a/fxBNTKf
>
> Change-Id: I017421e2efe9a8b31fd577647c1e2b5f73cb23f0
> Bug: 910436
> Reviewed-on: https://chromium-review.googlesource.com/c/1393331
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Reviewed-by: Becca Hughes <beccahughes@chromium.org>
> Reviewed-by: Eliot Courtney <edcourtney@chromium.org>
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> Commit-Queue: François Beaufort <beaufort.francois@gmail.com>
> Cr-Commit-Position: refs/heads/master@{#622306}
TBR=dcheng@chromium.org,beaufort.francois@gmail.com,mlamouri@chromium.org,jochen@chromium.org,edcourtney@chromium.org,beccahughes@chromium.org
Change-Id: I2d02199072a3d8f1fc173c74ad7be4203f4506c8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 910436
Reviewed-on: https://chromium-review.googlesource.com/c/1408269
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Commit-Queue: Roman Sorokin [CET] <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622308}diff --git a/ash/media/media_notification_item.cc b/ash/media/media_notification_item.cc
index ba9d0e54..f6b36c5 100644
--- a/ash/media/media_notification_item.cc
+++ b/ash/media/media_notification_item.cc
@@ -158,8 +158,6 @@
case MediaSessionAction::kStop:
media_controller_ptr_->Stop();
break;
- case MediaSessionAction::kSkipAd:
- break;
}
}
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 783fe9a..df0f3a5 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -5633,9 +5633,6 @@
<message name="IDS_PICTURE_IN_PICTURE_BACK_TO_TAB_CONTROL_TEXT" desc="Text label of the back to tab control button. The button appears when the user hovers over the Picture-in-Picture window.">
Back to tab
</message>
- <message name="IDS_PICTURE_IN_PICTURE_SKIP_AD_CONTROL_TEXT" desc="Text label of the skip ad control button. The button appears when the user hovers over the Picture-in-Picture window.">
- Skip Ad
- </message>
<message name="IDS_PICTURE_IN_PICTURE_CLOSE_CONTROL_TEXT" desc="Text label of the close control button. The button appears when the user hovers over the Picture-in-Picture window.">
Close
</message>
diff --git a/chrome/browser/chromeos/arc/pip/arc_picture_in_picture_window_controller_impl.cc b/chrome/browser/chromeos/arc/pip/arc_picture_in_picture_window_controller_impl.cc
index f68926c7..5de94d1 100644
--- a/chrome/browser/chromeos/arc/pip/arc_picture_in_picture_window_controller_impl.cc
+++ b/chrome/browser/chromeos/arc/pip/arc_picture_in_picture_window_controller_impl.cc
@@ -91,8 +91,4 @@
// Should be a no-op on ARC. This is managed on the Android side.
}
-void ArcPictureInPictureWindowControllerImpl::SkipAd() {
- // Should be a no-op on ARC. This is managed on the Android side.
-}
-
} // namespace arc
diff --git a/chrome/browser/chromeos/arc/pip/arc_picture_in_picture_window_controller_impl.h b/chrome/browser/chromeos/arc/pip/arc_picture_in_picture_window_controller_impl.h
index 0bc5b3f..6db70d2 100644
--- a/chrome/browser/chromeos/arc/pip/arc_picture_in_picture_window_controller_impl.h
+++ b/chrome/browser/chromeos/arc/pip/arc_picture_in_picture_window_controller_impl.h
@@ -50,7 +50,6 @@
void UpdatePlaybackState(bool is_playing,
bool reached_end_of_stream) override;
void SetAlwaysHidePlayPauseButton(bool is_visible) override;
- void SkipAd() override;
private:
arc::ArcPipBridge* const arc_pip_bridge_;
diff --git a/chrome/browser/picture_in_picture/DEPS b/chrome/browser/picture_in_picture/DEPS
index da2c556..7a8fcf0 100644
--- a/chrome/browser/picture_in_picture/DEPS
+++ b/chrome/browser/picture_in_picture/DEPS
@@ -3,6 +3,5 @@
"+ash/accelerators/accelerator_controller.h",
"+ash/shell.h",
"+chrome/browser/ui/views/overlay/overlay_window_views.h",
- "+chrome/browser/ui/views/overlay/skip_ad_label_button.h",
],
}
diff --git a/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc b/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc
index 2c7aa711..610b98f 100644
--- a/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc
+++ b/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc
@@ -6,7 +6,6 @@
#include "base/path_service.h"
#include "base/strings/utf_string_conversions.h"
-#include "base/test/scoped_feature_list.h"
#include "build/build_config.h"
#include "chrome/browser/devtools/devtools_window_testing.h"
#include "chrome/browser/picture_in_picture/picture_in_picture_window_manager.h"
@@ -25,19 +24,15 @@
#include "content/public/common/content_switches.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/test_navigation_observer.h"
-#include "media/base/media_switches.h"
#include "net/dns/mock_host_resolver.h"
-#include "services/media_session/public/cpp/features.h"
#include "skia/ext/image_operations.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "third_party/blink/public/common/picture_in_picture/picture_in_picture_control_info.h"
#include "ui/aura/window.h"
-#include "ui/events/base_event_utils.h"
#include "ui/gfx/codec/png_codec.h"
#if !defined(OS_ANDROID)
#include "chrome/browser/ui/views/overlay/overlay_window_views.h"
-#include "chrome/browser/ui/views/overlay/skip_ad_label_button.h"
#include "ui/views/controls/button/image_button.h"
#include "ui/views/widget/widget_observer.h"
#endif
@@ -78,7 +73,6 @@
MOCK_METHOD0(TogglePlayPause, bool());
MOCK_METHOD1(CustomControlPressed, void(const std::string&));
MOCK_METHOD1(SetAlwaysHidePlayPauseButton, void(bool));
- MOCK_METHOD0(SkipAd, void());
private:
DISALLOW_COPY_AND_ASSIGN(MockPictureInPictureWindowController);
@@ -1771,97 +1765,3 @@
active_web_contents, "isInPictureInPicture();", &in_picture_in_picture));
EXPECT_TRUE(in_picture_in_picture);
}
-
-class MediaSessionPictureInPictureWindowControllerBrowserTest
- : public PictureInPictureWindowControllerBrowserTest {
- public:
- void SetUpCommandLine(base::CommandLine* command_line) override {
- PictureInPictureWindowControllerBrowserTest::SetUpCommandLine(command_line);
- command_line->AppendSwitchASCII(switches::kEnableBlinkFeatures,
- "MediaSession");
- scoped_feature_list_.InitWithFeatures(
- {media_session::features::kMediaSessionService, media::kSkipAd}, {});
- }
-
-#if !defined(OS_ANDROID)
- void MoveMouseOver(OverlayWindowViews* window) {
- gfx::Point p(window->GetBounds().x(), window->GetBounds().y());
- ui::MouseEvent moved_over(ui::ET_MOUSE_MOVED, p, p, ui::EventTimeForNow(),
- ui::EF_NONE, ui::EF_NONE);
- window->OnMouseEvent(&moved_over);
- }
-#endif
-
- private:
- base::test::ScopedFeatureList scoped_feature_list_;
-};
-
-#if !defined(OS_ANDROID)
-// Tests that a Skip Ad button is displayed in the Picture-in-Picture window
-// when Media Session Action "skipad" is handled by the website.
-IN_PROC_BROWSER_TEST_F(MediaSessionPictureInPictureWindowControllerBrowserTest,
- SkipAdButtonVisibility) {
- LoadTabAndEnterPictureInPicture(browser());
- OverlayWindowViews* overlay_window = static_cast<OverlayWindowViews*>(
- window_controller()->GetWindowForTesting());
- ASSERT_TRUE(overlay_window);
-
- // Skip Ad button is not displayed initially when mouse is hovering over the
- // window.
- MoveMouseOver(overlay_window);
- EXPECT_FALSE(
- overlay_window->skip_ad_controls_view_for_testing()->layer()->visible());
-
- content::WebContents* active_web_contents =
- browser()->tab_strip_model()->GetActiveWebContents();
- ASSERT_TRUE(content::ExecuteScript(active_web_contents,
- "setMediaSessionSkipAdActionHandler();"));
-
- // Skip Ad button is not displayed if video is not playing even if mouse is
- // hovering over the window and media session action handler has been set.
- base::RunLoop().RunUntilIdle();
- MoveMouseOver(overlay_window);
- EXPECT_FALSE(
- overlay_window->skip_ad_controls_view_for_testing()->layer()->visible());
-
- EXPECT_FALSE(
- overlay_window->skip_ad_controls_view_for_testing()->layer()->visible());
- ASSERT_TRUE(content::ExecuteScript(active_web_contents, "video.play();"));
-
- // Set action handler and check that Skip Ad button is now displayed when
- // video plays and mouse is hovering over the window.
- base::RunLoop().RunUntilIdle();
- MoveMouseOver(overlay_window);
- EXPECT_TRUE(
- overlay_window->skip_ad_controls_view_for_testing()->layer()->visible());
-
- // Unset action handler and check that Skip Ad button is not displayed when
- // video plays and mouse is hovering over the window.
- ASSERT_TRUE(content::ExecuteScript(
- active_web_contents, "unsetMediaSessionSkipAdActionHandler();"));
- base::RunLoop().RunUntilIdle();
- MoveMouseOver(overlay_window);
- EXPECT_FALSE(
- overlay_window->skip_ad_controls_view_for_testing()->layer()->visible());
-}
-#endif
-
-// Tests that clicking the Skip Ad button in the Picture-in-Picture window
-// calls the Media Session Action "skipad" handler function.
-IN_PROC_BROWSER_TEST_F(MediaSessionPictureInPictureWindowControllerBrowserTest,
- SkipAdHandlerCalled) {
- LoadTabAndEnterPictureInPicture(browser());
- content::WebContents* active_web_contents =
- browser()->tab_strip_model()->GetActiveWebContents();
- ASSERT_TRUE(content::ExecuteScript(active_web_contents, "video.play();"));
- ASSERT_TRUE(content::ExecuteScript(active_web_contents,
- "setMediaSessionSkipAdActionHandler();"));
- base::RunLoop().RunUntilIdle();
-
- // Simulates user clicking "Skip Ad" and check the handler function is called.
- window_controller()->SkipAd();
- base::string16 expected_title = base::ASCIIToUTF16("skipad");
- EXPECT_EQ(expected_title,
- content::TitleWatcher(active_web_contents, expected_title)
- .WaitAndGetTitle());
-}
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
index 2583faf..e5a88db 100644
--- a/chrome/browser/ui/BUILD.gn
+++ b/chrome/browser/ui/BUILD.gn
@@ -2644,8 +2644,6 @@
"views/overlay/playback_image_button.h",
"views/overlay/resize_handle_button.cc",
"views/overlay/resize_handle_button.h",
- "views/overlay/skip_ad_label_button.cc",
- "views/overlay/skip_ad_label_button.h",
"views/page_action/page_action_icon_container_view.cc",
"views/page_action/page_action_icon_container_view.h",
"views/page_action/page_action_icon_view.cc",
diff --git a/chrome/browser/ui/views/overlay/overlay_window_views.cc b/chrome/browser/ui/views/overlay/overlay_window_views.cc
index 8184fea..d651225 100644
--- a/chrome/browser/ui/views/overlay/overlay_window_views.cc
+++ b/chrome/browser/ui/views/overlay/overlay_window_views.cc
@@ -17,12 +17,10 @@
#include "chrome/browser/ui/views/overlay/control_image_button.h"
#include "chrome/browser/ui/views/overlay/playback_image_button.h"
#include "chrome/browser/ui/views/overlay/resize_handle_button.h"
-#include "chrome/browser/ui/views/overlay/skip_ad_label_button.h"
#include "chrome/grit/generated_resources.h"
#include "components/vector_icons/vector_icons.h"
#include "content/public/browser/picture_in_picture_window_controller.h"
#include "content/public/browser/web_contents.h"
-#include "media/base/media_switches.h"
#include "media/base/video_util.h"
#include "third_party/blink/public/common/picture_in_picture/picture_in_picture_control_info.h"
#include "third_party/skia/include/core/SkColor.h"
@@ -117,7 +115,6 @@
OverlayWindowViews* window = static_cast<OverlayWindowViews*>(widget_);
if (window->AreControlsVisible() &&
(window->GetBackToTabControlsBounds().Contains(point) ||
- window->GetSkipAdControlsBounds().Contains(point) ||
window->GetCloseControlsBounds().Contains(point) ||
window->GetFirstCustomControlsBounds().Contains(point) ||
window->GetSecondCustomControlsBounds().Contains(point) ||
@@ -189,7 +186,6 @@
controls_scrim_view_(new views::View()),
controls_parent_view_(new views::View()),
back_to_tab_controls_view_(new views::BackToTabImageButton(this)),
- skip_ad_controls_view_(new views::SkipAdLabelButton(this)),
close_controls_view_(new views::CloseImageButton(this)),
#if defined(OS_CHROMEOS)
resize_handle_view_(new views::ResizeHandleButton(this)),
@@ -336,15 +332,8 @@
// views::View that closes the window and focuses initiator tab. ------------
back_to_tab_controls_view_->SetPaintToLayer(ui::LAYER_TEXTURED);
back_to_tab_controls_view_->layer()->SetFillsBoundsOpaquely(false);
- back_to_tab_controls_view_->layer()->set_name("BackToTabControlsView");
back_to_tab_controls_view_->set_owned_by_client();
- // views::View that holds the skip-ad label button. -------------------------
- skip_ad_controls_view_->SetPaintToLayer(ui::LAYER_TEXTURED);
- skip_ad_controls_view_->layer()->SetFillsBoundsOpaquely(true);
- skip_ad_controls_view_->layer()->set_name("SkipAdControlsView");
- skip_ad_controls_view_->set_owned_by_client();
-
// views::View that closes the window. --------------------------------------
close_controls_view_->SetPaintToLayer(ui::LAYER_TEXTURED);
close_controls_view_->layer()->SetFillsBoundsOpaquely(false);
@@ -368,11 +357,10 @@
resize_handle_view_->set_owned_by_client();
#endif
- // Set up view::Views hierarchy. --------------------------------------------
+ // Set up view::Views heirarchy. --------------------------------------------
controls_parent_view_->AddChildView(play_pause_controls_view_.get());
GetContentsView()->AddChildView(controls_scrim_view_.get());
GetContentsView()->AddChildView(controls_parent_view_.get());
- GetContentsView()->AddChildView(skip_ad_controls_view_.get());
GetContentsView()->AddChildView(back_to_tab_controls_view_.get());
GetContentsView()->AddChildView(close_controls_view_.get());
#if defined(OS_CHROMEOS)
@@ -417,7 +405,6 @@
GetControlsScrimLayer()->SetVisible(is_visible);
GetControlsParentLayer()->SetVisible(is_visible);
GetBackToTabControlsLayer()->SetVisible(is_visible);
- GetSkipAdControlsLayer()->SetVisible(is_visible && show_skip_ad_button_);
GetCloseControlsLayer()->SetVisible(is_visible);
#if defined(OS_CHROMEOS)
@@ -435,7 +422,6 @@
WindowQuadrant quadrant = GetCurrentWindowQuadrant(GetBounds(), controller_);
back_to_tab_controls_view_->SetPosition(GetBounds().size(), quadrant);
- skip_ad_controls_view_->SetPosition(GetBounds().size());
close_controls_view_->SetPosition(GetBounds().size(), quadrant);
#if defined(OS_CHROMEOS)
resize_handle_view_->SetPosition(GetBounds().size(), quadrant);
@@ -628,11 +614,6 @@
always_hide_play_pause_button_ = !is_visible;
}
-void OverlayWindowViews::SetSkipAdButtonVisibility(bool is_visible) {
- if (base::FeatureList::IsEnabled(media::kSkipAd))
- show_skip_ad_button_ = is_visible;
-}
-
void OverlayWindowViews::SetPictureInPictureCustomControls(
const std::vector<blink::PictureInPictureControlInfo>& controls) {
// Clear any existing controls.
@@ -808,9 +789,6 @@
if (GetBackToTabControlsBounds().Contains(event->location())) {
controller_->CloseAndFocusInitiator();
event->SetHandled();
- } else if (GetSkipAdControlsBounds().Contains(event->location())) {
- controller_->SkipAd();
- event->SetHandled();
} else if (GetCloseControlsBounds().Contains(event->location())) {
controller_->Close(true /* should_pause_video */,
true /* should_reset_pip_player */);
@@ -826,9 +804,6 @@
if (sender == back_to_tab_controls_view_.get())
controller_->CloseAndFocusInitiator();
- if (sender == skip_ad_controls_view_.get())
- controller_->SkipAd();
-
if (sender == close_controls_view_.get())
controller_->Close(true /* should_pause_video */,
true /* should_reset_pip_player */);
@@ -847,10 +822,6 @@
return back_to_tab_controls_view_->GetMirroredBounds();
}
-gfx::Rect OverlayWindowViews::GetSkipAdControlsBounds() {
- return skip_ad_controls_view_->GetMirroredBounds();
-}
-
gfx::Rect OverlayWindowViews::GetCloseControlsBounds() {
return close_controls_view_->GetMirroredBounds();
}
@@ -891,10 +862,6 @@
return back_to_tab_controls_view_->layer();
}
-ui::Layer* OverlayWindowViews::GetSkipAdControlsLayer() {
- return skip_ad_controls_view_->layer();
-}
-
ui::Layer* OverlayWindowViews::GetCloseControlsLayer() {
return close_controls_view_->layer();
}
@@ -924,11 +891,6 @@
return back_to_tab_controls_view_->origin();
}
-views::SkipAdLabelButton*
-OverlayWindowViews::skip_ad_controls_view_for_testing() const {
- return skip_ad_controls_view_.get();
-}
-
gfx::Point OverlayWindowViews::close_image_position_for_testing() const {
return close_controls_view_->origin();
}
diff --git a/chrome/browser/ui/views/overlay/overlay_window_views.h b/chrome/browser/ui/views/overlay/overlay_window_views.h
index 2f1e974..13e27504 100644
--- a/chrome/browser/ui/views/overlay/overlay_window_views.h
+++ b/chrome/browser/ui/views/overlay/overlay_window_views.h
@@ -21,7 +21,6 @@
class CloseImageButton;
class PlaybackImageButton;
class ResizeHandleButton;
-class SkipAdLabelButton;
} // namespace views
// The Chrome desktop implementation of OverlayWindow. This will only be
@@ -48,7 +47,6 @@
void UpdateVideoSize(const gfx::Size& natural_size) override;
void SetPlaybackState(PlaybackState playback_state) override;
void SetAlwaysHidePlayPauseButton(bool is_visible) override;
- void SetSkipAdButtonVisibility(bool is_visible) override;
void SetPictureInPictureCustomControls(
const std::vector<blink::PictureInPictureControlInfo>& controls) override;
ui::Layer* GetWindowBackgroundLayer() override;
@@ -72,7 +70,6 @@
// Gets the bounds of the controls.
gfx::Rect GetBackToTabControlsBounds();
- gfx::Rect GetSkipAdControlsBounds();
gfx::Rect GetCloseControlsBounds();
gfx::Rect GetResizeHandleControlsBounds();
gfx::Rect GetPlayPauseControlsBounds();
@@ -89,7 +86,6 @@
views::PlaybackImageButton* play_pause_controls_view_for_testing() const;
gfx::Point back_to_tab_image_position_for_testing() const;
- views::SkipAdLabelButton* skip_ad_controls_view_for_testing() const;
gfx::Point close_image_position_for_testing() const;
gfx::Point resize_handle_position_for_testing() const;
views::View* controls_parent_view_for_testing() const;
@@ -143,7 +139,6 @@
ui::Layer* GetControlsScrimLayer();
ui::Layer* GetBackToTabControlsLayer();
- ui::Layer* GetSkipAdControlsLayer();
ui::Layer* GetCloseControlsLayer();
ui::Layer* GetResizeHandleLayer();
ui::Layer* GetControlsParentLayer();
@@ -195,10 +190,9 @@
std::unique_ptr<views::View> video_view_;
std::unique_ptr<views::View> controls_scrim_view_;
// |controls_parent_view_| is the parent view of all control views except
- // the back-to-tab, close and skip-ad views.
+ // the back-to-tab and close views.
std::unique_ptr<views::View> controls_parent_view_;
std::unique_ptr<views::BackToTabImageButton> back_to_tab_controls_view_;
- std::unique_ptr<views::SkipAdLabelButton> skip_ad_controls_view_;
std::unique_ptr<views::CloseImageButton> close_controls_view_;
std::unique_ptr<views::ResizeHandleButton> resize_handle_view_;
std::unique_ptr<views::PlaybackImageButton> play_pause_controls_view_;
@@ -215,10 +209,6 @@
// used to toggle play/pause/replay button.
PlaybackState playback_state_for_testing_ = kEndOfVideo;
- // Whether or not the skip ad button will be shown. This is the
- // case when Media Session "skipad" action is handled by the website.
- bool show_skip_ad_button_ = false;
-
DISALLOW_COPY_AND_ASSIGN(OverlayWindowViews);
};
diff --git a/chrome/browser/ui/views/overlay/skip_ad_label_button.cc b/chrome/browser/ui/views/overlay/skip_ad_label_button.cc
deleted file mode 100644
index 2d15883..0000000
--- a/chrome/browser/ui/views/overlay/skip_ad_label_button.cc
+++ /dev/null
@@ -1,50 +0,0 @@
-// Copyright 2019 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 "chrome/browser/ui/views/overlay/skip_ad_label_button.h"
-
-#include "chrome/grit/generated_resources.h"
-#include "ui/base/l10n/l10n_util.h"
-#include "ui/gfx/color_palette.h"
-#include "ui/views/background.h"
-
-namespace {
-
-const int kSkipAdButtonWidth = 72;
-const int kSkipAdButtonHeight = 24;
-const int kSkipAdButtonMarginBottom = 40;
-
-constexpr SkColor kSkipAdButtonTextColor = SK_ColorWHITE;
-constexpr SkColor kSkipAdButtonBorderColor = SK_ColorWHITE;
-constexpr SkColor kSkipAdButtonBackgroundColor = gfx::kGoogleGrey700;
-
-} // namespace
-
-namespace views {
-
-SkipAdLabelButton::SkipAdLabelButton(ButtonListener* listener)
- : LabelButton(listener,
- l10n_util::GetStringUTF16(
- IDS_PICTURE_IN_PICTURE_SKIP_AD_CONTROL_TEXT)) {
- SetBackground(
- CreateBackgroundFromPainter(Painter::CreateRoundRectWith1PxBorderPainter(
- kSkipAdButtonBackgroundColor, kSkipAdButtonBorderColor, 1.f)));
- SetHorizontalAlignment(gfx::ALIGN_CENTER);
- SetEnabledTextColors(kSkipAdButtonTextColor);
- SetSize(gfx::Size(kSkipAdButtonWidth, kSkipAdButtonHeight));
-
- // Accessibility.
- SetFocusForPlatform();
- SetAccessibleName(label()->text());
- SetTooltipText(label()->text());
- SetInstallFocusRingOnFocus(true);
-}
-
-void SkipAdLabelButton::SetPosition(const gfx::Size& size) {
- LabelButton::SetPosition(gfx::Point(
- size.width() - kSkipAdButtonWidth + 1 /* border offset */,
- size.height() - kSkipAdButtonHeight - kSkipAdButtonMarginBottom));
-}
-
-} // namespace views
diff --git a/chrome/browser/ui/views/overlay/skip_ad_label_button.h b/chrome/browser/ui/views/overlay/skip_ad_label_button.h
deleted file mode 100644
index a3d545d..0000000
--- a/chrome/browser/ui/views/overlay/skip_ad_label_button.h
+++ /dev/null
@@ -1,28 +0,0 @@
-// Copyright 2019 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.
-
-#ifndef CHROME_BROWSER_UI_VIEWS_OVERLAY_SKIP_AD_LABEL_BUTTON_H_
-#define CHROME_BROWSER_UI_VIEWS_OVERLAY_SKIP_AD_LABEL_BUTTON_H_
-
-#include "chrome/browser/ui/views/overlay/overlay_window_views.h"
-#include "ui/views/controls/button/label_button.h"
-
-namespace views {
-
-// A label button representing a skip-ad button.
-class SkipAdLabelButton : public views::LabelButton {
- public:
- explicit SkipAdLabelButton(ButtonListener* listener);
- ~SkipAdLabelButton() override = default;
-
- // Sets the position of itself with an offset from the given window size.
- void SetPosition(const gfx::Size& size);
-
- private:
- DISALLOW_COPY_AND_ASSIGN(SkipAdLabelButton);
-};
-
-} // namespace views
-
-#endif // CHROME_BROWSER_UI_VIEWS_OVERLAY_SKIP_AD_LABEL_BUTTON_H_
diff --git a/chrome/test/data/media/picture-in-picture/window-size.html b/chrome/test/data/media/picture-in-picture/window-size.html
index b9f6977..57adcaeb 100644
--- a/chrome/test/data/media/picture-in-picture/window-size.html
+++ b/chrome/test/data/media/picture-in-picture/window-size.html
@@ -110,19 +110,9 @@
}
function addVisibilityChangeEventListener() {
- document.addEventListener('visibilitychange', () => {
- document.title = document.visibilityState;
- });
- }
-
- function setMediaSessionSkipAdActionHandler() {
- navigator.mediaSession.setActionHandler('skipad', _ => {
- document.title = 'skipad';
- });
- }
-
- function unsetMediaSessionSkipAdActionHandler() {
- navigator.mediaSession.setActionHandler('skipad', null);
+ document.addEventListener('visibilitychange', () => {
+ document.title = document.visibilityState;
+ });
}
</script>
</html>
diff --git a/chromecast/browser/cast_media_blocker_unittest.cc b/chromecast/browser/cast_media_blocker_unittest.cc
index 973fe35..f3a1170d5 100644
--- a/chromecast/browser/cast_media_blocker_unittest.cc
+++ b/chromecast/browser/cast_media_blocker_unittest.cc
@@ -44,7 +44,6 @@
MOCK_METHOD1(GetDebugInfo, void(GetDebugInfoCallback));
MOCK_METHOD0(PreviousTrack, void());
MOCK_METHOD0(NextTrack, void());
- MOCK_METHOD0(SkipAd, void());
MOCK_METHOD1(SetAudioFocusGroupId, void(const base::UnguessableToken&));
private:
diff --git a/content/browser/media/session/media_session_impl.cc b/content/browser/media/session/media_session_impl.cc
index d2a9592..0a2702a 100644
--- a/content/browser/media/session/media_session_impl.cc
+++ b/content/browser/media/session/media_session_impl.cc
@@ -81,8 +81,6 @@
return MediaSessionUserAction::SeekBackward;
case media_session::mojom::MediaSessionAction::kSeekForward:
return MediaSessionUserAction::SeekForward;
- case media_session::mojom::MediaSessionAction::kSkipAd:
- return MediaSessionUserAction::SkipAd;
case media_session::mojom::MediaSessionAction::kStop:
break;
}
@@ -839,10 +837,6 @@
DidReceiveAction(media_session::mojom::MediaSessionAction::kNextTrack);
}
-void MediaSessionImpl::SkipAd() {
- DidReceiveAction(media_session::mojom::MediaSessionAction::kSkipAd);
-}
-
void MediaSessionImpl::AbandonSystemAudioFocusIfNeeded() {
if (audio_focus_state_ == State::INACTIVE || !normal_players_.empty() ||
!pepper_players_.empty() || !one_shot_players_.empty()) {
diff --git a/content/browser/media/session/media_session_impl.h b/content/browser/media/session/media_session_impl.h
index 9ca371b..0fa48a29 100644
--- a/content/browser/media/session/media_session_impl.h
+++ b/content/browser/media/session/media_session_impl.h
@@ -242,9 +242,6 @@
// Skip to the next track.
CONTENT_EXPORT void NextTrack() override;
- // Skip ad.
- CONTENT_EXPORT void SkipAd() override;
-
const base::UnguessableToken& audio_focus_group_id() const {
return audio_focus_group_id_;
}
diff --git a/content/browser/media/session/media_session_impl_uma_unittest.cc b/content/browser/media/session/media_session_impl_uma_unittest.cc
index a1e659296..0ffec2d 100644
--- a/content/browser/media/session/media_session_impl_uma_unittest.cc
+++ b/content/browser/media/session/media_session_impl_uma_unittest.cc
@@ -61,7 +61,6 @@
{MediaSessionAction::kNextTrack, MediaSessionUserAction::NextTrack},
{MediaSessionAction::kSeekBackward, MediaSessionUserAction::SeekBackward},
{MediaSessionAction::kSeekForward, MediaSessionUserAction::SeekForward},
- {MediaSessionAction::kSkipAd, MediaSessionUserAction::SkipAd},
};
} // anonymous namespace
diff --git a/content/browser/media/session/media_session_uma_helper.h b/content/browser/media/session/media_session_uma_helper.h
index 4745e78..6b882eaf 100644
--- a/content/browser/media/session/media_session_uma_helper.h
+++ b/content/browser/media/session/media_session_uma_helper.h
@@ -41,8 +41,7 @@
NextTrack = 6,
SeekBackward = 7,
SeekForward = 8,
- SkipAd = 9,
- kMaxValue = SkipAd,
+ kMaxValue = SeekForward,
};
MediaSessionUmaHelper();
diff --git a/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.cc b/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.cc
index 50985897..1551d07c 100644
--- a/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.cc
+++ b/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.cc
@@ -10,11 +10,9 @@
#include "content/browser/web_contents/web_contents_impl.h"
#include "content/common/media/media_player_delegate_messages.h"
#include "content/public/browser/content_browser_client.h"
-#include "content/public/browser/media_session.h"
#include "content/public/browser/overlay_window.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_client.h"
-#include "media/base/media_switches.h"
namespace content {
@@ -54,8 +52,7 @@
PictureInPictureWindowControllerImpl::PictureInPictureWindowControllerImpl(
WebContents* initiator)
- : initiator_(static_cast<WebContentsImpl* const>(initiator)),
- observer_binding_(this) {
+ : initiator_(static_cast<WebContentsImpl* const>(initiator)) {
DCHECK(initiator_);
media_web_contents_observer_ = initiator_->media_web_contents_observer();
@@ -69,17 +66,8 @@
DCHECK(surface_id_.is_valid());
window_->Show();
- window_->SetSkipAdButtonVisibility(false);
initiator_->SetHasPictureInPictureVideo(true);
- if (observer_binding_.is_bound()) {
- observer_binding_.Close();
- }
-
- media_session::mojom::MediaSessionObserverPtr observer;
- observer_binding_.Bind(mojo::MakeRequest(&observer));
- MediaSession::Get(initiator_)->AddObserver(std::move(observer));
-
return window_->GetBounds().size();
}
@@ -213,27 +201,6 @@
window_->SetAlwaysHidePlayPauseButton(is_visible);
}
-void PictureInPictureWindowControllerImpl::SkipAd() {
- if (base::FeatureList::IsEnabled(media::kSkipAd))
- MediaSession::Get(initiator_)->SkipAd();
-}
-
-void PictureInPictureWindowControllerImpl::MediaSessionActionsChanged(
- const std::vector<media_session::mojom::MediaSessionAction>& actions) {
- if (!window_)
- return;
-
- // TODO(crbug.com/919842): Currently, the first Media Session to be created
- // (independently of the frame) will be used. This means, we could show a
- // Skip Ad button for a PiP video from another frame. Ideally, we should have
- // a Media Session per frame, not per tab. This is not implemented yet.
-
- auto result = std::find(std::begin(actions), std::end(actions),
- media_session::mojom::MediaSessionAction::kSkipAd);
- bool show_skip_ad_button = result != actions.end();
- window_->SetSkipAdButtonVisibility(show_skip_ad_button);
-}
-
void PictureInPictureWindowControllerImpl::OnLeavingPictureInPicture(
bool should_pause_video,
bool should_reset_pip_player) {
@@ -264,10 +231,6 @@
initiator_->SetHasPictureInPictureVideo(false);
OnLeavingPictureInPicture(should_pause_video, should_reset_pip_player);
-
- if (observer_binding_.is_bound()) {
- observer_binding_.Close();
- }
}
void PictureInPictureWindowControllerImpl::EnsureWindow() {
diff --git a/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h b/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h
index 6f302c5..71260f1 100644
--- a/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h
+++ b/content/browser/picture_in_picture/picture_in_picture_window_controller_impl.h
@@ -9,9 +9,6 @@
#include "components/viz/common/surfaces/parent_local_surface_id_allocator.h"
#include "content/public/browser/picture_in_picture_window_controller.h"
#include "content/public/browser/web_contents_user_data.h"
-#include "mojo/public/cpp/bindings/binding.h"
-#include "services/media_session/public/cpp/media_metadata.h"
-#include "services/media_session/public/mojom/media_session.mojom.h"
namespace content {
class OverlaySurfaceEmbedder;
@@ -26,8 +23,7 @@
// work with it. https://crbug.com/589840.
class PictureInPictureWindowControllerImpl
: public PictureInPictureWindowController,
- public WebContentsUserData<PictureInPictureWindowControllerImpl>,
- public media_session::mojom::MediaSessionObserver {
+ public WebContentsUserData<PictureInPictureWindowControllerImpl> {
public:
// Gets a reference to the controller associated with |initiator| and creates
// one if it does not exist. The returned pointer is guaranteed to be
@@ -57,16 +53,6 @@
CONTENT_EXPORT void UpdatePlaybackState(bool is_playing,
bool reached_end_of_stream) override;
CONTENT_EXPORT void SetAlwaysHidePlayPauseButton(bool is_visible) override;
- CONTENT_EXPORT void SkipAd() override;
-
- // media_session::mojom::MediaSessionObserver overrides.
- CONTENT_EXPORT void MediaSessionInfoChanged(
- media_session::mojom::MediaSessionInfoPtr session_info) override {}
- CONTENT_EXPORT void MediaSessionMetadataChanged(
- const base::Optional<media_session::MediaMetadata>& metadata) override {}
- CONTENT_EXPORT void MediaSessionActionsChanged(
- const std::vector<media_session::mojom::MediaSessionAction>& actions)
- override;
private:
friend class WebContentsUserData<PictureInPictureWindowControllerImpl>;
@@ -92,8 +78,6 @@
std::unique_ptr<OverlaySurfaceEmbedder> embedder_;
WebContentsImpl* const initiator_;
- mojo::Binding<media_session::mojom::MediaSessionObserver> observer_binding_;
-
// Used to determine the state of the media player and route messages to
// the corresponding media player with id |media_player_id_|.
MediaWebContentsObserver* media_web_contents_observer_;
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc
index 8ce1f5fc..28ba2bd 100644
--- a/content/browser/web_contents/web_contents_impl_unittest.cc
+++ b/content/browser/web_contents/web_contents_impl_unittest.cc
@@ -3272,7 +3272,6 @@
void UpdateVideoSize(const gfx::Size& natural_size) override {}
void SetPlaybackState(PlaybackState playback_state) override {}
void SetAlwaysHidePlayPauseButton(bool is_visible) override {}
- void SetSkipAdButtonVisibility(bool is_visible) override {}
ui::Layer* GetWindowBackgroundLayer() override { return nullptr; }
ui::Layer* GetVideoLayer() override { return nullptr; }
gfx::Rect GetVideoBounds() override { return gfx::Rect(); }
diff --git a/content/public/browser/media_session.h b/content/public/browser/media_session.h
index 969bd126..b08533a5e 100644
--- a/content/public/browser/media_session.h
+++ b/content/public/browser/media_session.h
@@ -78,9 +78,6 @@
// no-op.
void NextTrack() override = 0;
- // Skip ad.
- void SkipAd() override = 0;
-
// Seek the media session. If the media cannot seek then this will be a no-op.
// The |seek_time| is the time delta that the media will seek by and supports
// both positive and negative values.
diff --git a/content/public/browser/overlay_window.h b/content/public/browser/overlay_window.h
index 8eeeafd..a0d8a5b 100644
--- a/content/public/browser/overlay_window.h
+++ b/content/public/browser/overlay_window.h
@@ -59,7 +59,6 @@
virtual void SetPictureInPictureCustomControls(
const std::vector<blink::PictureInPictureControlInfo>& controls) = 0;
virtual void SetAlwaysHidePlayPauseButton(bool is_visible) = 0;
- virtual void SetSkipAdButtonVisibility(bool is_visible) = 0;
// Retrieves the ui::Layers corresponding to the window and video.
virtual ui::Layer* GetWindowBackgroundLayer() = 0;
diff --git a/content/public/browser/picture_in_picture_window_controller.h b/content/public/browser/picture_in_picture_window_controller.h
index 979086f7..e8ca224 100644
--- a/content/public/browser/picture_in_picture_window_controller.h
+++ b/content/public/browser/picture_in_picture_window_controller.h
@@ -67,9 +67,6 @@
bool reached_end_of_stream) = 0;
virtual void SetAlwaysHidePlayPauseButton(bool is_visible) = 0;
- // Called when the user interacts with the "Skip Ad" control.
- virtual void SkipAd() = 0;
-
// Commands.
// Returns true if the player is active (i.e. currently playing) after this
// call.
diff --git a/content/shell/browser/web_test/web_test_content_browser_client.cc b/content/shell/browser/web_test/web_test_content_browser_client.cc
index 5352c9cc..13627e8e 100644
--- a/content/shell/browser/web_test/web_test_content_browser_client.cc
+++ b/content/shell/browser/web_test/web_test_content_browser_client.cc
@@ -76,7 +76,6 @@
void UpdateVideoSize(const gfx::Size& natural_size) override {}
void SetPlaybackState(PlaybackState playback_state) override {}
void SetAlwaysHidePlayPauseButton(bool is_visible) override {}
- void SetSkipAdButtonVisibility(bool is_visible) override {}
ui::Layer* GetWindowBackgroundLayer() override { return nullptr; }
ui::Layer* GetVideoLayer() override { return nullptr; }
gfx::Rect GetVideoBounds() override { return gfx::Rect(); }
diff --git a/media/base/media_switches.cc b/media/base/media_switches.cc
index 30c4bc1c..fb979f3 100644
--- a/media/base/media_switches.cc
+++ b/media/base/media_switches.cc
@@ -204,9 +204,6 @@
#endif
};
-// Show Skip Ad button in Picture-in-Picture window.
-const base::Feature kSkipAd{"SkipAd", base::FEATURE_DISABLED_BY_DEFAULT};
-
// Only decode preload=metadata elements upon visibility?
const base::Feature kPreloadMetadataLazyLoad{"PreloadMetadataLazyLoad",
base::FEATURE_DISABLED_BY_DEFAULT};
diff --git a/media/base/media_switches.h b/media/base/media_switches.h
index 9c19f204..2f46cb062 100644
--- a/media/base/media_switches.h
+++ b/media/base/media_switches.h
@@ -130,7 +130,6 @@
MEDIA_EXPORT extern const base::Feature kRecordWebAudioEngagement;
MEDIA_EXPORT extern const base::Feature kResumeBackgroundVideo;
MEDIA_EXPORT extern const base::Feature kSpecCompliantCanPlayThrough;
-MEDIA_EXPORT extern const base::Feature kSkipAd;
MEDIA_EXPORT extern const base::Feature kUnifiedAutoplay;
MEDIA_EXPORT extern const base::Feature kUseAndroidOverlay;
MEDIA_EXPORT extern const base::Feature kUseAndroidOverlayAggressively;
diff --git a/services/media_session/public/cpp/test/mock_media_session.h b/services/media_session/public/cpp/test/mock_media_session.h
index 8294306..2312fc7 100644
--- a/services/media_session/public/cpp/test/mock_media_session.h
+++ b/services/media_session/public/cpp/test/mock_media_session.h
@@ -98,7 +98,6 @@
void GetDebugInfo(GetDebugInfoCallback callback) override;
void PreviousTrack() override;
void NextTrack() override;
- void SkipAd() override{};
void Seek(base::TimeDelta seek_time) override;
void Stop(SuspendType type) override;
diff --git a/services/media_session/public/mojom/media_session.mojom b/services/media_session/public/mojom/media_session.mojom
index 6341525..c06c51e3 100644
--- a/services/media_session/public/mojom/media_session.mojom
+++ b/services/media_session/public/mojom/media_session.mojom
@@ -25,7 +25,6 @@
kNextTrack,
kSeekBackward,
kSeekForward,
- kSkipAd,
kStop,
};
@@ -111,7 +110,7 @@
// WebContents or ARC app.
// TODO(https://crbug.com/875004): migrate media session from content/public
// to mojo.
-// Next Method ID: 12
+// Next Method ID: 11
interface MediaSession {
[Extensible]
enum SuspendType {
@@ -165,7 +164,4 @@
// Stop the media session.
// |type| represents the origin of the request.
Stop@10(SuspendType suspend_type);
-
- // Skip ad.
- SkipAd@11();
};
diff --git a/third_party/blink/renderer/modules/mediasession/media_session.cc b/third_party/blink/renderer/modules/mediasession/media_session.cc
index 920b7a4..6173ecf 100644
--- a/third_party/blink/renderer/modules/mediasession/media_session.cc
+++ b/third_party/blink/renderer/modules/mediasession/media_session.cc
@@ -33,7 +33,6 @@
("seekbackward"));
DEFINE_STATIC_LOCAL(const AtomicString, seek_forward_action_name,
("seekforward"));
- DEFINE_STATIC_LOCAL(const AtomicString, skip_ad_action_name, ("skipad"));
switch (action) {
case MediaSessionAction::kPlay:
@@ -48,8 +47,6 @@
return seek_backward_action_name;
case MediaSessionAction::kSeekForward:
return seek_forward_action_name;
- case MediaSessionAction::kSkipAd:
- return skip_ad_action_name;
default:
NOTREACHED();
}
@@ -70,8 +67,6 @@
return MediaSessionAction::kSeekBackward;
if ("seekforward" == action_name)
return MediaSessionAction::kSeekForward;
- if ("skipad" == action_name)
- return MediaSessionAction::kSkipAd;
NOTREACHED();
return base::nullopt;
diff --git a/third_party/blink/renderer/modules/mediasession/media_session.idl b/third_party/blink/renderer/modules/mediasession/media_session.idl
index 9057544..e47398d 100644
--- a/third_party/blink/renderer/modules/mediasession/media_session.idl
+++ b/third_party/blink/renderer/modules/mediasession/media_session.idl
@@ -18,8 +18,7 @@
"previoustrack",
"nexttrack",
"seekbackward",
- "seekforward",
- "skipad",
+ "seekforward"
};
callback MediaSessionActionHandler = void ();
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index f6da139..808a166 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -34568,7 +34568,6 @@
<int value="6" label="Next track"/>
<int value="7" label="Seek backward"/>
<int value="8" label="Seek forward"/>
- <int value="9" label="Skip ad"/>
</enum>
<enum name="MediaSinkType">
