Let PowerSaveBlocker manager anchor view in Java side

Let PowerSaveBlocker manager anchor view in Java side

This reduces the number of native-to-Java calls by one
with a view to mitigating the benchmark test regression
introduced with https://crrev.com/2103243002.
PowerSaveBlocker was the only class that created an anchor
view during the test. Though the root cause not being
entirely clear yet, having the view created on Java side
can possibly help decrease the browser thread cpu consumption.

BUG=635515
TBR=tedchoc@chromium.org

Committed: https://crrev.com/bc403d8872340a62c310c9665e3b3972344c8c5b
Cr-Commit-Position: refs/heads/master@{#413900}

[Jinsuk Kim, 2016-08-19 11:29:14]

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

[Jinsuk Kim, 2016-08-19 11:29:14]

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

boliu@chromium.org: Please review changes in device/power_save_blocker

[boliu, 2016-08-19 18:49:58]

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

https://codereview.chromium.org/2259493005/diff/1/ui/android/view_android.h#n...
ui/android/view_android.h:92: const base::android::ScopedJavaLocalRef<jobject>
err, maybe return GlobalRef if this is public?

also this is held weakly from native side so two problems, and you are
completely hiding that fact, two problems:
caller will probably forget to null check
caller may hold the strong ref natively, which is bad

so maybe actually just return JavaObjectWeakGlobalRef instead?

[Jinsuk Kim, 2016-08-19 20:44:46]

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

https://codereview.chromium.org/2259493005/diff/1/ui/android/view_android.h#n...
ui/android/view_android.h:92: const base::android::ScopedJavaLocalRef<jobject>
On 2016/08/19 18:49:58, boliu wrote:
> err, maybe return GlobalRef if this is public?
> 
> also this is held weakly from native side so two problems, and you are
> completely hiding that fact, two problems:
> caller will probably forget to null check
> caller may hold the strong ref natively, which is bad
> 
> so maybe actually just return JavaObjectWeakGlobalRef instead?

Done.

[boliu, 2016-08-19 20:47:44]

lgtm

[Jinsuk Kim, 2016-08-19 21:01:21]

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

[Jinsuk Kim, 2016-08-19 21:01:21]

tedchoc@ Please review changes in ui/android.

[Jinsuk Kim, 2016-08-21 22:20:41]

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

[commit-bot: I haz the power, 2016-08-21 22:20:49]

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-21 22:22:19]

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

[commit-bot: I haz the power, 2016-08-21 22:22:20]

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

[Jinsuk Kim, 2016-08-21 23:36:19]

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

[commit-bot: I haz the power, 2016-08-21 23:36:33]

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-22 00:23:05]

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

[commit-bot: I haz the power, 2016-08-22 00:23:06]

Dry run: This issue passed the CQ dry run.

[Jinsuk Kim, 2016-08-23 01:16:03]

On 2016/08/19 21:01:21, Jinsuk wrote:
> tedchoc@ Please review changes in ui/android.

ping?

[Jinsuk Kim, 2016-08-23 21:35:59]

Description was changed from

==========
Let PowerSaveBlocker manager anchor view in Java side

This reduces the number of native-to-Java calls by one
with a view to mitigating the benchmark test regression
introduced with https://crrev.com/2103243002.
PowerSaveBlocker was the only class that created an anchor
view during the test. Though the root cause not being
entirely clear yet, having the view created on Java side
can possibly help decrease the browser thread cpu consumption.

BUG=635515
==========

to

==========
Let PowerSaveBlocker manager anchor view in Java side

This reduces the number of native-to-Java calls by one
with a view to mitigating the benchmark test regression
introduced with https://crrev.com/2103243002.
PowerSaveBlocker was the only class that created an anchor
view during the test. Though the root cause not being
entirely clear yet, having the view created on Java side
can possibly help decrease the browser thread cpu consumption.

BUG=635515
TBR=tedchoc@chromium.org
==========

[Jinsuk Kim, 2016-08-23 21:36:20]

The CQ bit was checked by jinsukkim@chromium.org

[Jinsuk Kim, 2016-08-23 21:36:21]

The patchset sent to the CQ was uploaded after l-g-t-m from boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/2259493005/#ps40001
(title: "rebased")

[commit-bot: I haz the power, 2016-08-23 21:37:33]

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

[Ted C, 2016-08-23 23:36:46]

On 2016/08/23 21:37:33, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

lgtm

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

Description was changed from

==========
Let PowerSaveBlocker manager anchor view in Java side

This reduces the number of native-to-Java calls by one
with a view to mitigating the benchmark test regression
introduced with https://crrev.com/2103243002.
PowerSaveBlocker was the only class that created an anchor
view during the test. Though the root cause not being
entirely clear yet, having the view created on Java side
can possibly help decrease the browser thread cpu consumption.

BUG=635515
TBR=tedchoc@chromium.org
==========

to

==========
Let PowerSaveBlocker manager anchor view in Java side

This reduces the number of native-to-Java calls by one
with a view to mitigating the benchmark test regression
introduced with https://crrev.com/2103243002.
PowerSaveBlocker was the only class that created an anchor
view during the test. Though the root cause not being
entirely clear yet, having the view created on Java side
can possibly help decrease the browser thread cpu consumption.

BUG=635515
TBR=tedchoc@chromium.org
==========

[commit-bot: I haz the power, 2016-08-23 23:46:25]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-23 23:48:38]

Description was changed from

==========
Let PowerSaveBlocker manager anchor view in Java side

This reduces the number of native-to-Java calls by one
with a view to mitigating the benchmark test regression
introduced with https://crrev.com/2103243002.
PowerSaveBlocker was the only class that created an anchor
view during the test. Though the root cause not being
entirely clear yet, having the view created on Java side
can possibly help decrease the browser thread cpu consumption.

BUG=635515
TBR=tedchoc@chromium.org
==========

to

==========
Let PowerSaveBlocker manager anchor view in Java side

This reduces the number of native-to-Java calls by one
with a view to mitigating the benchmark test regression
introduced with https://crrev.com/2103243002.
PowerSaveBlocker was the only class that created an anchor
view during the test. Though the root cause not being
entirely clear yet, having the view created on Java side
can possibly help decrease the browser thread cpu consumption.

BUG=635515
TBR=tedchoc@chromium.org

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

[commit-bot: I haz the power, 2016-08-23 23:48:39]

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

[no sievers, 2016-08-26 19:34:45]

Can we revert this now, since it looks like it caused no improvement in the
measurement?

[no sievers, 2016-08-26 20:42:34]

On 2016/08/26 19:34:45, sievers wrote:
> Can we revert this now, since it looks like it caused no improvement in the
> measurement?

I think I found the actual culprit. Reverting this so I can rebase the fix on
top of the revert.

[no sievers, 2016-08-26 20:42:52]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2279313005/ by sievers@chromium.org.

The reason for reverting is: not needed. doesn't fix the regression..