Add missing using declarations in cronet code.

Add missing using declarations in cronet code.

Currently jni_generator_helper.h includes a using statement for
JavaParamRef and ScopedJavaLocalRef. To allow this to be removed (since
it shouldn't really be there in a header file), add a "using" statement
to cronet-specific files that do this (already removed for other android
configs).

BUG=568574
Committed: https://crrev.com/bc8890f04a40ef61b0f3b02e08a0fe6980aa81f5
Cr-Commit-Position: refs/heads/master@{#410032}

[Torne, 2016-08-04 17:05:39]

torne@chromium.org changed reviewers:
+ mef@chromium.org

[Torne, 2016-08-04 17:05:40]

While debugging that build breakage I realised a totally different change I'm
working on was about to break cronet building as well - this CL is the fix for
that :)

[mef, 2016-08-04 17:26:00]

https://codereview.chromium.org/2215903002/diff/1/components/cronet/android/c...
File components/cronet/android/chromium_url_request.cc (right):

https://codereview.chromium.org/2215903002/diff/1/components/cronet/android/c...
components/cronet/android/chromium_url_request.cc:19: using
base::android::JavaParamRef;
I think we need to add #include "base/android/scoped_java_ref.h" for this here
and elsewhere.

[Torne, 2016-08-04 18:24:16]

On 2016/08/04 17:26:00, mef wrote:
>
https://codereview.chromium.org/2215903002/diff/1/components/cronet/android/c...
> File components/cronet/android/chromium_url_request.cc (right):
> 
>
https://codereview.chromium.org/2215903002/diff/1/components/cronet/android/c...
> components/cronet/android/chromium_url_request.cc:19: using
> base::android::JavaParamRef;
> I think we need to add #include "base/android/scoped_java_ref.h" for this here
> and elsewhere.

Strictly for IWYU, yes, but it's included via the JNI generated header anyway
(unavoidably). I just want it to keep compiling when I remove the using
declarations right now. If someone wants to fix up the includes that's fine :)

[mef, 2016-08-04 18:53:08]

reluctantly lgtm.

[Torne, 2016-08-04 19:06:23]

On 2016/08/04 18:53:08, mef wrote:
> reluctantly lgtm.

I can script including this where necessary after this round of cleanups if you
want, but chromium isn't using IWYU tooling in general and so it's inevitably
going to regress as people add new JNI classes. I can't remove the transitive
include as the generated JNI headers need access to the actual contents of the
type, so any source file that includes a JNI header is always going to be able
to get away without including scoped_java_ref.h :/

[Torne, 2016-08-04 19:43:36]

The CQ bit was checked by torne@chromium.org

[commit-bot: I haz the power, 2016-08-04 19:44:14]

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-04 19:54:14]

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

[commit-bot: I haz the power, 2016-08-04 19:54:14]

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...)

[Torne, 2016-08-04 20:05:43]

torne@chromium.org changed reviewers:
+ mkwst@chromium.org

[Torne, 2016-08-04 20:05:43]

+mkwst for url OWNERS

[Mike West, 2016-08-05 10:54:13]

LGTM.

[Torne, 2016-08-05 11:16:41]

The CQ bit was checked by torne@chromium.org

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

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-05 11:49:03]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-08-05 11:51:17]

Description was changed from

==========
Add missing using declarations in cronet code.

Currently jni_generator_helper.h includes a using statement for
JavaParamRef and ScopedJavaLocalRef. To allow this to be removed (since
it shouldn't really be there in a header file), add a "using" statement
to cronet-specific files that do this (already removed for other android
configs).

BUG=568574
==========

to

==========
Add missing using declarations in cronet code.

Currently jni_generator_helper.h includes a using statement for
JavaParamRef and ScopedJavaLocalRef. To allow this to be removed (since
it shouldn't really be there in a header file), add a "using" statement
to cronet-specific files that do this (already removed for other android
configs).

BUG=568574

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

[commit-bot: I haz the power, 2016-08-05 11:51:18]

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

[Torne, 2016-08-05 12:13:44]

On 2016/08/04 19:06:23, Torne wrote:
> On 2016/08/04 18:53:08, mef wrote:
> > reluctantly lgtm.
> 
> I can script including this where necessary after this round of cleanups if
you
> want, but chromium isn't using IWYU tooling in general and so it's inevitably
> going to regress as people add new JNI classes. I can't remove the transitive
> include as the generated JNI headers need access to the actual contents of the
> type, so any source file that includes a JNI header is always going to be able
> to get away without including scoped_java_ref.h :/

I tried doing this and I can produce a CL that adds an #include in all the
places that reference the types but don't already include the header in their
.cc or .h (182 files) but I'm not sure it's actually necessary/desirable to add
an include in all cases - for some of them a forward declaration probably would
suffice (especially headers), and I can't think of a very simple way to
determine this since it compiles in any case due to the transitive inclusions.

If IWYU actually worked on chromium it would be able to do it, I guess, but it
doesn't look like it's being worked on right now.

What do you think?

[mef, 2016-08-05 13:29:08]

On 2016/08/05 12:13:44, Torne wrote:
> On 2016/08/04 19:06:23, Torne wrote:
> > On 2016/08/04 18:53:08, mef wrote:
> > > reluctantly lgtm.
> > 
> > I can script including this where necessary after this round of cleanups if
> you
> > want, but chromium isn't using IWYU tooling in general and so it's
inevitably
> > going to regress as people add new JNI classes. I can't remove the
transitive
> > include as the generated JNI headers need access to the actual contents of
the
> > type, so any source file that includes a JNI header is always going to be
able
> > to get away without including scoped_java_ref.h :/
> 
> I tried doing this and I can produce a CL that adds an #include in all the
> places that reference the types but don't already include the header in their
> .cc or .h (182 files) but I'm not sure it's actually necessary/desirable to
add
> an include in all cases - for some of them a forward declaration probably
would
> suffice (especially headers), and I can't think of a very simple way to
> determine this since it compiles in any case due to the transitive inclusions.
> 
> If IWYU actually worked on chromium it would be able to do it, I guess, but it
> doesn't look like it's being worked on right now.
> 
> What do you think?

Sorry for being PITA, and thanks for doing this!
I agree that this would add more unnecessary overhead without clear benefit.
I guess we should just leave it as is.

[Torne, 2016-08-05 13:31:06]

On 2016/08/05 13:29:08, mef wrote:
> On 2016/08/05 12:13:44, Torne wrote:
> > On 2016/08/04 19:06:23, Torne wrote:
> > > On 2016/08/04 18:53:08, mef wrote:
> > > > reluctantly lgtm.
> > > 
> > > I can script including this where necessary after this round of cleanups
if
> > you
> > > want, but chromium isn't using IWYU tooling in general and so it's
> inevitably
> > > going to regress as people add new JNI classes. I can't remove the
> transitive
> > > include as the generated JNI headers need access to the actual contents of
> the
> > > type, so any source file that includes a JNI header is always going to be
> able
> > > to get away without including scoped_java_ref.h :/
> > 
> > I tried doing this and I can produce a CL that adds an #include in all the
> > places that reference the types but don't already include the header in
their
> > .cc or .h (182 files) but I'm not sure it's actually necessary/desirable to
> add
> > an include in all cases - for some of them a forward declaration probably
> would
> > suffice (especially headers), and I can't think of a very simple way to
> > determine this since it compiles in any case due to the transitive
inclusions.
> > 
> > If IWYU actually worked on chromium it would be able to do it, I guess, but
it
> > doesn't look like it's being worked on right now.
> > 
> > What do you think?
> 
> Sorry for being PITA, and thanks for doing this!

It's totally fine; *because* i did it in a very simple way it only took ten
minutes to try. :)

> I agree that this would add more unnecessary overhead without clear benefit.
> I guess we should just leave it as is.