Disable hardware acceleration for ChromeActivities on low-end devices.

Disable hardware acceleration for ChromeActivities on low-end devices.

Disabling HW acceleration saves us several MiB of GL/EGL memory (4-10 as
reported by meminfo depending on the device). However, we do that only for
activities derived from ChromeActivity because they have simple UI, so side
effects of disabled HW acceleration (janky ripple animation, no shadows, etc)
are not noticeable.

Also, since we cannot really disable HW acceleration at runtime (only enable),
we disable it in the manifest and enable for high-end devices.

BUG=482727
Committed: https://crrev.com/58112bab464cbcbc4c82efb3ac2193d49f3131e8
Cr-Commit-Position: refs/heads/master@{#341393}

[Dmitry Skiba, 2015-07-15 21:59:15]

dskiba@google.com changed reviewers:
+ aelias@chromium.org, mariakhomenko@chromium.org, newt@chromium.org

[newt (away), 2015-07-16 19:46:16]

I'll defer to Maria and Alex for the initial review

https://codereview.chromium.org/1223263009/diff/1/chrome/android/java/Android...
File chrome/android/java/AndroidManifest.xml (right):

https://codereview.chromium.org/1223263009/diff/1/chrome/android/java/Android...
chrome/android/java/AndroidManifest.xml:111: <!-- Note: Set
android:hardwareAccelerated to "false" in the ".Main"
This comment in thoroughly out of date. Please remove it while you're here.

[aelias_OOO_until_Jul13, 2015-07-17 02:48:25]

Before we land anything, I think this needs further discussion around:

A) the likely jankiness for the ListView-based tab switcher -- this may call for
creating a simple-looking CC-based tab switcher mode that looks like a ListView
B) how do we recover the memory after something causes hardware acceleration to
be initialized (probably by firing a low memory signal at appropriate times)
C) how do we make sure this doesn't regress in the future as new kinds of Views
are added.

Let's chat offline about it tomorrow.

https://codereview.chromium.org/1223263009/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
(right):

https://codereview.chromium.org/1223263009/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java:1409:
private void checkHardwareAcceleration() {
We generally don't add code that intentionally crashes the browser, that can
backfire in the wild.  We only do so with DCHECK()s that only crash debug
builds.

[DmitrySkiba, 2015-07-17 16:55:13]

dskiba@chromium.org changed reviewers:
+ dskiba@chromium.org

[DmitrySkiba, 2015-07-17 16:55:14]

https://codereview.chromium.org/1223263009/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
(right):

https://codereview.chromium.org/1223263009/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java:1409:
private void checkHardwareAcceleration() {
On 2015/07/17 02:48:25, aelias wrote:
> We generally don't add code that intentionally crashes the browser, that can
> backfire in the wild.  We only do so with DCHECK()s that only crash debug
> builds.

This is rough DCHECK alternative for the Java - I check
ApplicationInfo.FLAG_DEBUGGABLE before doing further checks. As I understand
without crbug.com/462676 this closest as we can get.

[Maria, 2015-07-24 20:59:38]

lgtm

I played with the change for a bit and it looks good to me. Tab switcher seems
fine and given than > 90% of users have < 10 tabs, there doesn't seem to be much
issue.

I think we should do what we can for reclaiming memory from RenderThread, but I
wonder if we can estimate % session in which such a thread would get created
from the stats we have. Looks like ~4.5% of users go to settings in a 7 day
period. That doesn't seem too bad. I wonder if we could somehow monitor how
often RenderThread ends up getting created in practice.

In terms of future-proofing, do we have any QA coverage for low end devices
generally? This will probably require a human to look to figure out if a new
view is looking janky.

https://codereview.chromium.org/1223263009/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/hardware_acceleration/ChromeTabbedActivityHWATest.java
(right):

https://codereview.chromium.org/1223263009/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/hardware_acceleration/ChromeTabbedActivityHWATest.java:15:
* Tests that ChromeTabbedActivity is hardware accelerated only high-end devices.
only -> only on

https://codereview.chromium.org/1223263009/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/hardware_acceleration/ChromeTabbedActivityHWATest.java:18:
public class ChromeTabbedActivityHWATest extends DocumentModeTestBase {
This is weird, ChromeTabbedActivity test should probably not be deriving from
DocumentModeTestBase. Generally those derive from
ChromeTabbedActivityTestBase... is there a reason for doing it this way?

[Dmitry Skiba, 2015-07-27 16:03:02]

Alexandre, I believe I answered your concerns in crbug.com/482727 - is there
anything I need to do to get LGTM?

[aelias_OOO_until_Jul13, 2015-07-28 00:49:21]

As mentioned offline, I think this can regress very easily because any random
thing like a toast can cause the RenderThread to be created, so I don't think
the test coverage is sufficient yet.  I'd like a test that starts Chrome, loads
a page and then explicitly verifies that the RenderThread doesn't exist.

[Dmitry Skiba, 2015-07-28 21:23:53]

Alexandre, please check NoRenderThread tests I added to DocumentActivityHWATest,
IncognitoDocumentActivityHWATest and ChromeTabbedActivityHWATest.

Actually, that was very useful as I found an issue with the incognito activity -
CipherKeyActivity which we start before it, was hardware accelerated! So
incognito activity was ending up with RenderThread even though it has hardware
acceleration turned off.

[aelias_OOO_until_Jul13, 2015-07-28 23:04:31]

Thanks for the tests.  A few more things, I agree with Maria's comment here:

> I wonder if we could somehow monitor how often RenderThread ends up getting
created in practice.

How about adding a UMA stat collection at page load complete time that checks if
the RenderThread exists?  (We should also collect whether a device is "low-end"
at all at the same place, to have a comparison baseline.)  That will give us a
sense for how effective it is in practice.

Also, here is a list of UI features using Android Views that come to mind: URL
suggestions, permissions butterbars, contextual search, form autofill and
pull-to-refresh.  Could you test each of those things and verify it's in
software mode?  And if one of them turns out not to be and you change it, please
add a test specifically for that thing.

[Dmitry Skiba, 2015-07-29 22:28:57]

> How about adding a UMA stat collection at page load complete time that checks
if
> the RenderThread exists?  (We should also collect whether a device is
"low-end"
> at all at the same place, to have a comparison baseline.)  That will give us a
> sense for how effective it is in practice.

We talked about UMA a bit, there are some ideas, but we need to think more. I
created an issue: 515279


> Also, here is a list of UI features using Android Views that come to mind: URL
> suggestions, permissions butterbars, contextual search, form autofill and
> pull-to-refresh.  Could you test each of those things and verify it's in
> software mode?  And if one of them turns out not to be and you change it,
please
> add a test specifically for that thing.

I checked everything above with the exception of the contextual search (disabled
on Svelte), and didn't see RenderThread. Even autofill popup, which is actually
a separate window (AutofillPopup) doesn't trigger RenderThread (unlike Toasts,
see 514328).

[Dmitry Skiba, 2015-07-31 19:46:45]

The CQ bit was checked by dskiba@google.com

[Dmitry Skiba, 2015-07-31 19:46:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
mariakhomenko@chromium.org
Link to the patchset: https://codereview.chromium.org/1223263009/#ps80001
(title: "Fixed DocumentModeTestBase to compile")

[commit-bot: I haz the power, 2015-07-31 19:47:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1223263009/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1223263009/80001

[commit-bot: I haz the power, 2015-07-31 19:57:14]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-07-31 19:57:51]

Patchset 5 (id:??) landed as
https://crrev.com/58112bab464cbcbc4c82efb3ac2193d49f3131e8
Cr-Commit-Position: refs/heads/master@{#341393}