Make GetApplicationContext return a JavaRef.

Make GetApplicationContext return a JavaRef.

Instead of returning a bare jobject, return a const reference to a
JavaRef, making it clearer that the caller does not receive ownership.
Most uses pass the result directly to a Java method; update the cases
which store it in a variable or pass it elsewhere.

Also, clean up the code surrounding the GetApplicationContext calls a
bit:
- There's no need to DCHECK this or AttachCurrentThread as the
  implementations already do that internally
- Change method parameters that take contexts as jobject to JavaRef, and
  also other parameters of those methods where reasonable.

BUG=506850
Committed: https://crrev.com/314ad6c309924d08e12a66a90c1b07558ad75c50
Cr-Commit-Position: refs/heads/master@{#412219}

[Torne, 2016-08-10 17:39:42]

The CQ bit was checked by torne@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 17:40:30]

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-08-10 18:41:48]

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

[commit-bot: I haz the power, 2016-08-10 18:41:49]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Torne, 2016-08-11 11:26:56]

torne@chromium.org changed reviewers:
+ mcasas@chromium.org, nyquist@chromium.org, qinmin@chromium.org,
rmcilroy@chromium.org, sadrul@chromium.org, yfriedman@chromium.org

[Torne, 2016-08-11 11:26:57]

Hi, this is part of my ongoing effort to use JavaRef instead of bare objects.
Since Java calls now accept JavaRef as well as bare objects, only a few calls to
GetApplicationContext are a problem (ones that store it in a variable) and this
CL addresses those. Where it looks "nice" I've just eliminated the variable; in
other cases types have been updated, and in a few places I also changed other
parameter types to functions to keep things "matching".

If you don't like how I amended your code, please feel free to let me know and I
can try a different approach there :)

OWNERS:
rmcilroy: base/android
qinmin: content/browser/media, media/base/android
yfriedman: content/browser/android
nyquist: chrome/browser/invalidation, components/invalidation
sadrul: ui
mcasas: media/capture/video

[mcasas, 2016-08-11 14:58:30]

On 2016/08/11 11:26:57, Torne wrote:
> Hi, this is part of my ongoing effort to use JavaRef instead of bare objects.
> Since Java calls now accept JavaRef as well as bare objects, only a few calls
to
> GetApplicationContext are a problem (ones that store it in a variable) and
this
> CL addresses those. Where it looks "nice" I've just eliminated the variable;
in
> other cases types have been updated, and in a few places I also changed other
> parameter types to functions to keep things "matching".
> 
> If you don't like how I amended your code, please feel free to let me know and
I
> can try a different approach there :)
> 
> OWNERS:
> rmcilroy: base/android
> qinmin: content/browser/media, media/base/android
> yfriedman: content/browser/android
> nyquist: chrome/browser/invalidation, components/invalidation
> sadrul: ui
> mcasas: media/capture/video

media/capture/video lgtm

[Yaron, 2016-08-11 15:06:31]

content/browser/android lgtm

[rmcilroy, 2016-08-11 15:54:39]

lgtm

[nyquist, 2016-08-11 23:25:11]

chrome/browser/invalidation, components/invalidation lgtm

[sadrul, 2016-08-12 15:12:18]

Can you add a ui/android owner to review the other changes in ui/ too? I can
stamp

[Torne, 2016-08-12 15:38:05]

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

[Torne, 2016-08-12 15:38:05]

+twellington to review ui/ stuff as per sadrul@ comment

[qinmin, 2016-08-12 16:46:41]

lgtm

[Theresa, 2016-08-15 17:54:14]

lgtm % one comment

https://codereview.chromium.org/2231923002/diff/20001/ui/base/clipboard/clipb...
File ui/base/clipboard/clipboard_android_unittest.cc (left):

https://codereview.chromium.org/2231923002/diff/20001/ui/base/clipboard/clipb...
ui/base/clipboard/clipboard_android_unittest.cc:48: ASSERT_TRUE(context);
Why is this check no longer necessary?

[Torne, 2016-08-15 18:32:02]

It was never necessary; GetApplicationContext already asserts that the
context is set.

On Mon, 15 Aug 2016, 6:54 pm , <twellington@chromium.org> wrote:

> lgtm % one comment
>
>
>
>
https://codereview.chromium.org/2231923002/diff/20001/ui/base/clipboard/clipb...
> File ui/base/clipboard/clipboard_android_unittest.cc (left):
>
>
>
https://codereview.chromium.org/2231923002/diff/20001/ui/base/clipboard/clipb...
> ui/base/clipboard/clipboard_android_unittest.cc:48:
> ASSERT_TRUE(context);
> Why is this check no longer necessary?
>
> https://codereview.chromium.org/2231923002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Theresa, 2016-08-15 18:32:47]

On 2016/08/15 18:32:02, Torne wrote:
> It was never necessary; GetApplicationContext already asserts that the
> context is set.
> 
> On Mon, 15 Aug 2016, 6:54 pm , <mailto:twellington@chromium.org> wrote:
> 
> > lgtm % one comment
> >
> >
> >
> >
>
https://codereview.chromium.org/2231923002/diff/20001/ui/base/clipboard/clipb...
> > File ui/base/clipboard/clipboard_android_unittest.cc (left):
> >
> >
> >
>
https://codereview.chromium.org/2231923002/diff/20001/ui/base/clipboard/clipb...
> > ui/base/clipboard/clipboard_android_unittest.cc:48:
> > ASSERT_TRUE(context);
> > Why is this check no longer necessary?
> >
> > https://codereview.chromium.org/2231923002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

sg, thanks!

[sadrul, 2016-08-15 18:33:01]

rs lgtm

[Torne, 2016-08-16 11:09:03]

The CQ bit was checked by torne@chromium.org

[commit-bot: I haz the power, 2016-08-16 11:09:18]

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-16 12:21:26]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-16 12:22:57]

Description was changed from

==========
Make GetApplicationContext return a JavaRef.

Instead of returning a bare jobject, return a const reference to a
JavaRef, making it clearer that the caller does not receive ownership.
Most uses pass the result directly to a Java method; update the cases
which store it in a variable or pass it elsewhere.

Also, clean up the code surrounding the GetApplicationContext calls a
bit:
- There's no need to DCHECK this or AttachCurrentThread as the
  implementations already do that internally
- Change method parameters that take contexts as jobject to JavaRef, and
  also other parameters of those methods where reasonable.

BUG=506850
==========

to

==========
Make GetApplicationContext return a JavaRef.

Instead of returning a bare jobject, return a const reference to a
JavaRef, making it clearer that the caller does not receive ownership.
Most uses pass the result directly to a Java method; update the cases
which store it in a variable or pass it elsewhere.

Also, clean up the code surrounding the GetApplicationContext calls a
bit:
- There's no need to DCHECK this or AttachCurrentThread as the
  implementations already do that internally
- Change method parameters that take contexts as jobject to JavaRef, and
  also other parameters of those methods where reasonable.

BUG=506850

Committed: https://crrev.com/314ad6c309924d08e12a66a90c1b07558ad75c50
Cr-Commit-Position: refs/heads/master@{#412219}
==========

[commit-bot: I haz the power, 2016-08-16 12:22:59]

Patchset 2 (id:??) landed as
https://crrev.com/314ad6c309924d08e12a66a90c1b07558ad75c50
Cr-Commit-Position: refs/heads/master@{#412219}