Use ContainerView for PowerSaveBlocker to keep screen on

Use ContainerView for PowerSaveBlocker to keep screen on

PowerSaveBlocker used to add/remove a child view to the container
just to invoke |View.keepScreenOn| on it. This CL uses the container
view instead so as to avoid repeating view creation/deletion.

BUG=681738
Review-Url: https://codereview.chromium.org/2638973004
Cr-Commit-Position: refs/heads/master@{#444989}
Committed: https://chromium.googlesource.com/chromium/src/+/6795958b17b2aacf0ff1053d3df0948a3e4a1ad4

[Jinsuk Kim, 2017-01-18 03:19:53]

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

[commit-bot: I haz the power, 2017-01-18 03:20:10]

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, 2017-01-18 04:41:38]

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

[commit-bot: I haz the power, 2017-01-18 04:41:38]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Jinsuk Kim, 2017-01-18 07:21:55]

Patchset #2 (id:20001) has been deleted

[Jinsuk Kim, 2017-01-18 14:36:05]

Patchset #2 (id:40001) has been deleted

[Jinsuk Kim, 2017-01-18 15:10:47]

Patchset #2 (id:60001) has been deleted

[Jinsuk Kim, 2017-01-18 15:41:15]

jinsukkim@chromium.org changed reviewers:
+ boliu@chromium.org

[Jinsuk Kim, 2017-01-18 15:41:16]

Removed the weak reference to the view on which setKeepScreenOn is called in
PowerSaveBlocker. I don't think it's necessary any more since the view is not
held in native.

[boliu, 2017-01-18 18:21:16]

https://codereview.chromium.org/2638973004/diff/60002/device/power_save_block...
File
device/power_save_blocker/android/java/src/org/chromium/device/power_save_blocker/PowerSaveBlocker.java
(right):

https://codereview.chromium.org/2638973004/diff/60002/device/power_save_block...
device/power_save_blocker/android/java/src/org/chromium/device/power_save_blocker/PowerSaveBlocker.java:14:
private View mKeepScreenOnView;
the weakreference here is not because native side holds onto view, but because
native side holds onto PowerSaveBlocker

ie the weakreference here is still required

https://codereview.chromium.org/2638973004/diff/60002/ui/android/view_android.cc
File ui/android/view_android.cc (right):

https://codereview.chromium.org/2638973004/diff/60002/ui/android/view_android...
ui/android/view_android.cc:139: return ScopedJavaLocalRef<jobject>();
nullptr should work I think

https://codereview.chromium.org/2638973004/diff/60002/ui/android/view_android.h
File ui/android/view_android.h (right):

https://codereview.chromium.org/2638973004/diff/60002/ui/android/view_android...
ui/android/view_android.h:97: base::android::ScopedJavaLocalRef<jobject>
GetContainerView();
add a comment this may return null

[Jinsuk Kim, 2017-01-19 23:48:08]

https://codereview.chromium.org/2638973004/diff/60002/device/power_save_block...
File
device/power_save_blocker/android/java/src/org/chromium/device/power_save_blocker/PowerSaveBlocker.java
(right):

https://codereview.chromium.org/2638973004/diff/60002/device/power_save_block...
device/power_save_blocker/android/java/src/org/chromium/device/power_save_blocker/PowerSaveBlocker.java:14:
private View mKeepScreenOnView;
On 2017/01/18 18:21:16, boliu wrote:
> the weakreference here is not because native side holds onto view, but because
> native side holds onto PowerSaveBlocker
> 
> ie the weakreference here is still required

Done.

https://codereview.chromium.org/2638973004/diff/60002/ui/android/view_android.cc
File ui/android/view_android.cc (right):

https://codereview.chromium.org/2638973004/diff/60002/ui/android/view_android...
ui/android/view_android.cc:139: return ScopedJavaLocalRef<jobject>();
On 2017/01/18 18:21:16, boliu wrote:
> nullptr should work I think

Done.

https://codereview.chromium.org/2638973004/diff/60002/ui/android/view_android.h
File ui/android/view_android.h (right):

https://codereview.chromium.org/2638973004/diff/60002/ui/android/view_android...
ui/android/view_android.h:97: base::android::ScopedJavaLocalRef<jobject>
GetContainerView();
On 2017/01/18 18:21:16, boliu wrote:
> add a comment this may return null

Done.

[boliu, 2017-01-19 23:51:06]

lgtm

[Jinsuk Kim, 2017-01-20 00:08:48]

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

[Jinsuk Kim, 2017-01-20 00:08:48]

twellington@chromium.org: Please review changes in ui/android

[Theresa, 2017-01-20 00:09:54]

rubber stamp lgtm

boliu@ - are you ready to be a ui/android OWNER for specific files?

[Jinsuk Kim, 2017-01-20 00:15:54]

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

[commit-bot: I haz the power, 2017-01-20 00:17:23]

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

[boliu, 2017-01-20 01:19:11]

On 2017/01/20 00:09:54, Theresa wrote:
> rubber stamp lgtm
> 
> boliu@ - are you ready to be a ui/android OWNER for specific files?

yeah.. display/window/view_android related things are fine

[commit-bot: I haz the power, 2017-01-20 01:50:26]

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

[commit-bot: I haz the power, 2017-01-20 01:50:27]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Jinsuk Kim, 2017-01-20 03:10:58]

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

[commit-bot: I haz the power, 2017-01-20 03:11:21]

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, 2017-01-20 03:47:55]

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

[commit-bot: I haz the power, 2017-01-20 03:47:56]

Dry run: This issue passed the CQ dry run.

[Jinsuk Kim, 2017-01-20 05:50:19]

The CQ bit was checked by jinsukkim@chromium.org

[Jinsuk Kim, 2017-01-20 05:50:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
twellington@chromium.org, boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/2638973004/#ps110001
(title: "fix tests/2")

[commit-bot: I haz the power, 2017-01-20 05:50:32]

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, 2017-01-20 05:55:08]

CQ is committing da patch.

Bot data: {"patchset_id": 110001, "attempt_start_ts": 1484891419875060,
"parent_rev": "ff9ccb9534870244065db87766a709a23647ff6f", "commit_rev":
"6795958b17b2aacf0ff1053d3df0948a3e4a1ad4"}

[commit-bot: I haz the power, 2017-01-20 05:56:08]

Description was changed from

==========
Use ContainerView for PowerSaveBlocker to keep screen on

PowerSaveBlocker used to add/remove a child view to the container
just to invoke |View.keepScreenOn| on it. This CL uses the container
view instead so as to avoid repeating view creation/deletion.

BUG=681738
==========

to

==========
Use ContainerView for PowerSaveBlocker to keep screen on

PowerSaveBlocker used to add/remove a child view to the container
just to invoke |View.keepScreenOn| on it. This CL uses the container
view instead so as to avoid repeating view creation/deletion.

BUG=681738

Review-Url: https://codereview.chromium.org/2638973004
Cr-Commit-Position: refs/heads/master@{#444989}
Committed:
https://chromium.googlesource.com/chromium/src/+/6795958b17b2aacf0ff1053d3df0...
==========

[commit-bot: I haz the power, 2017-01-20 05:56:09]

Committed patchset #4 (id:110001) as
https://chromium.googlesource.com/chromium/src/+/6795958b17b2aacf0ff1053d3df0...