Android: Generate JNI code for MediaConstraints

Also improves ownership model by using std::unique_ptr in a couple of
places instead of raw pointers.

Bug: webrtc:8278
Change-Id: I0429ec3c416b5baa1ffa21dad71e0d64b004c446
Reviewed-on: https://webrtc-review.googlesource.com/25020
Commit-Queue: Magnus Jedvert <magjed@webrtc.org>
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20863}
diff --git a/api/mediaconstraintsinterface.h b/api/mediaconstraintsinterface.h
index ebeec00..73e4619 100644
--- a/api/mediaconstraintsinterface.h
+++ b/api/mediaconstraintsinterface.h
@@ -50,9 +50,6 @@
     bool FindFirst(const std::string& key, std::string* value) const;
   };
 
-  virtual const Constraints& GetMandatory() const = 0;
-  virtual const Constraints& GetOptional() const = 0;
-
   // Constraint keys used by a local video source.
   // Specified by draft-alvestrand-constraints-resolution-00b
   static const char kMinAspectRatio[];  // minAspectRatio
@@ -125,9 +122,10 @@
   // stripped by Chrome before passed down to Libjingle.
   static const char kInternalConstraintPrefix[];
 
- protected:
-  // Dtor protected as objects shouldn't be deleted via this interface
-  virtual ~MediaConstraintsInterface() {}
+  virtual ~MediaConstraintsInterface() = default;
+
+  virtual const Constraints& GetMandatory() const = 0;
+  virtual const Constraints& GetOptional() const = 0;
 };
 
 bool FindConstraint(const MediaConstraintsInterface* constraints,
diff --git a/sdk/android/BUILD.gn b/sdk/android/BUILD.gn
index 50ae3e7..ecf8a1e 100644
--- a/sdk/android/BUILD.gn
+++ b/sdk/android/BUILD.gn
@@ -278,6 +278,7 @@
 
 generate_jni("generated_peerconnection_jni") {
   sources = [
+    "api/org/webrtc/MediaConstraints.java",
     "api/org/webrtc/NetworkMonitor.java",
     "api/org/webrtc/NetworkMonitorAutoDetect.java",
   ]
diff --git a/sdk/android/api/org/webrtc/MediaConstraints.java b/sdk/android/api/org/webrtc/MediaConstraints.java
index 259d546..fc0c9c2 100644
--- a/sdk/android/api/org/webrtc/MediaConstraints.java
+++ b/sdk/android/api/org/webrtc/MediaConstraints.java
@@ -28,10 +28,12 @@
       this.value = value;
     }
 
+    @CalledByNative("KeyValuePair")
     public String getKey() {
       return key;
     }
 
+    @CalledByNative("KeyValuePair")
     public String getValue() {
       return value;
     }
@@ -83,4 +85,14 @@
     return "mandatory: " + stringifyKeyValuePairList(mandatory) + ", optional: "
         + stringifyKeyValuePairList(optional);
   }
+
+  @CalledByNative
+  List<KeyValuePair> getMandatory() {
+    return mandatory;
+  }
+
+  @CalledByNative
+  List<KeyValuePair> getOptional() {
+    return optional;
+  }
 }
diff --git a/sdk/android/src/jni/pc/mediaconstraints_jni.cc b/sdk/android/src/jni/pc/mediaconstraints_jni.cc
index 5f0b1d5..b7d05a2 100644
--- a/sdk/android/src/jni/pc/mediaconstraints_jni.cc
+++ b/sdk/android/src/jni/pc/mediaconstraints_jni.cc
@@ -10,39 +10,57 @@
 
 #include "sdk/android/src/jni/pc/mediaconstraints_jni.h"
 
+#include "rtc_base/ptr_util.h"
+#include "sdk/android/generated_peerconnection_jni/jni/MediaConstraints_jni.h"
+#include "sdk/android/src/jni/jni_helpers.h"
+
 namespace webrtc {
 namespace jni {
 
-MediaConstraintsJni::MediaConstraintsJni(JNIEnv* jni, jobject j_constraints) {
-  PopulateConstraintsFromJavaPairList(jni, j_constraints, "mandatory",
-                                      &mandatory_);
-  PopulateConstraintsFromJavaPairList(jni, j_constraints, "optional",
-                                      &optional_);
+namespace {
+
+// Helper for translating a List<Pair<String, String>> to a Constraints.
+MediaConstraintsInterface::Constraints PopulateConstraintsFromJavaPairList(
+    JNIEnv* env,
+    jobject j_list) {
+  MediaConstraintsInterface::Constraints constraints;
+  for (jobject entry : Iterable(env, j_list)) {
+    jstring j_key = Java_KeyValuePair_getKey(env, entry);
+    jstring j_value = Java_KeyValuePair_getValue(env, entry);
+    constraints.emplace_back(JavaToStdString(env, j_key),
+                             JavaToStdString(env, j_value));
+  }
+  return constraints;
 }
 
-// static
-void MediaConstraintsJni::PopulateConstraintsFromJavaPairList(
-    JNIEnv* jni,
-    jobject j_constraints,
-    const char* field_name,
-    Constraints* field) {
-  jfieldID j_id = GetFieldID(jni, GetObjectClass(jni, j_constraints),
-                             field_name, "Ljava/util/List;");
-  jobject j_list = GetObjectField(jni, j_constraints, j_id);
-  for (jobject entry : Iterable(jni, j_list)) {
-    jmethodID get_key = GetMethodID(jni, GetObjectClass(jni, entry), "getKey",
-                                    "()Ljava/lang/String;");
-    jstring j_key =
-        reinterpret_cast<jstring>(jni->CallObjectMethod(entry, get_key));
-    CHECK_EXCEPTION(jni) << "error during CallObjectMethod";
-    jmethodID get_value = GetMethodID(jni, GetObjectClass(jni, entry),
-                                      "getValue", "()Ljava/lang/String;");
-    jstring j_value =
-        reinterpret_cast<jstring>(jni->CallObjectMethod(entry, get_value));
-    CHECK_EXCEPTION(jni) << "error during CallObjectMethod";
-    field->push_back(
-        Constraint(JavaToStdString(jni, j_key), JavaToStdString(jni, j_value)));
-  }
+// Wrapper for a Java MediaConstraints object.  Copies all needed data so when
+// the constructor returns the Java object is no longer needed.
+class MediaConstraintsJni : public MediaConstraintsInterface {
+ public:
+  MediaConstraintsJni(JNIEnv* env, jobject j_constraints)
+      : mandatory_(PopulateConstraintsFromJavaPairList(
+            env,
+            Java_MediaConstraints_getMandatory(env, j_constraints))),
+        optional_(PopulateConstraintsFromJavaPairList(
+            env,
+            Java_MediaConstraints_getOptional(env, j_constraints))) {}
+  virtual ~MediaConstraintsJni() = default;
+
+  // MediaConstraintsInterface.
+  const Constraints& GetMandatory() const override { return mandatory_; }
+  const Constraints& GetOptional() const override { return optional_; }
+
+ private:
+  const Constraints mandatory_;
+  const Constraints optional_;
+};
+
+}  // namespace
+
+std::unique_ptr<MediaConstraintsInterface> JavaToNativeMediaConstraints(
+    JNIEnv* env,
+    jobject j_constraints) {
+  return rtc::MakeUnique<MediaConstraintsJni>(env, j_constraints);
 }
 
 }  // namespace jni
diff --git a/sdk/android/src/jni/pc/mediaconstraints_jni.h b/sdk/android/src/jni/pc/mediaconstraints_jni.h
index ec21f43..ce7d90d 100644
--- a/sdk/android/src/jni/pc/mediaconstraints_jni.h
+++ b/sdk/android/src/jni/pc/mediaconstraints_jni.h
@@ -11,33 +11,16 @@
 #ifndef SDK_ANDROID_SRC_JNI_PC_MEDIACONSTRAINTS_JNI_H_
 #define SDK_ANDROID_SRC_JNI_PC_MEDIACONSTRAINTS_JNI_H_
 
+#include <jni.h>
+
 #include "api/mediaconstraintsinterface.h"
-#include "sdk/android/src/jni/jni_helpers.h"
 
 namespace webrtc {
 namespace jni {
 
-// Wrapper for a Java MediaConstraints object.  Copies all needed data so when
-// the constructor returns the Java object is no longer needed.
-class MediaConstraintsJni : public MediaConstraintsInterface {
- public:
-  MediaConstraintsJni(JNIEnv* jni, jobject j_constraints);
-  virtual ~MediaConstraintsJni() {}
-
-  // MediaConstraintsInterface.
-  const Constraints& GetMandatory() const override { return mandatory_; }
-  const Constraints& GetOptional() const override { return optional_; }
-
- private:
-  // Helper for translating a List<Pair<String, String>> to a Constraints.
-  static void PopulateConstraintsFromJavaPairList(JNIEnv* jni,
-                                                  jobject j_constraints,
-                                                  const char* field_name,
-                                                  Constraints* field);
-
-  Constraints mandatory_;
-  Constraints optional_;
-};
+std::unique_ptr<MediaConstraintsInterface> JavaToNativeMediaConstraints(
+    JNIEnv* env,
+    jobject j_constraints);
 
 }  // namespace jni
 }  // namespace webrtc
diff --git a/sdk/android/src/jni/pc/peerconnection_jni.cc b/sdk/android/src/jni/pc/peerconnection_jni.cc
index aea277d..8183700 100644
--- a/sdk/android/src/jni/pc/peerconnection_jni.cc
+++ b/sdk/android/src/jni/pc/peerconnection_jni.cc
@@ -120,12 +120,12 @@
                          jobject j_pc,
                          jobject j_observer,
                          jobject j_constraints) {
-  MediaConstraintsJni* constraints =
-      new MediaConstraintsJni(jni, j_constraints);
+  std::unique_ptr<MediaConstraintsInterface> constraints =
+      JavaToNativeMediaConstraints(jni, j_constraints);
   rtc::scoped_refptr<CreateSdpObserverJni> observer(
       new rtc::RefCountedObject<CreateSdpObserverJni>(jni, j_observer,
-                                                      constraints));
-  ExtractNativePC(jni, j_pc)->CreateOffer(observer, constraints);
+                                                      std::move(constraints)));
+  ExtractNativePC(jni, j_pc)->CreateOffer(observer, observer->constraints());
 }
 
 JNI_FUNCTION_DECLARATION(void,
@@ -134,12 +134,12 @@
                          jobject j_pc,
                          jobject j_observer,
                          jobject j_constraints) {
-  MediaConstraintsJni* constraints =
-      new MediaConstraintsJni(jni, j_constraints);
+  std::unique_ptr<MediaConstraintsInterface> constraints =
+      JavaToNativeMediaConstraints(jni, j_constraints);
   rtc::scoped_refptr<CreateSdpObserverJni> observer(
       new rtc::RefCountedObject<CreateSdpObserverJni>(jni, j_observer,
-                                                      constraints));
-  ExtractNativePC(jni, j_pc)->CreateAnswer(observer, constraints);
+                                                      std::move(constraints)));
+  ExtractNativePC(jni, j_pc)->CreateAnswer(observer, observer->constraints());
 }
 
 JNI_FUNCTION_DECLARATION(void,
diff --git a/sdk/android/src/jni/pc/peerconnectionfactory_jni.cc b/sdk/android/src/jni/pc/peerconnectionfactory_jni.cc
index e34d1fd..c42ed54 100644
--- a/sdk/android/src/jni/pc/peerconnectionfactory_jni.cc
+++ b/sdk/android/src/jni/pc/peerconnectionfactory_jni.cc
@@ -321,8 +321,8 @@
                          jclass,
                          jlong native_factory,
                          jobject j_constraints) {
-  std::unique_ptr<MediaConstraintsJni> constraints(
-      new MediaConstraintsJni(jni, j_constraints));
+  std::unique_ptr<MediaConstraintsInterface> constraints =
+      JavaToNativeMediaConstraints(jni, j_constraints);
   rtc::scoped_refptr<PeerConnectionFactoryInterface> factory(
       factoryFromJava(native_factory));
   cricket::AudioOptions options;
@@ -429,7 +429,7 @@
 
   PeerConnectionObserverJni* observer =
       reinterpret_cast<PeerConnectionObserverJni*>(observer_p);
-  observer->SetConstraints(new MediaConstraintsJni(jni, j_constraints));
+  observer->SetConstraints(JavaToNativeMediaConstraints(jni, j_constraints));
   CopyConstraintsIntoRtcConfiguration(observer->constraints(), &rtc_config);
   rtc::scoped_refptr<PeerConnectionInterface> pc(
       f->CreatePeerConnection(rtc_config, nullptr, nullptr, observer));
diff --git a/sdk/android/src/jni/pc/peerconnectionobserver_jni.cc b/sdk/android/src/jni/pc/peerconnectionobserver_jni.cc
index 6105255..a0eac91 100644
--- a/sdk/android/src/jni/pc/peerconnectionobserver_jni.cc
+++ b/sdk/android/src/jni/pc/peerconnectionobserver_jni.cc
@@ -337,9 +337,9 @@
 }
 
 void PeerConnectionObserverJni::SetConstraints(
-    MediaConstraintsJni* constraints) {
+    std::unique_ptr<MediaConstraintsInterface> constraints) {
   RTC_CHECK(!constraints_.get()) << "constraints already set!";
-  constraints_.reset(constraints);
+  constraints_ = std::move(constraints);
 }
 
 void PeerConnectionObserverJni::DisposeRemoteStream(
diff --git a/sdk/android/src/jni/pc/peerconnectionobserver_jni.h b/sdk/android/src/jni/pc/peerconnectionobserver_jni.h
index a48828b..d6fd0f3 100644
--- a/sdk/android/src/jni/pc/peerconnectionobserver_jni.h
+++ b/sdk/android/src/jni/pc/peerconnectionobserver_jni.h
@@ -52,8 +52,8 @@
                   const std::vector<rtc::scoped_refptr<MediaStreamInterface>>&
                       streams) override;
 
-  void SetConstraints(MediaConstraintsJni* constraints);
-  const MediaConstraintsJni* constraints() { return constraints_.get(); }
+  void SetConstraints(std::unique_ptr<MediaConstraintsInterface> constraints);
+  const MediaConstraintsInterface* constraints() { return constraints_.get(); }
 
  private:
   typedef std::map<MediaStreamInterface*, jobject> NativeToJavaStreamsMap;
@@ -127,7 +127,7 @@
   // manually deleted upon removal. Use DisposeRemoteStream().
   NativeToJavaStreamsMap remote_streams_;
   NativeToJavaRtpReceiverMap rtp_receivers_;
-  std::unique_ptr<MediaConstraintsJni> constraints_;
+  std::unique_ptr<MediaConstraintsInterface> constraints_;
   std::vector<std::unique_ptr<webrtc::MediaStreamObserver>> stream_observers_;
 };
 
diff --git a/sdk/android/src/jni/pc/sdpobserver_jni.h b/sdk/android/src/jni/pc/sdpobserver_jni.h
index b330b69..9b8db63 100644
--- a/sdk/android/src/jni/pc/sdpobserver_jni.h
+++ b/sdk/android/src/jni/pc/sdpobserver_jni.h
@@ -16,7 +16,6 @@
 
 #include "api/peerconnectioninterface.h"
 #include "sdk/android/src/jni/jni_helpers.h"
-#include "sdk/android/src/jni/pc/mediaconstraints_jni.h"
 
 namespace webrtc {
 namespace jni {
@@ -29,8 +28,8 @@
  public:
   SdpObserverJni(JNIEnv* jni,
                  jobject j_observer,
-                 MediaConstraintsJni* constraints)
-      : constraints_(constraints),
+                 std::unique_ptr<MediaConstraintsInterface> constraints)
+      : constraints_(std::move(constraints)),
         j_observer_global_(jni, j_observer),
         j_observer_class_(jni, GetObjectClass(jni, j_observer)) {}
 
@@ -57,6 +56,8 @@
     delete desc;
   }
 
+  MediaConstraintsInterface* constraints() { return constraints_.get(); }
+
  protected:
   // Common implementation for failure of Set & Create types, distinguished by
   // |op| being "Set" or "Create".
@@ -71,7 +72,7 @@
   JNIEnv* jni() { return AttachCurrentThreadIfNeeded(); }
 
  private:
-  std::unique_ptr<MediaConstraintsJni> constraints_;
+  std::unique_ptr<MediaConstraintsInterface> constraints_;
   const ScopedGlobalRef<jobject> j_observer_global_;
   const ScopedGlobalRef<jclass> j_observer_class_;
 };
@@ -81,8 +82,8 @@
  public:
   CreateSdpObserverJni(JNIEnv* jni,
                        jobject j_observer,
-                       MediaConstraintsJni* constraints)
-      : SdpObserverJni(jni, j_observer, constraints) {}
+                       std::unique_ptr<MediaConstraintsInterface> constraints)
+      : SdpObserverJni(jni, j_observer, std::move(constraints)) {}
 
   void OnFailure(const std::string& error) override {
     ScopedLocalRefFrame local_ref_frame(jni());
@@ -94,8 +95,8 @@
  public:
   SetSdpObserverJni(JNIEnv* jni,
                     jobject j_observer,
-                    MediaConstraintsJni* constraints)
-      : SdpObserverJni(jni, j_observer, constraints) {}
+                    std::unique_ptr<MediaConstraintsInterface> constraints)
+      : SdpObserverJni(jni, j_observer, std::move(constraints)) {}
 
   void OnFailure(const std::string& error) override {
     ScopedLocalRefFrame local_ref_frame(jni());