Move JNI bindings for url_formatter from chrome to //components/url_formatter

Move JNI bindings for url_formatter from chrome to //components/url_formatter

By moving the url_formatter methods out of
org.chromium.chrome.browser.UrlUtilities to it's component, other emdedders than
Chrome can benefit from the Java version.

This CL removes the tests originally written for a Java implementation of the
methods replaced by FormatUrlForSecurityDisplay in
https://codereview.chromium.org/1357563002. It's not trivial to move the tests
over to the component as they depend on native library initialization performed
by //content which is disallowed in components/url_formatter/DEPS. Also, the
tests are redundant as the code is thoroughly tested by other means.

Currently there are no users of formatUrlForDisplay() in Chromium, but Opera
would like it exposed.

BUG=624407
Committed: https://crrev.com/b232498ead0784193c039ea1997ecafc391503ef
Cr-Commit-Position: refs/heads/master@{#414356}

[ingemara, 2016-06-29 13:48:04]

The CQ bit was checked by ingemara@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-29 13:48:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-29 13:51:12]

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

[commit-bot: I haz the power, 2016-06-29 13:51:13]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[ingemara, 2016-06-29 13:57:23]

The CQ bit was unchecked by ingemara@opera.com

[ingemara, 2016-06-29 14:02:23]

The CQ bit was checked by ingemara@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-29 14:02:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-29 14:06:13]

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

[commit-bot: I haz the power, 2016-06-29 14:06:14]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[ingemara, 2016-06-29 17:14:48]

The CQ bit was unchecked by ingemara@opera.com

[ingemara, 2016-06-29 17:15:25]

The CQ bit was checked by ingemara@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-29 17:16:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-29 18:34:37]

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

[commit-bot: I haz the power, 2016-06-29 18:34:38]

Dry run: This issue passed the CQ dry run.

[ingemara, 2016-06-30 09:04:11]

Description was changed from

==========
Move JNI bindings for url_formatter from chrome to //components/url_formatter

By moving the url_formatter methods out of
org.chromium.chrome.browser.UrlUtilities to it's component, other emdedders than
Chrome can benefit from the Java version.

Currently there are no users of formatUrlForDisplay() in Chromium, but Opera
would like it exposed.

BUG=624407
==========

to

==========
Move JNI bindings for url_formatter from chrome to //components/url_formatter

By moving the url_formatter methods out of
org.chromium.chrome.browser.UrlUtilities to it's component, other emdedders than
Chrome can benefit from the Java version.

This CL removes the tests originally written for a Java implementation of the
methods replaced by FormatUrlForSecurityDisplay in
https://codereview.chromium.org/1357563002. Its not trivial to move the tests
over to the component as they depend on native library initialization performed
by //content which is disallowed in components/url_formatter/DEPS. Also, the
tests are redundant as the code is thoroughly tested by other means.

Currently there are no users of formatUrlForDisplay() in Chromium, but Opera
would like it exposed.

BUG=624407
==========

[ingemara, 2016-06-30 09:04:11]

ingemara@opera.com changed reviewers:
+ mlamouri@chromium.org, pkasting@chromium.org

[ingemara, 2016-06-30 09:04:36]

Description was changed from

==========
Move JNI bindings for url_formatter from chrome to //components/url_formatter

By moving the url_formatter methods out of
org.chromium.chrome.browser.UrlUtilities to it's component, other emdedders than
Chrome can benefit from the Java version.

This CL removes the tests originally written for a Java implementation of the
methods replaced by FormatUrlForSecurityDisplay in
https://codereview.chromium.org/1357563002. Its not trivial to move the tests
over to the component as they depend on native library initialization performed
by //content which is disallowed in components/url_formatter/DEPS. Also, the
tests are redundant as the code is thoroughly tested by other means.

Currently there are no users of formatUrlForDisplay() in Chromium, but Opera
would like it exposed.

BUG=624407
==========

to

==========
Move JNI bindings for url_formatter from chrome to //components/url_formatter

By moving the url_formatter methods out of
org.chromium.chrome.browser.UrlUtilities to it's component, other emdedders than
Chrome can benefit from the Java version.

This CL removes the tests originally written for a Java implementation of the
methods replaced by FormatUrlForSecurityDisplay in
https://codereview.chromium.org/1357563002. Its not trivial to move the tests
over to the component as they depend on native library initialization performed
by //content which is disallowed in components/url_formatter/DEPS. Also, the
tests are redundant as the code is thoroughly tested by other means.

Currently there are no users of formatUrlForDisplay() in Chromium, but Opera
would like it exposed.

BUG=624407
==========

[ingemara, 2016-06-30 09:05:22]

Description was changed from

==========
Move JNI bindings for url_formatter from chrome to //components/url_formatter

By moving the url_formatter methods out of
org.chromium.chrome.browser.UrlUtilities to it's component, other emdedders than
Chrome can benefit from the Java version.

This CL removes the tests originally written for a Java implementation of the
methods replaced by FormatUrlForSecurityDisplay in
https://codereview.chromium.org/1357563002. Its not trivial to move the tests
over to the component as they depend on native library initialization performed
by //content which is disallowed in components/url_formatter/DEPS. Also, the
tests are redundant as the code is thoroughly tested by other means.

Currently there are no users of formatUrlForDisplay() in Chromium, but Opera
would like it exposed.

BUG=624407
==========

to

==========
Move JNI bindings for url_formatter from chrome to //components/url_formatter

By moving the url_formatter methods out of
org.chromium.chrome.browser.UrlUtilities to it's component, other emdedders than
Chrome can benefit from the Java version.

This CL removes the tests originally written for a Java implementation of the
methods replaced by FormatUrlForSecurityDisplay in
https://codereview.chromium.org/1357563002. It's not trivial to move the tests
over to the component as they depend on native library initialization performed
by //content which is disallowed in components/url_formatter/DEPS. Also, the
tests are redundant as the code is thoroughly tested by other means.

Currently there are no users of formatUrlForDisplay() in Chromium, but Opera
would like it exposed.

BUG=624407
==========

[ingemara, 2016-06-30 09:10:05]

PTAL and feel free to suggest any reviewers you might think would be suitable
reviewing this cl.

I'm a bit unsure about whether I have to do anything to any .gyp files. There is
no //components/url_formatter.gypi and no mention of "url_formatter" in
//components/components.gyp.

[mlamouri (slow - plz ping), 2016-06-30 10:55:35]

usage lgtm

[sdefresne, 2016-06-30 11:21:43]

drive-by:

The gyp changes should be made to components/url_formatter/url_formatter.gyp.

[ingemara, 2016-06-30 13:57:21]

On 2016/06/30 11:21:43, sdefresne wrote:
> drive-by:
> 
> The gyp changes should be made to components/url_formatter/url_formatter.gyp.

Thanks, totally missed it. I added the corresponding gyp changes, but I don't
think that Android + gyp is working at this point, which made it kind impossible
to verify that it works. I tried checking out the LKGR but still got unrelated
errors while running build/gyp_chromium (configured for Android, "default" works
fine).

[Peter Kasting, 2016-07-01 06:59:33]

Did not review any .java files

https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
File components/url_formatter/BUILD.gn (right):

https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
components/url_formatter/BUILD.gn:27: if (!is_android) {
Nit: Reverse condition and arms

https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
File components/url_formatter/android/BUILD.gn (right):

https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
components/url_formatter/android/BUILD.gn:13: [
"java/src/org/chromium/components/url_formatter/UrlFormatter.java" ]
Nit: Should this be formatted like the others?:

  java_files = [
    "java/src/org/chromium/components/url_formatter/UrlFormatter.java",
  ]

https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
File components/url_formatter/android/component_jni_registrar.cc (right):

https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
components/url_formatter/android/component_jni_registrar.cc:15: static
base::android::RegistrationMethod kUrlFormatterRegisteredMethods[] = {
Nit: Declare this inside the function below to keep the scope as narrow as
possible.  Can maybe be constexpr as well.  Probably doesn't need to be
function-static.

https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
File components/url_formatter/url_formatter_android.cc (right):

https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
components/url_formatter/url_formatter_android.cc:17: using
base::android::ConvertJavaStringToUTF8;
Nit: Avoid this; you already fully-qualify one use below so this is only for one
spot, and you might as well just put the qualifier in there.

https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
components/url_formatter/url_formatter_android.cc:31: static
ScopedJavaLocalRef<jstring> FixupUrl(JNIEnv* env,
Where are any of these static functions exposed to any callers?  It doesn't seem
like anyone can use them.

Nit: Prefer anonymous namespaces to marking functions static.

[ingemara, 2016-07-01 08:52:27]

On 2016/07/01 06:59:33, Peter Kasting (slow) wrote:
>
https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
> components/url_formatter/android/BUILD.gn:13: [
> "java/src/org/chromium/components/url_formatter/UrlFormatter.java" ]
> Nit: Should this be formatted like the others?:
> 
>   java_files = [
>     "java/src/org/chromium/components/url_formatter/UrlFormatter.java",
>   ]

This is what 'git cl format' thinks I should do. Strangely enough, it doesn't
like this formatting for 'sources'. If you insist, I can change 'java_files' to
follow 'sources', but it's a bit of a pain when the gn formatter disagrees.

>
https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
> components/url_formatter/android/component_jni_registrar.cc:15: static
> base::android::RegistrationMethod kUrlFormatterRegisteredMethods[] = {
> Nit: Declare this inside the function below to keep the scope as narrow as
> possible.  Can maybe be constexpr as well.  Probably doesn't need to be
> function-static.

This seems to be the pattern for declaring JNI bindings. 'grep
"RegistrationMethod" $(find components -name component_jni_registrar.cc)' all
yields results identical to this. While I agree that we'd probably be better off
declaring the methods within the registration function, it's probably better to
follow the pattern? I noticed I actually didn't build the
'component_jni_registrar.cc' files, but a naming collision actually made it
work. Fixed it in the last patch set.

>
https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
> components/url_formatter/url_formatter_android.cc:31: static
> ScopedJavaLocalRef<jstring> FixupUrl(JNIEnv* env,
> Where are any of these static functions exposed to any callers?  It doesn't
seem
> like anyone can use them.
> 
> Nit: Prefer anonymous namespaces to marking functions static.

Because the way the JNI generator works. It generates the
"jni/UrlFormatter_jni.h" file which contains 'static' function prototypes which
I'm implementing here. For each of the 'static' functions there's a
corresponding exported function with a JNI-compatible identifier calling "my"
function. Like this:

static ScopedJavaLocalRef<jstring> FixupUrl(JNIEnv* env, const
    JavaParamRef<jclass>& jcaller,
    const JavaParamRef<jstring>& url);

extern "C" __attribute__((visibility("default")))
jstring
   
Java_org_chromium_components_url_1formatter_UrlFormatter_nativeFixupUrl(JNIEnv*
    env, jclass jcaller,
    jstring url) {
  return FixupUrl(env, JavaParamRef<jclass>(env, jcaller),
      JavaParamRef<jstring>(env, url)).Release();
}

It also contains the 'RegisterNativesImpl(JNIEnv*)' function which then iterates
through a generated list of these Java-friendly functions and registers them if
needed.

[Peter Kasting, 2016-07-01 20:34:36]

Small request: when you reply to comments, please do so on the codereview site
itself, using the tools to respond directly on a particular diff.  That way the
context of the discussion is all in the same place.  Replying out-of-line like
this makes it very hard to look at previous patch sets or to come back later and
understand why things changed from one patch set to the next, because all the
discussion is buried in reply messages that have to all be examined closely.

On 2016/07/01 08:52:27, ingemara wrote:
> On 2016/07/01 06:59:33, Peter Kasting (slow) wrote:
> >
>
https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
> > components/url_formatter/android/BUILD.gn:13: [
> > "java/src/org/chromium/components/url_formatter/UrlFormatter.java" ]
> > Nit: Should this be formatted like the others?:
> > 
> >   java_files = [
> >     "java/src/org/chromium/components/url_formatter/UrlFormatter.java",
> >   ]
> 
> This is what 'git cl format' thinks I should do.

git cl format has a concept of GN files?

> Strangely enough, it doesn't
> like this formatting for 'sources'. If you insist, I can change 'java_files'
to
> follow 'sources', but it's a bit of a pain when the gn formatter disagrees.

Is the disagreement because you didn't have a comma at the end of the string,
and if you add one it does what I suggest above?

If not, then the disagreement here is strange.  Please get this filed against
git cl format and fixed so we agree one way or the other.  In the meantime, I
suggest picking whichever way you think makes sense and making the file
consistent one way or the other.

>
https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
> > components/url_formatter/android/component_jni_registrar.cc:15: static
> > base::android::RegistrationMethod kUrlFormatterRegisteredMethods[] = {
> > Nit: Declare this inside the function below to keep the scope as narrow as
> > possible.  Can maybe be constexpr as well.  Probably doesn't need to be
> > function-static.
> 
> This seems to be the pattern for declaring JNI bindings. 'grep
> "RegistrationMethod" $(find components -name component_jni_registrar.cc)' all
> yields results identical to this. While I agree that we'd probably be better
off
> declaring the methods within the registration function, it's probably better
to
> follow the pattern?

I don't think that's the sort of pattern that we need to enforce consistency
with for readability's sake.  If you agree with me that declaring this in-method
is better/more compliant with the style guide's "keep scopes as narrow as
possible" recommendation, I would just do this.

>
https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
> > components/url_formatter/url_formatter_android.cc:31: static
> > ScopedJavaLocalRef<jstring> FixupUrl(JNIEnv* env,
> > Where are any of these static functions exposed to any callers?  It doesn't
> seem
> > like anyone can use them.
> > 
> > Nit: Prefer anonymous namespaces to marking functions static.
> 
> Because the way the JNI generator works. It generates the
> "jni/UrlFormatter_jni.h" file which contains 'static' function prototypes
which
> I'm implementing here.

Huh.  OK... static in a header is weird to me, but I guess the generator is
trying to avoid the possibility of symbol conflicts.

LGTM

[ingemara, 2016-07-04 09:10:49]

On 2016/07/01 20:34:36, Peter Kasting wrote:
> Small request: when you reply to comments, please do so on the codereview site
> itself, using the tools to respond directly on a particular diff.  That way
the
> context of the discussion is all in the same place.  Replying out-of-line like
> this makes it very hard to look at previous patch sets or to come back later
and
> understand why things changed from one patch set to the next, because all the
> discussion is buried in reply messages that have to all be examined closely.

Yes, sorry about that. I actually saw that you could reply in-line just after
hitting "Send Message" :(

> On 2016/07/01 08:52:27, ingemara wrote:
> > On 2016/07/01 06:59:33, Peter Kasting (slow) wrote:
> > >
> >
>
https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
> > > components/url_formatter/android/BUILD.gn:13: [
> > > "java/src/org/chromium/components/url_formatter/UrlFormatter.java" ]
> > > Nit: Should this be formatted like the others?:
> > > 
> > >   java_files = [
> > >     "java/src/org/chromium/components/url_formatter/UrlFormatter.java",
> > >   ]
> > 
> > This is what 'git cl format' thinks I should do.
> 
> git cl format has a concept of GN files?
> 
> > Strangely enough, it doesn't
> > like this formatting for 'sources'. If you insist, I can change 'java_files'
> to
> > follow 'sources', but it's a bit of a pain when the gn formatter disagrees.
> 
> Is the disagreement because you didn't have a comma at the end of the string,
> and if you add one it does what I suggest above?
> 
> If not, then the disagreement here is strange.  Please get this filed against
> git cl format and fixed so we agree one way or the other.  In the meantime, I
> suggest picking whichever way you think makes sense and making the file
> consistent one way or the other.

A trailing comma made no difference. I opened http://crbug.com/625576.

> >
>
https://codereview.chromium.org/2110543004/diff/100001/components/url_formatt...
> > > components/url_formatter/url_formatter_android.cc:31: static
> > > ScopedJavaLocalRef<jstring> FixupUrl(JNIEnv* env,
> > > Where are any of these static functions exposed to any callers?  It
doesn't
> > seem
> > > like anyone can use them.
> > > 
> > > Nit: Prefer anonymous namespaces to marking functions static.
> > 
> > Because the way the JNI generator works. It generates the
> > "jni/UrlFormatter_jni.h" file which contains 'static' function prototypes
> which
> > I'm implementing here.
> 
> Huh.  OK... static in a header is weird to me, but I guess the generator is
> trying to avoid the possibility of symbol conflicts.

Perhaps. It also makes it possible for the compiler to warn when a static is
undefined.

> LGTM

Thanks for taking a look. Before I go ahead and do anything I shouldn't: Should
I try to get a hold of any reviewer for the Java file, or am I good to go?

[Peter Kasting, 2016-07-04 21:57:34]

On 2016/07/04 09:10:49, ingemara wrote:
> Thanks for taking a look. Before I go ahead and do anything I shouldn't:
Should
> I try to get a hold of any reviewer for the Java file, or am I good to go?

Given how many Java files you have and the number of lines in them, including in
the url_cormatter file, I would find a Java reviewer first.

[ingemara, 2016-07-05 12:32:44]

On 2016/06/30 10:55:35, Mounir Lamouri wrote:
> usage lgtm

Do you think you could suggest any potential reviewers for the Java files?

[Peter Kasting, 2016-07-06 20:31:27]

On 2016/07/05 12:32:44, ingemara wrote:
> On 2016/06/30 10:55:35, Mounir Lamouri wrote:
> > usage lgtm
> 
> Do you think you could suggest any potential reviewers for the Java files?

Sorry, I know nothing about Java, Android, etc.  Take a look at the OWNERS in
the source tree as well as the people in git blame.

[Peter Kasting, 2016-08-20 02:58:07]

Bump -- it looks like nothing has happened here, please be sure to set some
possible reviewers for the remaining bits in question and send for review, so
this doesn't just stall.

[ingemara, 2016-08-22 15:23:29]

Description was changed from

==========
Move JNI bindings for url_formatter from chrome to //components/url_formatter

By moving the url_formatter methods out of
org.chromium.chrome.browser.UrlUtilities to it's component, other emdedders than
Chrome can benefit from the Java version.

This CL removes the tests originally written for a Java implementation of the
methods replaced by FormatUrlForSecurityDisplay in
https://codereview.chromium.org/1357563002. It's not trivial to move the tests
over to the component as they depend on native library initialization performed
by //content which is disallowed in components/url_formatter/DEPS. Also, the
tests are redundant as the code is thoroughly tested by other means.

Currently there are no users of formatUrlForDisplay() in Chromium, but Opera
would like it exposed.

BUG=624407
==========

to

==========
Move JNI bindings for url_formatter from chrome to //components/url_formatter

By moving the url_formatter methods out of
org.chromium.chrome.browser.UrlUtilities to it's component, other emdedders than
Chrome can benefit from the Java version.

This CL removes the tests originally written for a Java implementation of the
methods replaced by FormatUrlForSecurityDisplay in
https://codereview.chromium.org/1357563002. It's not trivial to move the tests
over to the component as they depend on native library initialization performed
by //content which is disallowed in components/url_formatter/DEPS. Also, the
tests are redundant as the code is thoroughly tested by other means.

Currently there are no users of formatUrlForDisplay() in Chromium, but Opera
would like it exposed.

BUG=624407
==========

[ingemara, 2016-08-22 15:23:29]

ingemara@opera.com changed reviewers:
+ dfalcantara@chromium.org, thakis@chromium.org

[ingemara, 2016-08-22 15:25:56]

On 2016/08/20 02:58:07, Peter Kasting wrote:
> Bump -- it looks like nothing has happened here, please be sure to set some
> possible reviewers for the remaining bits in question and send for review, so
> this doesn't just stall.

Ah, sorry! Vacation happened and then I totally forgot about this :(

@thakis, @dfalcantara: Would you mind taking a look?

[Nico, 2016-08-22 16:35:49]

what files do you want me to look at?

[gone, 2016-08-22 17:05:28]

What happened to the tests?

[ingemara, 2016-08-23 07:55:13]

On 2016/08/22 17:05:28, dfalcantara wrote:
> What happened to the tests?

The code is already tested in //components/url_formatter/elide_url_unittest.cc.
The tests I removed were more or less just duplicates of the C++ unit tests.

[ingemara, 2016-08-23 07:59:24]

On 2016/08/22 16:35:49, Nico wrote:
> what files do you want me to look at?

Hmm, perhaps 'git cl owners' didn't do a very good job assigning reviewers for
me (I see now lots of lines saying 'per_file *.mm' when grepping OWNERS)? Sorry
about the spam :(

[gone, 2016-08-23 15:34:10]

lgtm

[ingemara, 2016-08-24 08:02:58]

The CQ bit was checked by ingemara@opera.com

[ingemara, 2016-08-24 08:02:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlamouri@chromium.org, pkasting@chromium.org
Link to the patchset:
https://chromiumcodereview.appspot.com/2110543004/#ps160001 (title: "Rebase")

[commit-bot: I haz the power, 2016-08-24 08:03:19]

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

[commit-bot: I haz the power, 2016-08-24 08:05:55]

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

[commit-bot: I haz the power, 2016-08-24 08:05:56]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[ingemara, 2016-08-24 08:16:31]

The CQ bit was checked by ingemara@opera.com

[ingemara, 2016-08-24 08:16:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org, dfalcantara@chromium.org, mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/2110543004/#ps180001
(title: "Rebase")

[commit-bot: I haz the power, 2016-08-24 08:16:50]

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

[commit-bot: I haz the power, 2016-08-24 08:24:15]

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

[commit-bot: I haz the power, 2016-08-24 08:24:17]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[ingemara, 2016-08-24 08:46:17]

ingemara@opera.com changed reviewers:
+ jochen@chromium.org

[ingemara, 2016-08-24 08:47:09]

On 2016/08/24 08:24:17, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

jochen, could you look at the build file changes? I'm missing a LGTM for
//chrome/chrome.gyp.

[jochen (gone - plz use gerrit), 2016-08-24 20:59:45]

lgtm

[ingemara, 2016-08-25 07:10:32]

The CQ bit was checked by ingemara@opera.com

[commit-bot: I haz the power, 2016-08-25 07:10:52]

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

[commit-bot: I haz the power, 2016-08-25 08:13:54]

Description was changed from

==========
Move JNI bindings for url_formatter from chrome to //components/url_formatter

By moving the url_formatter methods out of
org.chromium.chrome.browser.UrlUtilities to it's component, other emdedders than
Chrome can benefit from the Java version.

This CL removes the tests originally written for a Java implementation of the
methods replaced by FormatUrlForSecurityDisplay in
https://codereview.chromium.org/1357563002. It's not trivial to move the tests
over to the component as they depend on native library initialization performed
by //content which is disallowed in components/url_formatter/DEPS. Also, the
tests are redundant as the code is thoroughly tested by other means.

Currently there are no users of formatUrlForDisplay() in Chromium, but Opera
would like it exposed.

BUG=624407
==========

to

==========
Move JNI bindings for url_formatter from chrome to //components/url_formatter

By moving the url_formatter methods out of
org.chromium.chrome.browser.UrlUtilities to it's component, other emdedders than
Chrome can benefit from the Java version.

This CL removes the tests originally written for a Java implementation of the
methods replaced by FormatUrlForSecurityDisplay in
https://codereview.chromium.org/1357563002. It's not trivial to move the tests
over to the component as they depend on native library initialization performed
by //content which is disallowed in components/url_formatter/DEPS. Also, the
tests are redundant as the code is thoroughly tested by other means.

Currently there are no users of formatUrlForDisplay() in Chromium, but Opera
would like it exposed.

BUG=624407
==========

[commit-bot: I haz the power, 2016-08-25 08:13:55]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-08-25 08:15:11]

Description was changed from

==========
Move JNI bindings for url_formatter from chrome to //components/url_formatter

By moving the url_formatter methods out of
org.chromium.chrome.browser.UrlUtilities to it's component, other emdedders than
Chrome can benefit from the Java version.

This CL removes the tests originally written for a Java implementation of the
methods replaced by FormatUrlForSecurityDisplay in
https://codereview.chromium.org/1357563002. It's not trivial to move the tests
over to the component as they depend on native library initialization performed
by //content which is disallowed in components/url_formatter/DEPS. Also, the
tests are redundant as the code is thoroughly tested by other means.

Currently there are no users of formatUrlForDisplay() in Chromium, but Opera
would like it exposed.

BUG=624407
==========

to

==========
Move JNI bindings for url_formatter from chrome to //components/url_formatter

By moving the url_formatter methods out of
org.chromium.chrome.browser.UrlUtilities to it's component, other emdedders than
Chrome can benefit from the Java version.

This CL removes the tests originally written for a Java implementation of the
methods replaced by FormatUrlForSecurityDisplay in
https://codereview.chromium.org/1357563002. It's not trivial to move the tests
over to the component as they depend on native library initialization performed
by //content which is disallowed in components/url_formatter/DEPS. Also, the
tests are redundant as the code is thoroughly tested by other means.

Currently there are no users of formatUrlForDisplay() in Chromium, but Opera
would like it exposed.

BUG=624407

Committed: https://crrev.com/b232498ead0784193c039ea1997ecafc391503ef
Cr-Commit-Position: refs/heads/master@{#414356}
==========

[commit-bot: I haz the power, 2016-08-25 08:15:13]

Patchset 10 (id:??) landed as
https://crrev.com/b232498ead0784193c039ea1997ecafc391503ef
Cr-Commit-Position: refs/heads/master@{#414356}