Modify JavaToStdString to allow ISO-8859-1 encoded strings.

Modify JavaToStdString to allow ISO-8859-1 encoded strings.

Current implementation of JavaToStdString applies additional encoding that modifies ISO-8859-1 encoded strings (e.g. byte array). This CL is to fix this.

A planned use of this is to pass a protobuf serialized string as a MediaConstraint to WebRTC to configure audio network adaptor.

BUG=webrtc:6815
Committed: https://crrev.com/50254636d4c9bedb333e65c906b54e1f36305571
Cr-Commit-Position: refs/heads/master@{#15509}

[minyue-webrtc, 2016-12-02 14:15:00]

Patchset #1 (id:1) has been deleted

[minyue-webrtc, 2016-12-02 14:24:10]

Description was changed from

==========
Passing MediaConstraints as byte array.

BUG=
==========

to

==========
Passing MediaConstraints as byte array.

The Java interface of MediaContraints uses a JAVA string, which has difficulty
to hold a UTF-8 string. This CL tries to switch it to byte array.

A planned use of this is to pass a protobuf serialized string from JAVA app to
WebRTC to configure audio network adaptor.

BUG=
==========

[minyue-webrtc, 2016-12-02 14:24:11]

minyue@webrtc.org changed reviewers:
+ magjed@webrtc.org

[minyue-webrtc, 2016-12-02 14:26:40]

Description was changed from

==========
Passing MediaConstraints as byte array.

The Java interface of MediaContraints uses a JAVA string, which has difficulty
to hold a UTF-8 string. This CL tries to switch it to byte array.

A planned use of this is to pass a protobuf serialized string from JAVA app to
WebRTC to configure audio network adaptor.

BUG=
==========

to

==========
Passing MediaConstraints as byte array.

The Java interface of MediaContraints uses a JAVA string, which has difficulty
to hold a UTF-8 string. This CL tries to switch it to byte array.

A planned use of this is to pass a protobuf serialized string from JAVA app to
WebRTC to configure audio network adaptor.

BUG=webrtc:6815
==========

[minyue-webrtc, 2016-12-02 14:27:28]

Hi Magnus,

Would you help me in reviewing this CL? Thanks!

[magjed_webrtc, 2016-12-03 12:03:42]

I don't understand the problem, why do you want to hold it in Java as a UTF-8
string? Why doesn't the standard UTF-16 work?

You start with a Java String in MediaConstraints.java and eventually end up with
a std::string, so that sounds exactly what JavaToStdString should do already. So
I'm wondering when will the result of JavaToStdString(java_string) be different
from the new:
JavaByteArrayToStdString(java_string.getBytes(StandardCharsets.UTF_8))?

[minyue-webrtc, 2016-12-05 08:36:26]

On 2016/12/03 12:03:42, magjed_webrtc wrote:
> I don't understand the problem, why do you want to hold it in Java as a UTF-8
> string? Why doesn't the standard UTF-16 work?
> 
> You start with a Java String in MediaConstraints.java and eventually end up
with
> a std::string, so that sounds exactly what JavaToStdString should do already.
So
> I'm wondering when will the result of JavaToStdString(java_string) be
different
> from the new:
> JavaByteArrayToStdString(java_string.getBytes(StandardCharsets.UTF_8))?

Hi Magnus,

The actual problem there is that the JNI method GetStringUTFChars does not give
UTF-8 string. It uses a modified UTF-8 encoding. See discussions like
http://stackoverflow.com/questions/32205446/getting-true-utf-8-characters-in-....
So passing a JAVA String with UTF-8 content through JNI is a bit painful.

I think this can be a reason that proto buffer JAVA API does not use JAVA
String. It uses byte array instead.

[magjed_webrtc, 2016-12-05 10:26:44]

On 2016/12/05 08:36:26, minyue-webrtc wrote:
> On 2016/12/03 12:03:42, magjed_webrtc wrote:
> > I don't understand the problem, why do you want to hold it in Java as a
UTF-8
> > string? Why doesn't the standard UTF-16 work?
> > 
> > You start with a Java String in MediaConstraints.java and eventually end up
> with
> > a std::string, so that sounds exactly what JavaToStdString should do
already.
> So
> > I'm wondering when will the result of JavaToStdString(java_string) be
> different
> > from the new:
> > JavaByteArrayToStdString(java_string.getBytes(StandardCharsets.UTF_8))?
> 
> Hi Magnus,
> 
> The actual problem there is that the JNI method GetStringUTFChars does not
give
> UTF-8 string. It uses a modified UTF-8 encoding. See discussions like
>
http://stackoverflow.com/questions/32205446/getting-true-utf-8-characters-in-....
> So passing a JAVA String with UTF-8 content through JNI is a bit painful.
> 
> I think this can be a reason that proto buffer JAVA API does not use JAVA
> String. It uses byte array instead.

I see. I think it would be better to fix JavaToStdString to return a plain (non
Java-modified) UTF8 string instead, and not add a separate function for byte
arrays. Can't you just replace the implementation of JavaToStdString with the
new JavaByteArrayToStdString implementation where you get the byte array with
String.getBytes(StandardCharsets.UTF_8) in JNI?

[minyue-webrtc, 2016-12-07 15:39:48]

On 2016/12/05 10:26:44, magjed_webrtc wrote:
> On 2016/12/05 08:36:26, minyue-webrtc wrote:
> > On 2016/12/03 12:03:42, magjed_webrtc wrote:
> > > I don't understand the problem, why do you want to hold it in Java as a
> UTF-8
> > > string? Why doesn't the standard UTF-16 work?
> > > 
> > > You start with a Java String in MediaConstraints.java and eventually end
up
> > with
> > > a std::string, so that sounds exactly what JavaToStdString should do
> already.
> > So
> > > I'm wondering when will the result of JavaToStdString(java_string) be
> > different
> > > from the new:
> > > JavaByteArrayToStdString(java_string.getBytes(StandardCharsets.UTF_8))?
> > 
> > Hi Magnus,
> > 
> > The actual problem there is that the JNI method GetStringUTFChars does not
> give
> > UTF-8 string. It uses a modified UTF-8 encoding. See discussions like
> >
>
http://stackoverflow.com/questions/32205446/getting-true-utf-8-characters-in-....
> > So passing a JAVA String with UTF-8 content through JNI is a bit painful.
> > 
> > I think this can be a reason that proto buffer JAVA API does not use JAVA
> > String. It uses byte array instead.
> 
> I see. I think it would be better to fix JavaToStdString to return a plain
(non
> Java-modified) UTF8 string instead, and not add a separate function for byte
> arrays. Can't you just replace the implementation of JavaToStdString with the
> new JavaByteArrayToStdString implementation where you get the byte array with
> String.getBytes(StandardCharsets.UTF_8) in JNI?

Hi Magnus,

I tried to change only the helper method, and made it to work. See PS2. But it
reads differently than we thought.

What I eventually need is convert a Java byte[] to C++ std::string.

Now we have to go through Java bytes[] -> Java string -> C++ std::string, where
the beginning and end expect memory exact. Any re-encoding into and from the
Java string in middle can be wrong except for ISO-8859-1.

I prefer my earlier solution, since it is easier to understand. WDYT?

[minyue-webrtc, 2016-12-07 18:53:52]

On 2016/12/07 15:39:48, minyue-webrtc wrote:
> On 2016/12/05 10:26:44, magjed_webrtc wrote:
> > On 2016/12/05 08:36:26, minyue-webrtc wrote:
> > > On 2016/12/03 12:03:42, magjed_webrtc wrote:
> > > > I don't understand the problem, why do you want to hold it in Java as a
> > UTF-8
> > > > string? Why doesn't the standard UTF-16 work?
> > > > 
> > > > You start with a Java String in MediaConstraints.java and eventually end
> up
> > > with
> > > > a std::string, so that sounds exactly what JavaToStdString should do
> > already.
> > > So
> > > > I'm wondering when will the result of JavaToStdString(java_string) be
> > > different
> > > > from the new:
> > > > JavaByteArrayToStdString(java_string.getBytes(StandardCharsets.UTF_8))?
> > > 
> > > Hi Magnus,
> > > 
> > > The actual problem there is that the JNI method GetStringUTFChars does not
> > give
> > > UTF-8 string. It uses a modified UTF-8 encoding. See discussions like
> > >
> >
>
http://stackoverflow.com/questions/32205446/getting-true-utf-8-characters-in-....
> > > So passing a JAVA String with UTF-8 content through JNI is a bit painful.
> > > 
> > > I think this can be a reason that proto buffer JAVA API does not use JAVA
> > > String. It uses byte array instead.
> > 
> > I see. I think it would be better to fix JavaToStdString to return a plain
> (non
> > Java-modified) UTF8 string instead, and not add a separate function for byte
> > arrays. Can't you just replace the implementation of JavaToStdString with
the
> > new JavaByteArrayToStdString implementation where you get the byte array
with
> > String.getBytes(StandardCharsets.UTF_8) in JNI?
> 
> Hi Magnus,
> 
> I tried to change only the helper method, and made it to work. See PS2. But it
> reads differently than we thought.
> 
> What I eventually need is convert a Java byte[] to C++ std::string.
> 
> Now we have to go through Java bytes[] -> Java string -> C++ std::string,
where
> the beginning and end expect memory exact. Any re-encoding into and from the
> Java string in middle can be wrong except for ISO-8859-1.
> 
> I prefer my earlier solution, since it is easier to understand. WDYT?

To add, I should be wrong from the beginning by saying that we would like to
pass a UTF-8 string with MediaContraints. Actually, we would like it to pass a
byte array.

[magjed_webrtc, 2016-12-08 09:57:32]

On 2016/12/07 18:53:52, minyue-webrtc wrote:
> On 2016/12/07 15:39:48, minyue-webrtc wrote:
> > On 2016/12/05 10:26:44, magjed_webrtc wrote:
> > > On 2016/12/05 08:36:26, minyue-webrtc wrote:
> > > > On 2016/12/03 12:03:42, magjed_webrtc wrote:
> > > > > I don't understand the problem, why do you want to hold it in Java as
a
> > > UTF-8
> > > > > string? Why doesn't the standard UTF-16 work?
> > > > > 
> > > > > You start with a Java String in MediaConstraints.java and eventually
end
> > up
> > > > with
> > > > > a std::string, so that sounds exactly what JavaToStdString should do
> > > already.
> > > > So
> > > > > I'm wondering when will the result of JavaToStdString(java_string) be
> > > > different
> > > > > from the new:
> > > > >
JavaByteArrayToStdString(java_string.getBytes(StandardCharsets.UTF_8))?
> > > > 
> > > > Hi Magnus,
> > > > 
> > > > The actual problem there is that the JNI method GetStringUTFChars does
not
> > > give
> > > > UTF-8 string. It uses a modified UTF-8 encoding. See discussions like
> > > >
> > >
> >
>
http://stackoverflow.com/questions/32205446/getting-true-utf-8-characters-in-....
> > > > So passing a JAVA String with UTF-8 content through JNI is a bit
painful.
> > > > 
> > > > I think this can be a reason that proto buffer JAVA API does not use
JAVA
> > > > String. It uses byte array instead.
> > > 
> > > I see. I think it would be better to fix JavaToStdString to return a plain
> > (non
> > > Java-modified) UTF8 string instead, and not add a separate function for
byte
> > > arrays. Can't you just replace the implementation of JavaToStdString with
> the
> > > new JavaByteArrayToStdString implementation where you get the byte array
> with
> > > String.getBytes(StandardCharsets.UTF_8) in JNI?
> > 
> > Hi Magnus,
> > 
> > I tried to change only the helper method, and made it to work. See PS2. But
it
> > reads differently than we thought.
> > 
> > What I eventually need is convert a Java byte[] to C++ std::string.
> > 
> > Now we have to go through Java bytes[] -> Java string -> C++ std::string,
> where
> > the beginning and end expect memory exact. Any re-encoding into and from the
> > Java string in middle can be wrong except for ISO-8859-1.
> > 
> > I prefer my earlier solution, since it is easier to understand. WDYT?
> 
> To add, I should be wrong from the beginning by saying that we would like to
> pass a UTF-8 string with MediaContraints. Actually, we would like it to pass a
> byte array.

I think we should fix JavaToStdString if it's currently broken regardless if we
add a new Java byte[] -> std::string function. I looked at the corresponding
function in Chromium (ConvertJavaStringToUTF8,
https://cs.chromium.org/chromium/src/base/android/jni_string.cc?rcl=0&l=42) and
they are doing a similar workaround as your latest patch set.

Can we start with landing this CL for just fixing JavaToStdString and it should
solve your problems in the near term? You can then upload a new CL for adding a
Java byte[] ctor in MediaConstraints.KeyValuePair and a new Java byte[] ->
std::string function. I agree it's better to not go via Java String if we start
with Java byte[]. I don't see where we ever start with a Java byte[] for
MediaConstraints.KeyValuePair in the WebRTC code base so I guess this is for
external use?

[minyue-webrtc, 2016-12-08 10:45:25]

On 2016/12/08 09:57:32, magjed_webrtc wrote:
> On 2016/12/07 18:53:52, minyue-webrtc wrote:
> > On 2016/12/07 15:39:48, minyue-webrtc wrote:
> > > On 2016/12/05 10:26:44, magjed_webrtc wrote:
> > > > On 2016/12/05 08:36:26, minyue-webrtc wrote:
> > > > > On 2016/12/03 12:03:42, magjed_webrtc wrote:
> > > > > > I don't understand the problem, why do you want to hold it in Java
as
> a
> > > > UTF-8
> > > > > > string? Why doesn't the standard UTF-16 work?
> > > > > > 
> > > > > > You start with a Java String in MediaConstraints.java and eventually
> end
> > > up
> > > > > with
> > > > > > a std::string, so that sounds exactly what JavaToStdString should do
> > > > already.
> > > > > So
> > > > > > I'm wondering when will the result of JavaToStdString(java_string)
be
> > > > > different
> > > > > > from the new:
> > > > > >
> JavaByteArrayToStdString(java_string.getBytes(StandardCharsets.UTF_8))?
> > > > > 
> > > > > Hi Magnus,
> > > > > 
> > > > > The actual problem there is that the JNI method GetStringUTFChars does
> not
> > > > give
> > > > > UTF-8 string. It uses a modified UTF-8 encoding. See discussions like
> > > > >
> > > >
> > >
> >
>
http://stackoverflow.com/questions/32205446/getting-true-utf-8-characters-in-....
> > > > > So passing a JAVA String with UTF-8 content through JNI is a bit
> painful.
> > > > > 
> > > > > I think this can be a reason that proto buffer JAVA API does not use
> JAVA
> > > > > String. It uses byte array instead.
> > > > 
> > > > I see. I think it would be better to fix JavaToStdString to return a
plain
> > > (non
> > > > Java-modified) UTF8 string instead, and not add a separate function for
> byte
> > > > arrays. Can't you just replace the implementation of JavaToStdString
with
> > the
> > > > new JavaByteArrayToStdString implementation where you get the byte array
> > with
> > > > String.getBytes(StandardCharsets.UTF_8) in JNI?
> > > 
> > > Hi Magnus,
> > > 
> > > I tried to change only the helper method, and made it to work. See PS2.
But
> it
> > > reads differently than we thought.
> > > 
> > > What I eventually need is convert a Java byte[] to C++ std::string.
> > > 
> > > Now we have to go through Java bytes[] -> Java string -> C++ std::string,
> > where
> > > the beginning and end expect memory exact. Any re-encoding into and from
the
> > > Java string in middle can be wrong except for ISO-8859-1.
> > > 
> > > I prefer my earlier solution, since it is easier to understand. WDYT?
> > 
> > To add, I should be wrong from the beginning by saying that we would like to
> > pass a UTF-8 string with MediaContraints. Actually, we would like it to pass
a
> > byte array.
> 
> I think we should fix JavaToStdString if it's currently broken regardless if
we
> add a new Java byte[] -> std::string function. I looked at the corresponding
> function in Chromium (ConvertJavaStringToUTF8,
> https://cs.chromium.org/chromium/src/base/android/jni_string.cc?rcl=0&l=42)
and
> they are doing a similar workaround as your latest patch set.
> 
> Can we start with landing this CL for just fixing JavaToStdString and it
should
> solve your problems in the near term? You can then upload a new CL for adding
a
> Java byte[] ctor in MediaConstraints.KeyValuePair and a new Java byte[] ->
> std::string function. I agree it's better to not go via Java String if we
start
> with Java byte[]. I don't see where we ever start with a Java byte[] for
> MediaConstraints.KeyValuePair in the WebRTC code base so I guess this is for
> external use?

Hi Magnus,

This is not about fixing a current breakage. It is a feature to add.
Specifically, we are going to pass a protobuf serialized string (byte[]) to
WebRTC to config audio network adapter. So I would perfer adding Java byte[]
ctor in MediaConstraints.KeyValuePair and a new Java byte[] -> std::string
function in this CL.

[magjed_webrtc, 2016-12-08 12:22:02]

On 2016/12/08 10:45:25, minyue-webrtc wrote:
> On 2016/12/08 09:57:32, magjed_webrtc wrote:
> > On 2016/12/07 18:53:52, minyue-webrtc wrote:
> > > On 2016/12/07 15:39:48, minyue-webrtc wrote:
> > > > On 2016/12/05 10:26:44, magjed_webrtc wrote:
> > > > > On 2016/12/05 08:36:26, minyue-webrtc wrote:
> > > > > > On 2016/12/03 12:03:42, magjed_webrtc wrote:
> > > > > > > I don't understand the problem, why do you want to hold it in Java
> as
> > a
> > > > > UTF-8
> > > > > > > string? Why doesn't the standard UTF-16 work?
> > > > > > > 
> > > > > > > You start with a Java String in MediaConstraints.java and
eventually
> > end
> > > > up
> > > > > > with
> > > > > > > a std::string, so that sounds exactly what JavaToStdString should
do
> > > > > already.
> > > > > > So
> > > > > > > I'm wondering when will the result of JavaToStdString(java_string)
> be
> > > > > > different
> > > > > > > from the new:
> > > > > > >
> > JavaByteArrayToStdString(java_string.getBytes(StandardCharsets.UTF_8))?
> > > > > > 
> > > > > > Hi Magnus,
> > > > > > 
> > > > > > The actual problem there is that the JNI method GetStringUTFChars
does
> > not
> > > > > give
> > > > > > UTF-8 string. It uses a modified UTF-8 encoding. See discussions
like
> > > > > >
> > > > >
> > > >
> > >
> >
>
http://stackoverflow.com/questions/32205446/getting-true-utf-8-characters-in-....
> > > > > > So passing a JAVA String with UTF-8 content through JNI is a bit
> > painful.
> > > > > > 
> > > > > > I think this can be a reason that proto buffer JAVA API does not use
> > JAVA
> > > > > > String. It uses byte array instead.
> > > > > 
> > > > > I see. I think it would be better to fix JavaToStdString to return a
> plain
> > > > (non
> > > > > Java-modified) UTF8 string instead, and not add a separate function
for
> > byte
> > > > > arrays. Can't you just replace the implementation of JavaToStdString
> with
> > > the
> > > > > new JavaByteArrayToStdString implementation where you get the byte
array
> > > with
> > > > > String.getBytes(StandardCharsets.UTF_8) in JNI?
> > > > 
> > > > Hi Magnus,
> > > > 
> > > > I tried to change only the helper method, and made it to work. See PS2.
> But
> > it
> > > > reads differently than we thought.
> > > > 
> > > > What I eventually need is convert a Java byte[] to C++ std::string.
> > > > 
> > > > Now we have to go through Java bytes[] -> Java string -> C++
std::string,
> > > where
> > > > the beginning and end expect memory exact. Any re-encoding into and from
> the
> > > > Java string in middle can be wrong except for ISO-8859-1.
> > > > 
> > > > I prefer my earlier solution, since it is easier to understand. WDYT?
> > > 
> > > To add, I should be wrong from the beginning by saying that we would like
to
> > > pass a UTF-8 string with MediaContraints. Actually, we would like it to
pass
> a
> > > byte array.
> > 
> > I think we should fix JavaToStdString if it's currently broken regardless if
> we
> > add a new Java byte[] -> std::string function. I looked at the corresponding
> > function in Chromium (ConvertJavaStringToUTF8,
> > https://cs.chromium.org/chromium/src/base/android/jni_string.cc?rcl=0&l=42)
> and
> > they are doing a similar workaround as your latest patch set.
> > 
> > Can we start with landing this CL for just fixing JavaToStdString and it
> should
> > solve your problems in the near term? You can then upload a new CL for
adding
> a
> > Java byte[] ctor in MediaConstraints.KeyValuePair and a new Java byte[] ->
> > std::string function. I agree it's better to not go via Java String if we
> start
> > with Java byte[]. I don't see where we ever start with a Java byte[] for
> > MediaConstraints.KeyValuePair in the WebRTC code base so I guess this is for
> > external use?
> 
> Hi Magnus,
> 
> This is not about fixing a current breakage. It is a feature to add.
> Specifically, we are going to pass a protobuf serialized string (byte[]) to
> WebRTC to config audio network adapter. So I would perfer adding Java byte[]
> ctor in MediaConstraints.KeyValuePair and a new Java byte[] -> std::string
> function in this CL.

If I understand correctly, JavaToStdString is currently broken because it
returns a "modified" UTF-8 byte encoding, so for example if you start with an
std::string x you are not guaranteed that x ==
JavaToStdString(JavaStringFromStdString(x)). Is this not the case? And since you
have already implemented a fix for this, I want to land it. If you want to land
it here or upload a different CL with the fix doesn't matter.

Also, I was under the impression that fixing JavaToStdString would unblock your
feature. Is that not the case?

[minyue-webrtc, 2016-12-09 10:26:58]

Description was changed from

==========
Passing MediaConstraints as byte array.

The Java interface of MediaContraints uses a JAVA string, which has difficulty
to hold a UTF-8 string. This CL tries to switch it to byte array.

A planned use of this is to pass a protobuf serialized string from JAVA app to
WebRTC to configure audio network adaptor.

BUG=webrtc:6815
==========

to

==========
Modify JavaToStdString to allow ISO-8859-1 encoded strings.

Current implementation of JavaToStdString applies additional encoding that
modifies ISO-8859-1 encoded strings (e.g. byte array). This CL is to fix this.

A planned use of this is to pass a protobuf serialized string as a
MediaConstraint to WebRTC to configure audio network adaptor.

BUG=webrtc:6815
==========

[minyue-webrtc, 2016-12-09 10:29:58]

On 2016/12/08 12:22:02, magjed_webrtc wrote:
> On 2016/12/08 10:45:25, minyue-webrtc wrote:
> > On 2016/12/08 09:57:32, magjed_webrtc wrote:
> > > On 2016/12/07 18:53:52, minyue-webrtc wrote:
> > > > On 2016/12/07 15:39:48, minyue-webrtc wrote:
> > > > > On 2016/12/05 10:26:44, magjed_webrtc wrote:
> > > > > > On 2016/12/05 08:36:26, minyue-webrtc wrote:
> > > > > > > On 2016/12/03 12:03:42, magjed_webrtc wrote:
> > > > > > > > I don't understand the problem, why do you want to hold it in
Java
> > as
> > > a
> > > > > > UTF-8
> > > > > > > > string? Why doesn't the standard UTF-16 work?
> > > > > > > > 
> > > > > > > > You start with a Java String in MediaConstraints.java and
> eventually
> > > end
> > > > > up
> > > > > > > with
> > > > > > > > a std::string, so that sounds exactly what JavaToStdString
should
> do
> > > > > > already.
> > > > > > > So
> > > > > > > > I'm wondering when will the result of
JavaToStdString(java_string)
> > be
> > > > > > > different
> > > > > > > > from the new:
> > > > > > > >
> > > JavaByteArrayToStdString(java_string.getBytes(StandardCharsets.UTF_8))?
> > > > > > > 
> > > > > > > Hi Magnus,
> > > > > > > 
> > > > > > > The actual problem there is that the JNI method GetStringUTFChars
> does
> > > not
> > > > > > give
> > > > > > > UTF-8 string. It uses a modified UTF-8 encoding. See discussions
> like
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
http://stackoverflow.com/questions/32205446/getting-true-utf-8-characters-in-....
> > > > > > > So passing a JAVA String with UTF-8 content through JNI is a bit
> > > painful.
> > > > > > > 
> > > > > > > I think this can be a reason that proto buffer JAVA API does not
use
> > > JAVA
> > > > > > > String. It uses byte array instead.
> > > > > > 
> > > > > > I see. I think it would be better to fix JavaToStdString to return a
> > plain
> > > > > (non
> > > > > > Java-modified) UTF8 string instead, and not add a separate function
> for
> > > byte
> > > > > > arrays. Can't you just replace the implementation of JavaToStdString
> > with
> > > > the
> > > > > > new JavaByteArrayToStdString implementation where you get the byte
> array
> > > > with
> > > > > > String.getBytes(StandardCharsets.UTF_8) in JNI?
> > > > > 
> > > > > Hi Magnus,
> > > > > 
> > > > > I tried to change only the helper method, and made it to work. See
PS2.
> > But
> > > it
> > > > > reads differently than we thought.
> > > > > 
> > > > > What I eventually need is convert a Java byte[] to C++ std::string.
> > > > > 
> > > > > Now we have to go through Java bytes[] -> Java string -> C++
> std::string,
> > > > where
> > > > > the beginning and end expect memory exact. Any re-encoding into and
from
> > the
> > > > > Java string in middle can be wrong except for ISO-8859-1.
> > > > > 
> > > > > I prefer my earlier solution, since it is easier to understand. WDYT?
> > > > 
> > > > To add, I should be wrong from the beginning by saying that we would
like
> to
> > > > pass a UTF-8 string with MediaContraints. Actually, we would like it to
> pass
> > a
> > > > byte array.
> > > 
> > > I think we should fix JavaToStdString if it's currently broken regardless
if
> > we
> > > add a new Java byte[] -> std::string function. I looked at the
corresponding
> > > function in Chromium (ConvertJavaStringToUTF8,
> > >
https://cs.chromium.org/chromium/src/base/android/jni_string.cc?rcl=0&l=42)
> > and
> > > they are doing a similar workaround as your latest patch set.
> > > 
> > > Can we start with landing this CL for just fixing JavaToStdString and it
> > should
> > > solve your problems in the near term? You can then upload a new CL for
> adding
> > a
> > > Java byte[] ctor in MediaConstraints.KeyValuePair and a new Java byte[] ->
> > > std::string function. I agree it's better to not go via Java String if we
> > start
> > > with Java byte[]. I don't see where we ever start with a Java byte[] for
> > > MediaConstraints.KeyValuePair in the WebRTC code base so I guess this is
for
> > > external use?
> > 
> > Hi Magnus,
> > 
> > This is not about fixing a current breakage. It is a feature to add.
> > Specifically, we are going to pass a protobuf serialized string (byte[]) to
> > WebRTC to config audio network adapter. So I would perfer adding Java byte[]
> > ctor in MediaConstraints.KeyValuePair and a new Java byte[] -> std::string
> > function in this CL.
> 
> If I understand correctly, JavaToStdString is currently broken because it
> returns a "modified" UTF-8 byte encoding, so for example if you start with an
> std::string x you are not guaranteed that x ==
> JavaToStdString(JavaStringFromStdString(x)). Is this not the case? And since
you
> have already implemented a fix for this, I want to land it. If you want to
land
> it here or upload a different CL with the fix doesn't matter.
> 
> Also, I was under the impression that fixing JavaToStdString would unblock
your
> feature. Is that not the case?

It would be good to land PS2. No actual need to additionally send PS1 as another
CL, since PS2 should solve the problem.

I updated the CL description according to the PS2.

PTAL

[magjed_webrtc, 2016-12-09 10:44:49]

lgtm

[minyue-webrtc, 2016-12-09 10:49:46]

The CQ bit was checked by minyue@webrtc.org

[commit-bot: I haz the power, 2016-12-09 10:49:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-12-09 10:55:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-09 10:55:06]

Try jobs failed on following builders:
  android_clang_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_clang_dbg/build...)
  android_compile_mips_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_mips_db...)
  android_compile_x64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x64_dbg...)

[minyue-webrtc, 2016-12-09 13:55:48]

The CQ bit was checked by minyue@webrtc.org

[minyue-webrtc, 2016-12-09 13:55:48]

The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2549783002/#ps60001 (title:
"adding missing header")

[commit-bot: I haz the power, 2016-12-09 13:55:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-12-09 14:13:40]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481291748378060,
"parent_rev": "9225f5a1e4e60c15db60ca623dfe3578defc917e", "commit_rev":
"ef8088d31d99131d80cf98c64a261036104542f0"}

[commit-bot: I haz the power, 2016-12-09 14:13:42]

Description was changed from

==========
Modify JavaToStdString to allow ISO-8859-1 encoded strings.

Current implementation of JavaToStdString applies additional encoding that
modifies ISO-8859-1 encoded strings (e.g. byte array). This CL is to fix this.

A planned use of this is to pass a protobuf serialized string as a
MediaConstraint to WebRTC to configure audio network adaptor.

BUG=webrtc:6815
==========

to

==========
Modify JavaToStdString to allow ISO-8859-1 encoded strings.

Current implementation of JavaToStdString applies additional encoding that
modifies ISO-8859-1 encoded strings (e.g. byte array). This CL is to fix this.

A planned use of this is to pass a protobuf serialized string as a
MediaConstraint to WebRTC to configure audio network adaptor.

BUG=webrtc:6815

Review-Url: https://codereview.webrtc.org/2549783002
==========

[commit-bot: I haz the power, 2016-12-09 14:13:43]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-12-09 14:13:52]

Description was changed from

==========
Modify JavaToStdString to allow ISO-8859-1 encoded strings.

Current implementation of JavaToStdString applies additional encoding that
modifies ISO-8859-1 encoded strings (e.g. byte array). This CL is to fix this.

A planned use of this is to pass a protobuf serialized string as a
MediaConstraint to WebRTC to configure audio network adaptor.

BUG=webrtc:6815

Review-Url: https://codereview.webrtc.org/2549783002
==========

to

==========
Modify JavaToStdString to allow ISO-8859-1 encoded strings.

Current implementation of JavaToStdString applies additional encoding that
modifies ISO-8859-1 encoded strings (e.g. byte array). This CL is to fix this.

A planned use of this is to pass a protobuf serialized string as a
MediaConstraint to WebRTC to configure audio network adaptor.

BUG=webrtc:6815

Committed: https://crrev.com/50254636d4c9bedb333e65c906b54e1f36305571
Cr-Commit-Position: refs/heads/master@{#15509}
==========

[commit-bot: I haz the power, 2016-12-09 14:13:54]

Patchset 3 (id:??) landed as
https://crrev.com/50254636d4c9bedb333e65c906b54e1f36305571
Cr-Commit-Position: refs/heads/master@{#15509}