Skip to content

Commit

Permalink
Reland "Add Skip Ad button to Picture-in-Picture window."
Browse files Browse the repository at this point in the history
This is a reland of b0281b1

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

Bug: 910436
Change-Id: Id6b275daeef55552651d4272f98ff527b1d82230
Reviewed-on: https://chromium-review.googlesource.com/c/1408250
Commit-Queue: François Beaufort <beaufort.francois@gmail.com>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622411}
  • Loading branch information
beaufortfrancois authored and Commit Bot committed Jan 14, 2019
1 parent 29499f8 commit a376eb7
Show file tree
Hide file tree
Showing 32 changed files with 350 additions and 10 deletions.
2 changes: 2 additions & 0 deletions ash/media/media_notification_item.cc
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ void MediaNotificationItem::OnNotificationClicked(
case MediaSessionAction::kStop:
media_controller_ptr_->Stop();
break;
case MediaSessionAction::kSkipAd:
break;
}
}

Expand Down
3 changes: 3 additions & 0 deletions chrome/app/generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -5630,6 +5630,9 @@ the Bookmarks menu.">
<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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,8 @@ void ArcPictureInPictureWindowControllerImpl::SetAlwaysHidePlayPauseButton(
// 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
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class ArcPictureInPictureWindowControllerImpl
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_;
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/picture_in_picture/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ specific_include_rules = {
"+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",
],
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#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"
Expand All @@ -24,15 +25,19 @@
#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
Expand Down Expand Up @@ -73,6 +78,7 @@ class MockPictureInPictureWindowController
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);
Expand Down Expand Up @@ -1765,3 +1771,97 @@ IN_PROC_BROWSER_TEST_F(PictureInPictureWindowControllerBrowserTest,
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());
}
2 changes: 2 additions & 0 deletions chrome/browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2644,6 +2644,8 @@ jumbo_split_static_library("ui") {
"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",
Expand Down
40 changes: 39 additions & 1 deletion chrome/browser/ui/views/overlay/overlay_window_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@
#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"
Expand Down Expand Up @@ -115,6 +117,7 @@ class OverlayWindowFrameView : public views::NonClientFrameView {
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) ||
Expand Down Expand Up @@ -186,6 +189,7 @@ OverlayWindowViews::OverlayWindowViews(
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)),
Expand Down Expand Up @@ -332,8 +336,15 @@ void OverlayWindowViews::SetUpViews() {
// 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);
Expand All @@ -357,10 +368,11 @@ void OverlayWindowViews::SetUpViews() {
resize_handle_view_->set_owned_by_client();
#endif

// Set up view::Views heirarchy. --------------------------------------------
// Set up view::Views hierarchy. --------------------------------------------
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)
Expand Down Expand Up @@ -405,6 +417,7 @@ void OverlayWindowViews::UpdateControlsVisibility(bool is_visible) {
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)
Expand All @@ -422,6 +435,7 @@ void OverlayWindowViews::UpdateControlsBounds() {

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);
Expand Down Expand Up @@ -614,6 +628,11 @@ void OverlayWindowViews::SetAlwaysHidePlayPauseButton(bool is_visible) {
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.
Expand Down Expand Up @@ -789,6 +808,9 @@ void OverlayWindowViews::OnGestureEvent(ui::GestureEvent* event) {
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 */);
Expand All @@ -804,6 +826,9 @@ void OverlayWindowViews::ButtonPressed(views::Button* sender,
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 */);
Expand All @@ -822,6 +847,10 @@ gfx::Rect OverlayWindowViews::GetBackToTabControlsBounds() {
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();
}
Expand Down Expand Up @@ -862,6 +891,10 @@ ui::Layer* OverlayWindowViews::GetBackToTabControlsLayer() {
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();
}
Expand Down Expand Up @@ -891,6 +924,11 @@ gfx::Point OverlayWindowViews::back_to_tab_image_position_for_testing() const {
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();
}
Expand Down
12 changes: 11 additions & 1 deletion chrome/browser/ui/views/overlay/overlay_window_views.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class ControlImageButton;
class CloseImageButton;
class PlaybackImageButton;
class ResizeHandleButton;
class SkipAdLabelButton;
} // namespace views

// The Chrome desktop implementation of OverlayWindow. This will only be
Expand All @@ -47,6 +48,7 @@ class OverlayWindowViews : public content::OverlayWindow,
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;
Expand All @@ -70,6 +72,7 @@ class OverlayWindowViews : public content::OverlayWindow,

// Gets the bounds of the controls.
gfx::Rect GetBackToTabControlsBounds();
gfx::Rect GetSkipAdControlsBounds();
gfx::Rect GetCloseControlsBounds();
gfx::Rect GetResizeHandleControlsBounds();
gfx::Rect GetPlayPauseControlsBounds();
Expand All @@ -86,6 +89,7 @@ class OverlayWindowViews : public content::OverlayWindow,

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;
Expand Down Expand Up @@ -139,6 +143,7 @@ class OverlayWindowViews : public content::OverlayWindow,

ui::Layer* GetControlsScrimLayer();
ui::Layer* GetBackToTabControlsLayer();
ui::Layer* GetSkipAdControlsLayer();
ui::Layer* GetCloseControlsLayer();
ui::Layer* GetResizeHandleLayer();
ui::Layer* GetControlsParentLayer();
Expand Down Expand Up @@ -190,9 +195,10 @@ class OverlayWindowViews : public content::OverlayWindow,
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 and close views.
// the back-to-tab, close and skip-ad 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_;
Expand All @@ -209,6 +215,10 @@ class OverlayWindowViews : public content::OverlayWindow,
// 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);
};

Expand Down
Loading

0 comments on commit a376eb7

Please sign in to comment.