content/common/android/media_metadata_proxy.h - Issue 1458703003: Media Session API: use MediaMetadata in the browser process.

Side by Side Diff: content/common/android/media_metadata_proxy.h

Issue 1458703003: Media Session API: use MediaMetadata in the browser process. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@media_session_browser_side

Patch Set: remove dependencies from other CLs Created 4 years, 9 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java ('K') | « content/common/android/common_jni_registrar.cc ('k') | content/common/android/media_metadata_proxy.cc » ('j') | content/public/android/java/src/org/chromium/content_public/common/MediaMetadata.java » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 // Copyright 2016 The Chromium Authors. All rights reserved.

	2 // Use of this source code is governed by a BSD-style license that can be

	3 // found in the LICENSE file.

	4

	5 #ifndef CONTENT_COMMON_ANDROID_MEDIA_METADATA_PROXY_H_

	6 #define CONTENT_COMMON_ANDROID_MEDIA_METADATA_PROXY_H_

	7

	8 #include <jni.h>

	9

	10 #include "base/android/scoped_java_ref.h"

	11

	12 namespace content {

	13

	14 struct MediaMetadata;

	15

	16 class MediaMetadataProxy {
	Ted C 2016/03/01 17:18:11 The common naming would be MediaMetadataAndroid. The common naming would be MediaMetadataAndroid. I also wouldn't object making MediaMetadata a class that has this method on it defined only for Android. Having to create this second class is somewhat sad to me. mlamouri (slow - plz ping) 2016/03/16 15:47:15 Isn't that mostly used for android implementations Show quoted text On 2016/03/01 at 17:18:11, Ted C wrote: > The common naming would be MediaMetadataAndroid. Isn't that mostly used for android implementations of a class? Show quoted text > I also wouldn't object making MediaMetadata a class that has this method on it defined only for Android. Having to create this second class is somewhat sad to me. The issue is that MediaMetadata is a content/public/ class so it would be exposed to content embedders (ie. chrome/) while this proxy is only exposed to content/. We could work around that by having MediaMetadata and MediaMetadataImpl which is content/ only and have the CreateJavaObject() method but that sounds like more boilerplate :) Ted C 2016/03/16 16:55:58 Nope, web_contents_android, profile_android are pr Show quoted text On 2016/03/16 15:47:15, Mounir Lamouri wrote: > On 2016/03/01 at 17:18:11, Ted C wrote: > > The common naming would be MediaMetadataAndroid. > > Isn't that mostly used for android implementations of a class? Nope, web_contents_android, profile_android are prime examples where the android class is just there to expose functionality to java. _android usage: https://code.google.com/p/chromium/codesearch#search/&q=file:._android%5C.h&... _proxy usage: https://code.google.com/p/chromium/codesearch#search/&q=file:._proxy.h%20fil... While the _android.h is certainly muddled and can definitely mean the android version of some class. I still think it makes sense here more than proxy. This isn't a proxy, it is actually just a class that creates java equivalent versions. MediaMetadataFactoryAndroid might be more accurate but more verbose as well, so I still stand by that MediaMetadataAndroid is the best option for this. Show quoted text > > > I also wouldn't object making MediaMetadata a class that has this method on it > defined only for Android. Having to create this second class is somewhat sad to > me. > > The issue is that MediaMetadata is a content/public/ class so it would be > exposed to content embedders (ie. chrome/) while this proxy is only exposed to > content/. We could work around that by having MediaMetadata and > MediaMetadataImpl which is content/ only and have the CreateJavaObject() method > but that sounds like more boilerplate :) mlamouri (slow - plz ping) 2016/03/16 20:31:54 Ok. Done :) Show quoted text On 2016/03/16 at 16:55:58, Ted C wrote: > On 2016/03/16 15:47:15, Mounir Lamouri wrote: > > On 2016/03/01 at 17:18:11, Ted C wrote: > > > The common naming would be MediaMetadataAndroid. > > > > Isn't that mostly used for android implementations of a class? > > Nope, web_contents_android, profile_android are prime examples where the android class is just there to expose functionality to java. > > _android usage: > https://code.google.com/p/chromium/codesearch#search/&q=file:._android%5C.h&... > > _proxy usage: > https://code.google.com/p/chromium/codesearch#search/&q=file:._proxy.h%20fil... > > While the _android.h is certainly muddled and can definitely mean the android version of some class. I still think it makes sense here more than proxy. > > This isn't a proxy, it is actually just a class that creates java equivalent versions. MediaMetadataFactoryAndroid might be more accurate but more verbose as well, so I still stand by that MediaMetadataAndroid is the best option for this. Ok. Done :)
	17 public:

	18 static base::android::ScopedJavaLocalRef<jobject> GetJavaObject(
	Ted C 2016/03/01 17:18:11 I would use CreateJavaObject, Get implies reuse to I would use CreateJavaObject, Get implies reuse to me mlamouri (slow - plz ping) 2016/03/16 15:47:15 Done. Show quoted text On 2016/03/01 at 17:18:11, Ted C wrote: > I would use CreateJavaObject, Get implies reuse to me Done.
	19 JNIEnv* env, const MediaMetadata& metadata);

	20

	21 static bool Register(JNIEnv* env);

	22

	23 private:

	24 DISALLOW_IMPLICIT_CONSTRUCTORS(MediaMetadataProxy);

	25 };

	26

	27 } // namespace content

	28

	29 #endif // CONTENT_COMMON_ANDROID_MEDIA_METADATA_PROXY_H_

OLD	NEW