Skip to content

Commit

Permalink
cros: make VK_MEDIA_PLAY_PAUSE handle media contents on chrome browser
Browse files Browse the repository at this point in the history
changes:
(1) Provide the support to make VK_MEDIA_PLAY_PAUSE handle toggling play
and pause on active browser's active tab. If there is an active browser,
then instead of dispatching this event, handle active tab's media session
play pause.
(2) add FindBrowserWithActiveWindow() in browser_finder.h.
(3) add Activate/IsActive in TestBrowserWindowAura, which is hooked to
the aura window's activation.
(4) refactor test in MultiUserWindowManagerChromeOSTest for new change.

state on active browser's active tab.

Bug: 763483
Test: tested on device, the play/pause button could toggle the playback
Change-Id: Ied566bf1f6c84edeaa762bb3a6e133720c1291c5
Reviewed-on: https://chromium-review.googlesource.com/666199
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Qiang(Joe) Xu <warx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503510}
  • Loading branch information
Qiang Xu authored and Commit Bot committed Sep 21, 2017
1 parent 878325c commit cb6871b
Show file tree
Hide file tree
Showing 12 changed files with 84 additions and 60 deletions.
24 changes: 5 additions & 19 deletions chrome/browser/ui/ash/chrome_new_window_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_list.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/chrome_pages.h"
#include "chrome/browser/ui/extensions/app_launch_params.h"
Expand All @@ -33,7 +32,6 @@
#include "extensions/common/constants.h"
#include "services/service_manager/public/cpp/connector.h"
#include "ui/base/window_open_disposition.h"
#include "ui/wm/public/activation_client.h"

namespace {

Expand All @@ -45,19 +43,6 @@ void RestoreTabUsingProfile(Profile* profile) {

} // namespace

// static
Browser* ChromeNewWindowClient::GetActiveBrowser() {
Browser* browser = BrowserList::GetInstance()->GetLastActive();
if (browser) {
aura::Window* window = browser->window()->GetNativeWindow();
wm::ActivationClient* client =
wm::GetActivationClient(window->GetRootWindow());
if (client->GetActiveWindow() == window)
return browser;
}
return nullptr;
}

ChromeNewWindowClient::ChromeNewWindowClient() : binding_(this) {
service_manager::Connector* connector =
content::ServiceManagerConnection::GetForProcess()->GetConnector();
Expand Down Expand Up @@ -115,7 +100,7 @@ class ChromeNewWindowClient::TabRestoreHelper
};

void ChromeNewWindowClient::NewTab() {
Browser* browser = GetActiveBrowser();
Browser* browser = chrome::FindBrowserWithActiveWindow();
if (browser && browser->is_type_tabbed()) {
chrome::NewTab(browser);
return;
Expand All @@ -133,7 +118,7 @@ void ChromeNewWindowClient::NewTab() {
}

void ChromeNewWindowClient::NewWindow(bool is_incognito) {
Browser* browser = GetActiveBrowser();
Browser* browser = chrome::FindBrowserWithActiveWindow();
Profile* profile = (browser && browser->profile())
? browser->profile()->GetOriginalProfile()
: ProfileManager::GetActiveUserProfile();
Expand Down Expand Up @@ -186,7 +171,7 @@ void ChromeNewWindowClient::RestoreTab() {
return;
}

Browser* browser = GetActiveBrowser();
Browser* browser = chrome::FindBrowserWithActiveWindow();
Profile* profile = browser ? browser->profile() : nullptr;
if (!profile)
profile = ProfileManager::GetActiveUserProfile();
Expand Down Expand Up @@ -218,5 +203,6 @@ void ChromeNewWindowClient::ShowTaskManager() {
}

void ChromeNewWindowClient::OpenFeedbackPage() {
chrome::OpenFeedbackDialog(GetActiveBrowser(), chrome::kFeedbackSourceAsh);
chrome::OpenFeedbackDialog(chrome::FindBrowserWithActiveWindow(),
chrome::kFeedbackSourceAsh);
}
5 changes: 0 additions & 5 deletions chrome/browser/ui/ash/chrome_new_window_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,11 @@
#include "base/macros.h"
#include "mojo/public/cpp/bindings/associated_binding.h"

class Browser;

class ChromeNewWindowClient : public ash::mojom::NewWindowClient {
public:
ChromeNewWindowClient();
~ChromeNewWindowClient() override;

// Returns the active browser that has active browser window, if any.
static Browser* GetActiveBrowser();

// Overridden from ash::mojom::NewWindowClient:
void NewTab() override;
void NewWindow(bool incognito) override;
Expand Down
15 changes: 15 additions & 0 deletions chrome/browser/ui/ash/media_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/ui/ash/chrome_shell_content_state.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_list.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/tab_contents/tab_contents_iterator.h"
Expand Down Expand Up @@ -153,6 +154,20 @@ void MediaClient::HandleMediaNextTrack() {
}

void MediaClient::HandleMediaPlayPause() {
// If there is an active browser, then instead of dispatching this event,
// handle active tab's media session play pause.
Browser* browser = chrome::FindBrowserWithActiveWindow();
if (browser) {
content::MediaSession* media_session = content::MediaSession::Get(
browser->tab_strip_model()->GetActiveWebContents());
if (media_session->IsControllable()) {
if (media_session->IsActuallyPaused())
media_session->Resume(content::MediaSession::SuspendType::UI);
else
media_session->Suspend(content::MediaSession::SuspendType::UI);
return;
}
}
extensions::MediaPlayerAPI::Get(ProfileManager::GetActiveUserProfile())
->media_player_event_router()
->NotifyTogglePlayState();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include "chrome/browser/ui/ash/session_controller_client.h"
#include "chrome/browser/ui/ash/session_util.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_list.h"
#include "chrome/test/base/test_browser_window_aura.h"
#include "chrome/test/base/testing_browser_process.h"
Expand Down Expand Up @@ -1600,10 +1601,10 @@ TEST_F(MultiUserWindowManagerChromeOSTest, WindowsOrderPreservedTests) {
EXPECT_EQ(mru_list[2], window(2));
}

// Tests that ChromeNewWindowClient::GetActiveBrowser works properly in
// Tests that chrome::FindBrowserWithActiveWindow works properly in
// multi-user scenario, that is it should return the browser with active window
// associated with it (crbug.com/675265).
TEST_F(MultiUserWindowManagerChromeOSTest, GetActiveBrowserTest) {
TEST_F(MultiUserWindowManagerChromeOSTest, FindBrowserWithActiveWindow) {
SetUpForThisManyWindows(1);

const AccountId account_id_A(AccountId::FromUserEmail("A"));
Expand All @@ -1614,28 +1615,25 @@ TEST_F(MultiUserWindowManagerChromeOSTest, GetActiveBrowserTest) {
multi_user_window_manager()->ActiveUserChanged(
user_manager()->FindUser(account_id_A));

::wm::ActivationClient* activation_client =
::wm::GetActivationClient(window(0)->GetRootWindow());
multi_user_window_manager()->SetWindowOwner(window(0), account_id_A);
Profile* profile = multi_user_util::GetProfileFromAccountId(account_id_A);
Browser::CreateParams params(profile, true);
std::unique_ptr<Browser> browser(CreateTestBrowser(
CreateTestWindowInShellWithId(0), gfx::Rect(16, 32, 640, 320), &params));
aura::Window* browser_native_window = browser->window()->GetNativeWindow();
activation_client->ActivateWindow(browser_native_window);
browser->window()->Activate();
// Manually set last active browser in BrowserList for testing.
BrowserList::GetInstance()->SetLastActive(browser.get());
EXPECT_EQ(browser.get(), BrowserList::GetInstance()->GetLastActive());
EXPECT_EQ(browser_native_window, activation_client->GetActiveWindow());
EXPECT_EQ(browser.get(), ChromeNewWindowClient::GetActiveBrowser());
EXPECT_TRUE(browser->window()->IsActive());
EXPECT_EQ(browser.get(), chrome::FindBrowserWithActiveWindow());

// Switch to another user's desktop with no active window.
user_manager()->SwitchActiveUser(account_id_B);
multi_user_window_manager()->ActiveUserChanged(
user_manager()->FindUser(account_id_B));
EXPECT_EQ(browser.get(), BrowserList::GetInstance()->GetLastActive());
EXPECT_EQ(nullptr, activation_client->GetActiveWindow());
EXPECT_EQ(nullptr, ChromeNewWindowClient::GetActiveBrowser());
EXPECT_FALSE(browser->window()->IsActive());
EXPECT_EQ(nullptr, chrome::FindBrowserWithActiveWindow());
}

} // namespace ash
5 changes: 5 additions & 0 deletions chrome/browser/ui/browser_finder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,11 @@ Browser* FindBrowserWithWindow(gfx::NativeWindow window) {
return NULL;
}

Browser* FindBrowserWithActiveWindow() {
Browser* browser = BrowserList::GetInstance()->GetLastActive();
return browser && browser->window()->IsActive() ? browser : nullptr;
}

Browser* FindBrowserWithWebContents(const WebContents* web_contents) {
DCHECK(web_contents);
for (TabContentsIterator it; !it.done(); it.Next()) {
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/ui/browser_finder.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ Browser* FindBrowserWithID(SessionID::id_type desired_id);
// Find the browser represented by |window| or NULL if not found.
Browser* FindBrowserWithWindow(gfx::NativeWindow window);

// Find the browser with active window or NULL if not found.
Browser* FindBrowserWithActiveWindow();

// Find the browser containing |web_contents| or NULL if none is found.
// |web_contents| must not be NULL.
Browser* FindBrowserWithWebContents(const content::WebContents* web_contents);
Expand Down
11 changes: 11 additions & 0 deletions chrome/test/base/test_browser_window_aura.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <utility>

#include "base/memory/ptr_util.h"
#include "ui/wm/public/activation_client.h"

namespace chrome {

Expand Down Expand Up @@ -47,6 +48,16 @@ void TestBrowserWindowAura::Hide() {
native_window_->Hide();
}

void TestBrowserWindowAura::Activate() {
::wm::GetActivationClient(native_window_->GetRootWindow())
->ActivateWindow(native_window_.get());
}

bool TestBrowserWindowAura::IsActive() const {
return ::wm::GetActivationClient(native_window_->GetRootWindow())
->GetActiveWindow() == native_window_.get();
}

gfx::Rect TestBrowserWindowAura::GetBounds() const {
return native_window_->bounds();
}
Expand Down
2 changes: 2 additions & 0 deletions chrome/test/base/test_browser_window_aura.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ class TestBrowserWindowAura : public TestBrowserWindow {
gfx::NativeWindow GetNativeWindow() const override;
void Show() override;
void Hide() override;
void Activate() override;
bool IsActive() const override;
gfx::Rect GetBounds() const override;

std::unique_ptr<Browser> CreateBrowser(Browser::CreateParams* params);
Expand Down
2 changes: 2 additions & 0 deletions chromecast/browser/cast_media_blocker_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ class MockMediaSession : public content::MediaSession {
MOCK_METHOD1(Resume, void(content::MediaSession::SuspendType));
MOCK_METHOD1(Suspend, void(content::MediaSession::SuspendType));
MOCK_METHOD1(Stop, void(content::MediaSession::SuspendType));
MOCK_CONST_METHOD0(IsControllable, bool());
MOCK_CONST_METHOD0(IsActuallyPaused, bool());
MOCK_METHOD0(StartDucking, void());
MOCK_METHOD0(StopDucking, void());
MOCK_METHOD1(DidReceiveAction, void(blink::mojom::MediaSessionAction));
Expand Down
36 changes: 18 additions & 18 deletions content/browser/media/session/media_session_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,24 @@ void MediaSessionImpl::Stop(SuspendType suspend_type) {
AbandonSystemAudioFocusIfNeeded();
}

bool MediaSessionImpl::IsControllable() const {
// Only media session having focus Gain can be controllable unless it is
// inactive. Also, the session will be uncontrollable if it contains one-shot
// players.
return audio_focus_state_ != State::INACTIVE &&
audio_focus_type_ == AudioFocusManager::AudioFocusType::Gain &&
one_shot_players_.empty();
}

bool MediaSessionImpl::IsActuallyPaused() const {
if (routed_service_ && routed_service_->playback_state() ==
blink::mojom::MediaSessionPlaybackState::PLAYING) {
return false;
}

return !IsActive();
}

void MediaSessionImpl::StartDucking() {
if (is_ducking_)
return;
Expand Down Expand Up @@ -428,24 +446,6 @@ bool MediaSessionImpl::IsSuspended() const {
return audio_focus_state_ == State::SUSPENDED;
}

bool MediaSessionImpl::IsControllable() const {
// Only media session having focus Gain can be controllable unless it is
// inactive. Also, the session will be uncontrollable if it contains one-shot
// players.
return audio_focus_state_ != State::INACTIVE &&
audio_focus_type_ == AudioFocusManager::AudioFocusType::Gain &&
one_shot_players_.empty();
}

bool MediaSessionImpl::IsActuallyPaused() const {
if (routed_service_ && routed_service_->playback_state() ==
blink::mojom::MediaSessionPlaybackState::PLAYING) {
return false;
}

return !IsActive();
}

bool MediaSessionImpl::HasPepper() const {
return !pepper_players_.empty();
}
Expand Down
16 changes: 8 additions & 8 deletions content/browser/media/session/media_session_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,14 @@ class MediaSessionImpl : public MediaSession,
// |type| represents the origin of the request.
CONTENT_EXPORT void Stop(MediaSession::SuspendType suspend_type) override;

// Returns if the session can be controlled by Resume() and Suspend() calls
// above.
CONTENT_EXPORT bool IsControllable() const override;

// Compute if the actual playback state is paused by combining the
// MediaSessionService declared state and guessed state (audio_focus_state_).
CONTENT_EXPORT bool IsActuallyPaused() const override;

// Let the media session start ducking such that the volume multiplier is
// reduced.
CONTENT_EXPORT void StartDucking() override;
Expand All @@ -138,10 +146,6 @@ class MediaSessionImpl : public MediaSession,
void AddObserver(MediaSessionObserver* observer) override;
void RemoveObserver(MediaSessionObserver* observer) override;

// Returns if the session can be controlled by Resume() and Suspend calls
// above.
CONTENT_EXPORT bool IsControllable() const;

// Returns if the session is currently active.
CONTENT_EXPORT bool IsActive() const;

Expand Down Expand Up @@ -253,10 +257,6 @@ class MediaSessionImpl : public MediaSession,
// ducking.
double GetVolumeMultiplier() const;

// Compute if the actual playback state is paused by combining the
// MediaSessionService declared state and guessed state (audio_focus_state_).
bool IsActuallyPaused() const;

// Registers a MediaSessionImpl state change callback.
CONTENT_EXPORT std::unique_ptr<base::CallbackList<void(State)>::Subscription>
RegisterMediaSessionStateChangedCallbackForTest(
Expand Down
7 changes: 7 additions & 0 deletions content/public/browser/media_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ class MediaSession {
// |type| represents the origin of the request.
virtual void Stop(SuspendType suspend_type) = 0;

// Return if the session can be controlled by Resume() and Suspend() calls
// above.
virtual bool IsControllable() const = 0;

// Return if the actual playback state is paused.
virtual bool IsActuallyPaused() const = 0;

// Tell the media session a user action has performed.
virtual void DidReceiveAction(blink::mojom::MediaSessionAction action) = 0;

Expand Down

0 comments on commit cb6871b

Please sign in to comment.