Skip to content

Commit

Permalink
Replace MediaController with media::FlingingController
Browse files Browse the repository at this point in the history
FlingingRenderer needs to be updated when status changes happen on a
device that it is currently remoting to. Instead of coupling the
MediaController interface to the MediaStatusObserver interface, it is
simpler to add a new interface that groups the MC and MSO.

The best name for this interface is probably MediaRouteController, but
there is a naming collision with media_router::MediaRouteController.
mr::MRC is a class with baked in desktop and Mojo concepts, and
precise lifetime management, that is not immediatly extractable into
an interface. It is simpler to add a new interface now, and have
mr::MRC implement this interface later.

This CL adds the FlingingController as a high level, simple interface.
It will eventually be renamed to MediaRouteController, as part of
the work to unify the desktop and mobile remoting experience
(see crbug.com/820277).

Bug: 790766
Change-Id: I8248f2fd4b2022c371f37a8fd73af267b3735e7e
Reviewed-on: https://chromium-review.googlesource.com/1136008
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576942}
  • Loading branch information
tguilbert-google authored and Commit Bot committed Jul 20, 2018
1 parent 7a20ffc commit fc4431b
Show file tree
Hide file tree
Showing 24 changed files with 232 additions and 145 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -390,14 +390,14 @@ public void sendStringMessage(String routeId, String message, int callbackId) {
*/
@Nullable
@CalledByNative
public MediaControllerBridge getMediaControllerBridge(String routeId) {
public FlingingControllerBridge getFlingingControllerBridge(String routeId) {
MediaRouteProvider provider = mRouteIdsToProviders.get(routeId);
if (provider == null) return null;

MediaController controller = provider.getMediaController(routeId);
if (controller == null) return null;

return new MediaControllerBridge(controller);
return new FlingingControllerBridge(controller);
}

@VisibleForTesting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@
import org.chromium.base.annotations.JNINamespace;

/**
* A wrapper around a MediaController that allows the native code to use it.
* See chrome/browser/media/android/remote/media_controller_bridge.h for the corresponding native
* code.
* A wrapper around a MediaController that allows the native code to use it, and to subscribe to
* status changes. See chrome/browser/media/android/remote/flinging_controller_bridge.h for the
* corresponding native code.
*/
@JNINamespace("media_router")
public class MediaControllerBridge {
public class FlingingControllerBridge {
private final MediaController mMediaController;

public MediaControllerBridge(MediaController mediaController) {
public FlingingControllerBridge(MediaController mediaController) {
mMediaController = mediaController;
}

Expand Down
2 changes: 1 addition & 1 deletion chrome/android/java_sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -723,8 +723,8 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/media/router/ChromeMediaRouterDialogController.java",
"java/src/org/chromium/chrome/browser/media/router/DiscoveryCallback.java",
"java/src/org/chromium/chrome/browser/media/router/DiscoveryDelegate.java",
"java/src/org/chromium/chrome/browser/media/router/FlingingControllerBridge.java",
"java/src/org/chromium/chrome/browser/media/router/MediaController.java",
"java/src/org/chromium/chrome/browser/media/router/MediaControllerBridge.java",
"java/src/org/chromium/chrome/browser/media/router/MediaRoute.java",
"java/src/org/chromium/chrome/browser/media/router/MediaRouteChooserDialogManager.java",
"java/src/org/chromium/chrome/browser/media/router/MediaRouteControllerDialogManager.java",
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2338,8 +2338,8 @@ jumbo_split_static_library("browser") {
"media/android/cdm/media_drm_license_manager.h",
"media/android/cdm/media_drm_storage_factory.cc",
"media/android/cdm/media_drm_storage_factory.h",
"media/android/remote/media_controller_bridge.cc",
"media/android/remote/media_controller_bridge.h",
"media/android/remote/flinging_controller_bridge.cc",
"media/android/remote/flinging_controller_bridge.h",
"media/android/remote/record_cast_action.cc",
"media/android/remote/remote_media_player_bridge.cc",
"media/android/remote/remote_media_player_bridge.h",
Expand Down Expand Up @@ -4475,7 +4475,7 @@ if (is_android) {
"../android/java/src/org/chromium/chrome/browser/media/remote/RemoteMediaPlayerBridge.java",
"../android/java/src/org/chromium/chrome/browser/media/router/ChromeMediaRouter.java",
"../android/java/src/org/chromium/chrome/browser/media/router/ChromeMediaRouterDialogController.java",
"../android/java/src/org/chromium/chrome/browser/media/router/MediaControllerBridge.java",
"../android/java/src/org/chromium/chrome/browser/media/router/FlingingControllerBridge.java",
"../android/java/src/org/chromium/chrome/browser/metrics/LaunchMetrics.java",
"../android/java/src/org/chromium/chrome/browser/metrics/PageLoadMetrics.java",
"../android/java/src/org/chromium/chrome/browser/metrics/UmaSessionStats.java",
Expand Down
73 changes: 73 additions & 0 deletions chrome/browser/media/android/remote/flinging_controller_bridge.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// Copyright 2018 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/media/android/remote/flinging_controller_bridge.h"

#include "jni/FlingingControllerBridge_jni.h"

namespace media_router {

FlingingControllerBridge::FlingingControllerBridge(
base::android::ScopedJavaGlobalRef<jobject> controller)
: j_flinging_controller_bridge_(controller) {}

FlingingControllerBridge::~FlingingControllerBridge() = default;

void FlingingControllerBridge::Play() {
JNIEnv* env = base::android::AttachCurrentThread();
DCHECK(env);

Java_FlingingControllerBridge_play(env, j_flinging_controller_bridge_);
}

void FlingingControllerBridge::Pause() {
JNIEnv* env = base::android::AttachCurrentThread();
DCHECK(env);

Java_FlingingControllerBridge_pause(env, j_flinging_controller_bridge_);
}

void FlingingControllerBridge::SetMute(bool mute) {
JNIEnv* env = base::android::AttachCurrentThread();
DCHECK(env);

Java_FlingingControllerBridge_setMute(env, j_flinging_controller_bridge_,
mute);
}

void FlingingControllerBridge::SetVolume(float volume) {
JNIEnv* env = base::android::AttachCurrentThread();
DCHECK(env);

Java_FlingingControllerBridge_setVolume(env, j_flinging_controller_bridge_,
volume);
}

void FlingingControllerBridge::Seek(base::TimeDelta time) {
JNIEnv* env = base::android::AttachCurrentThread();
DCHECK(env);

Java_FlingingControllerBridge_seek(env, j_flinging_controller_bridge_,
time.InMilliseconds());
}

media::MediaController* FlingingControllerBridge::GetMediaController() {
return this;
}

void FlingingControllerBridge::AddMediaStatusObserver(
media::MediaStatusObserver* observer) {
NOTREACHED();
}
void FlingingControllerBridge::RemoveMediaStatusObserver(
media::MediaStatusObserver* observer) {
NOTREACHED();
}

base::TimeDelta FlingingControllerBridge::GetApproximateCurrentTime() {
// TODO(https://crbug.com/830871): Implement this method.
return base::TimeDelta();
}

} // namespace media_router
45 changes: 45 additions & 0 deletions chrome/browser/media/android/remote/flinging_controller_bridge.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright 2018 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_MEDIA_ANDROID_REMOTE_FLINGING_CONTROLLER_BRIDGE_H_
#define CHROME_BROWSER_MEDIA_ANDROID_REMOTE_FLINGING_CONTROLLER_BRIDGE_H_

#include "base/android/scoped_java_ref.h"
#include "base/time/time.h"
#include "media/base/flinging_controller.h"
#include "media/base/media_controller.h"

namespace media_router {

// Allows native code to call into a Java FlingingController.
class FlingingControllerBridge : public media::FlingingController,
public media::MediaController {
public:
explicit FlingingControllerBridge(
base::android::ScopedJavaGlobalRef<jobject> controller);
~FlingingControllerBridge() override;

// FlingingController implementation.
media::MediaController* GetMediaController() override;
void AddMediaStatusObserver(media::MediaStatusObserver* observer) override;
void RemoveMediaStatusObserver(media::MediaStatusObserver* observer) override;
base::TimeDelta GetApproximateCurrentTime() override;

// MediaController implementation.
void Play() override;
void Pause() override;
void SetMute(bool mute) override;
void SetVolume(float volume) override;
void Seek(base::TimeDelta time) override;

private:
// Java MediaControllerBridge instance.
base::android::ScopedJavaGlobalRef<jobject> j_flinging_controller_bridge_;

DISALLOW_COPY_AND_ASSIGN(FlingingControllerBridge);
};

} // namespace media_router

#endif // CHROME_BROWSER_MEDIA_ANDROID_REMOTE_FLINGING_CONTROLLER_BRIDGE_H_
53 changes: 0 additions & 53 deletions chrome/browser/media/android/remote/media_controller_bridge.cc

This file was deleted.

37 changes: 0 additions & 37 deletions chrome/browser/media/android/remote/media_controller_bridge.h

This file was deleted.

6 changes: 3 additions & 3 deletions chrome/browser/media/android/router/media_router_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -316,9 +316,9 @@ void MediaRouterAndroid::RemoveRoute(const MediaRoute::Id& route_id) {
observer.OnRoutesUpdated(active_routes_, std::vector<MediaRoute::Id>());
}

std::unique_ptr<media::MediaController> MediaRouterAndroid::GetMediaController(
const MediaRoute::Id& route_id) {
return bridge_->GetMediaController(route_id);
std::unique_ptr<media::FlingingController>
MediaRouterAndroid::GetFlingingController(const MediaRoute::Id& route_id) {
return bridge_->GetFlingingController(route_id);
}

} // namespace media_router
2 changes: 1 addition & 1 deletion chrome/browser/media/android/router/media_router_android.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class MediaRouterAndroid : public MediaRouterBase {
const std::string& search_input,
const std::string& domain,
MediaSinkSearchResponseCallback sink_callback) override;
std::unique_ptr<media::MediaController> GetMediaController(
std::unique_ptr<media::FlingingController> GetFlingingController(
const MediaRoute::Id& route_id) override;

// The methods called by the Java bridge.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

#include "base/android/jni_android.h"
#include "base/android/jni_string.h"
#include "chrome/browser/media/android/remote/media_controller_bridge.h"
#include "chrome/browser/media/android/remote/flinging_controller_bridge.h"
#include "chrome/browser/media/android/router/media_router_android.h"
#include "jni/ChromeMediaRouter_jni.h"
#include "media/base/media_controller.h"
Expand Down Expand Up @@ -120,21 +120,22 @@ void MediaRouterAndroidBridge::StopObservingMediaSinks(
jsource_id);
}

std::unique_ptr<media::MediaController>
MediaRouterAndroidBridge::GetMediaController(const MediaRoute::Id& route_id) {
std::unique_ptr<media::FlingingController>
MediaRouterAndroidBridge::GetFlingingController(
const MediaRoute::Id& route_id) {
JNIEnv* env = base::android::AttachCurrentThread();
ScopedJavaLocalRef<jstring> jroute_id =
base::android::ConvertUTF8ToJavaString(env, route_id);

ScopedJavaGlobalRef<jobject> media_controller;
ScopedJavaGlobalRef<jobject> flinging_controller;

media_controller.Reset(Java_ChromeMediaRouter_getMediaControllerBridge(
flinging_controller.Reset(Java_ChromeMediaRouter_getFlingingControllerBridge(
env, java_media_router_, jroute_id));

if (media_controller.is_null())
if (flinging_controller.is_null())
return nullptr;

return std::make_unique<MediaControllerBridge>(media_controller);
return std::make_unique<FlingingControllerBridge>(flinging_controller);
}

void MediaRouterAndroidBridge::OnSinksReceived(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "chrome/common/media_router/media_route.h"
#include "chrome/common/media_router/media_sink.h"
#include "chrome/common/media_router/media_source.h"
#include "media/base/media_controller.h"
#include "media/base/flinging_controller.h"
#include "url/origin.h"

namespace media_router {
Expand Down Expand Up @@ -44,7 +44,7 @@ class MediaRouterAndroidBridge {
virtual void DetachRoute(const MediaRoute::Id& route_id);
virtual bool StartObservingMediaSinks(const MediaSource::Id& source_id);
virtual void StopObservingMediaSinks(const MediaSource::Id& source_id);
virtual std::unique_ptr<media::MediaController> GetMediaController(
virtual std::unique_ptr<media::FlingingController> GetFlingingController(
const MediaRoute::Id& route_id);

// Methods called by the Java counterpart.
Expand Down
7 changes: 4 additions & 3 deletions chrome/browser/media/router/media_router.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#include "components/keyed_service/core/keyed_service.h"
#include "components/sessions/core/session_id.h"
#include "content/public/browser/presentation_service_delegate.h"
#include "media/base/media_controller.h"
#include "media/base/flinging_controller.h"

namespace content {
class WebContents;
Expand Down Expand Up @@ -190,9 +190,10 @@ class MediaRouter : public KeyedService {
// there is a change to the media routes, subclass MediaRoutesObserver.
virtual std::vector<MediaRoute> GetCurrentRoutes() const = 0;

// Returns a controller that directly sends commands to media within a route.
// Returns a controller that sends commands to media within a route, and
// propagates MediaStatus changes.
// Returns a nullptr if no controller can be be found from |route_id|.
virtual std::unique_ptr<media::MediaController> GetMediaController(
virtual std::unique_ptr<media::FlingingController> GetFlingingController(
const MediaRoute::Id& route_id) = 0;

#if !defined(OS_ANDROID)
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/media/router/media_router_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ std::vector<MediaRoute> MediaRouterBase::GetCurrentRoutes() const {
return internal_routes_observer_->current_routes;
}

std::unique_ptr<media::MediaController> MediaRouterBase::GetMediaController(
const MediaRoute::Id& route_id) {
std::unique_ptr<media::FlingingController>
MediaRouterBase::GetFlingingController(const MediaRoute::Id& route_id) {
return nullptr;
}

Expand Down
Loading

0 comments on commit fc4431b

Please sign in to comment.