New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 614388 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

GSAServiceClient keeps GSA alive, effectively increasing Chrome's memory footprint.

Project Member Reported by lizeb@chromium.org, May 24 2016

Issue description

- ChromeActivity#onCreate() calls GSAServiceClient#connect().
- This connects to GSA "SsbService" using the default BIND_AUTO_CREATE.

This means that the binding makes GSA "visible".
On a Nexus 5 running L, this shows up as:

70534 kB: com.google.android.googlequicksearchbox:search (pid 1363)

Which basically mean that in the case where an application is sending an intent to Chrome, it will be killed earlier (since 70MB have been "stolen" from it).

Since "visible" is above "perceptible", "A Services", "previous" and "home", this means that this makes all these categories of applications killable before GSA.

The solution is to unbind from GSA, or to use BIND_WAIVE_PRIORITY.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c2ccc5b47944725186cce4af5f51111c25fbedde

commit c2ccc5b47944725186cce4af5f51111c25fbedde
Author: lizeb <lizeb@chromium.org>
Date: Wed Jun 22 08:49:44 2016

Android: Don't systematically move GSA to the foreground cgroup.

Chrome binds to a GSA service in onStartWithNative(). By default, this
moves GSA to the foreground cgroup, which is not what we want. Add the
required bind flag to prevent this.

See https://developer.android.com/reference/android/content/Context.html#BIND_NOT_FOREGROUND.

BUG= 614388 

Review-Url: https://codereview.chromium.org/2085993003
Cr-Commit-Position: refs/heads/master@{#401233}

[modify] https://crrev.com/c2ccc5b47944725186cce4af5f51111c25fbedde/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAServiceClient.java

Comment 3 by lizeb@chromium.org, Aug 1 2016

Cc: primiano@chromium.org picksi@chromium.org

Comment 4 by lizeb@chromium.org, Oct 24 2016

Status: Started (was: Assigned)
Internal bug ID: 32363735
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e1d19b388fc67f31372a4c2df33e20c0c18dfd97

commit e1d19b388fc67f31372a4c2df33e20c0c18dfd97
Author: lizeb <lizeb@chromium.org>
Date: Wed Oct 26 22:06:31 2016

android: Don't keep GSA alive if it supports account change broadcasts.

Chrome currently connects to a bound service exposed by GSA to get
account change notifications. This elevates GSA's priority for the
system to the foreground level when Chrome is in the foreground,
artificially increasing Chrome's memory footprint. On some phones, the
memory consumption of the GSA process can be above 100MB (PSS), even
though only the account change notifications are required.

Newer versions of GSA send a broadcast intent when the account
changes. This CL listens to the broadcasts, and disonnects from the GSA
service when GSA support the account change broadcast mechanism, freing
the memory for Chrome or other apps on the system.

Note that Chrome still briefly connects to GSA on startup to confirm
that it supports the broadcast. A forthcoming will remove this.

BUG= 614388 

Review-Url: https://codereview.chromium.org/2431223004
Cr-Commit-Position: refs/heads/master@{#427840}

[modify] https://crrev.com/e1d19b388fc67f31372a4c2df33e20c0c18dfd97/chrome/android/java/AndroidManifest.xml
[modify] https://crrev.com/e1d19b388fc67f31372a4c2df33e20c0c18dfd97/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[add] https://crrev.com/e1d19b388fc67f31372a4c2df33e20c0c18dfd97/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAAccountChangeListener.java
[modify] https://crrev.com/e1d19b388fc67f31372a4c2df33e20c0c18dfd97/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAServiceClient.java
[modify] https://crrev.com/e1d19b388fc67f31372a4c2df33e20c0c18dfd97/chrome/android/java_sources.gni

Comment 6 by picksi@chromium.org, Oct 27 2016

This is great! Thanks Benoit :)
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 2 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cf2cb0230ac1399b0225f081858d2c410c4629e1

commit cf2cb0230ac1399b0225f081858d2c410c4629e1
Author: lizeb <lizeb@chromium.org>
Date: Wed Nov 02 12:12:13 2016

android: Histogram for the GSA account change broadcast mechanism.

Over time, users should migrate to the broadcast-based notification
mechanism. Add a histogram to track that, and make sure that we don't
regress.

BUG= 614388 

Review-Url: https://codereview.chromium.org/2450353003
Cr-Commit-Position: refs/heads/master@{#429253}

[modify] https://crrev.com/cf2cb0230ac1399b0225f081858d2c410c4629e1/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAAccountChangeListener.java
[modify] https://crrev.com/cf2cb0230ac1399b0225f081858d2c410c4629e1/tools/metrics/histograms/histograms.xml

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0d37cc58e86c456e635fc75681768303b13ced8e

commit 0d37cc58e86c456e635fc75681768303b13ced8e
Author: lizeb <lizeb@chromium.org>
Date: Mon Nov 07 16:27:12 2016

android: Record the memory size of the GSA process.

Chrome connects to a bound service exposed by GSA, elevating its
importance to the system (see the linked bug). This requirement is going
away, and this CL is assessing the amount of memory wasted / saved this
way.

On a Nexus 5X, the cost of gathering this data has been
measured (through Trace events) to be ~60ms of wall clock time (on a
low-priority background thread), with ~2ms of CPU time on a little
core. The data is gathered at most once per startup.

BUG= 614388 

Review-Url: https://codereview.chromium.org/2477513004
Cr-Commit-Position: refs/heads/master@{#430288}

[modify] https://crrev.com/0d37cc58e86c456e635fc75681768303b13ced8e/base/android/java/src/org/chromium/base/metrics/RecordHistogram.java
[modify] https://crrev.com/0d37cc58e86c456e635fc75681768303b13ced8e/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAServiceClient.java
[modify] https://crrev.com/0d37cc58e86c456e635fc75681768303b13ced8e/chrome/android/java_sources.gni
[add] https://crrev.com/0d37cc58e86c456e635fc75681768303b13ced8e/chrome/android/javatests/src/org/chromium/chrome/browser/gsa/GSAServiceClientTest.java
[modify] https://crrev.com/0d37cc58e86c456e635fc75681768303b13ced8e/tools/metrics/histograms/histograms.xml

Comment 9 by lizeb@chromium.org, Nov 14 2016

Labels: Merge-Request-55
Requesting a merge of e1d19b388fc67f31372a4c2df33e20c0c18dfd97 to M55.
Reason: 100+MB memory savings (provided that GSA supports the new mechanism as well).

Comment 10 by lizeb@chromium.org, Nov 14 2016

Labels: -Pri-2 Pri-1

Comment 11 by dimu@chromium.org, Nov 14 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)
Project Member

Comment 12 by bugdroid1@chromium.org, Nov 14 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0d49cd8de23891fc2d4d4260271795c4c294bcd4

commit 0d49cd8de23891fc2d4d4260271795c4c294bcd4
Author: Benoit Lize <lizeb@chromium.org>
Date: Mon Nov 14 14:52:41 2016

android: Don't keep GSA alive if it supports account change broadcasts.

Chrome currently connects to a bound service exposed by GSA to get
account change notifications. This elevates GSA's priority for the
system to the foreground level when Chrome is in the foreground,
artificially increasing Chrome's memory footprint. On some phones, the
memory consumption of the GSA process can be above 100MB (PSS), even
though only the account change notifications are required.

Newer versions of GSA send a broadcast intent when the account
changes. This CL listens to the broadcasts, and disonnects from the GSA
service when GSA support the account change broadcast mechanism, freing
the memory for Chrome or other apps on the system.

Note that Chrome still briefly connects to GSA on startup to confirm
that it supports the broadcast. A forthcoming will remove this.

BUG= 614388 

Review-Url: https://codereview.chromium.org/2431223004
Cr-Commit-Position: refs/heads/master@{#427840}
(cherry picked from commit e1d19b388fc67f31372a4c2df33e20c0c18dfd97)

Review URL: https://codereview.chromium.org/2497313002 .

Cr-Commit-Position: refs/branch-heads/2883@{#558}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/0d49cd8de23891fc2d4d4260271795c4c294bcd4/chrome/android/java/AndroidManifest.xml
[modify] https://crrev.com/0d49cd8de23891fc2d4d4260271795c4c294bcd4/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[add] https://crrev.com/0d49cd8de23891fc2d4d4260271795c4c294bcd4/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAAccountChangeListener.java
[modify] https://crrev.com/0d49cd8de23891fc2d4d4260271795c4c294bcd4/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAServiceClient.java
[modify] https://crrev.com/0d49cd8de23891fc2d4d4260271795c4c294bcd4/chrome/android/java_sources.gni

Project Member

Comment 13 by bugdroid1@chromium.org, Nov 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/580de3daca57f3205f9e28496540f483b0612f7d

commit 580de3daca57f3205f9e28496540f483b0612f7d
Author: amineer <amineer@chromium.org>
Date: Tue Nov 15 20:37:16 2016

Revert of android: Don't keep GSA alive if it supports account change broadcasts. (patchset #1 id:1 of https://codereview.chromium.org/2497313002/ )

Reason for revert:
Causing crashes, see crbug/665396

Original issue's description:
> android: Don't keep GSA alive if it supports account change broadcasts.
>
> Chrome currently connects to a bound service exposed by GSA to get
> account change notifications. This elevates GSA's priority for the
> system to the foreground level when Chrome is in the foreground,
> artificially increasing Chrome's memory footprint. On some phones, the
> memory consumption of the GSA process can be above 100MB (PSS), even
> though only the account change notifications are required.
>
> Newer versions of GSA send a broadcast intent when the account
> changes. This CL listens to the broadcasts, and disonnects from the GSA
> service when GSA support the account change broadcast mechanism, freing
> the memory for Chrome or other apps on the system.
>
> Note that Chrome still briefly connects to GSA on startup to confirm
> that it supports the broadcast. A forthcoming will remove this.
>
> BUG= 614388 
>
> Review-Url: https://codereview.chromium.org/2431223004
> Cr-Commit-Position: refs/heads/master@{#427840}
> (cherry picked from commit e1d19b388fc67f31372a4c2df33e20c0c18dfd97)
>
> Committed: https://chromium.googlesource.com/chromium/src/+/0d49cd8de23891fc2d4d4260271795c4c294bcd4

TBR=lizeb@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 614388 
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2507523003
Cr-Commit-Position: refs/branch-heads/2883@{#576}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/580de3daca57f3205f9e28496540f483b0612f7d/chrome/android/java/AndroidManifest.xml
[modify] https://crrev.com/580de3daca57f3205f9e28496540f483b0612f7d/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[delete] https://crrev.com/29974b044c6376c6295091ef755f8f75b5eb6022/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAAccountChangeListener.java
[modify] https://crrev.com/580de3daca57f3205f9e28496540f483b0612f7d/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAServiceClient.java
[modify] https://crrev.com/580de3daca57f3205f9e28496540f483b0612f7d/chrome/android/java_sources.gni

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f8e7b55cbaf9147033f46e730a029347d3ae5baa

commit f8e7b55cbaf9147033f46e730a029347d3ae5baa
Author: lizeb <lizeb@chromium.org>
Date: Thu Jan 19 10:57:56 2017

android: Record the source of account change notifications from GSA.

GSA can notify Chrome of an account change through either a service
call, or through a broadcast. This histogram records the number of
account changes of each type that Chrome sees.

This is intended to act as a migration tracker and sanity check as we
migrate from the service to the broadcast mechanism. The total count
should stay approximately the same, while the proportion of each type
changes.

BUG= 614388 

Review-Url: https://codereview.chromium.org/2638653005
Cr-Commit-Position: refs/heads/master@{#444705}

[modify] https://crrev.com/f8e7b55cbaf9147033f46e730a029347d3ae5baa/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAAccountChangeListener.java
[modify] https://crrev.com/f8e7b55cbaf9147033f46e730a029347d3ae5baa/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAServiceClient.java
[add] https://crrev.com/f8e7b55cbaf9147033f46e730a029347d3ae5baa/chrome/android/java/src/org/chromium/chrome/browser/gsa/OWNERS
[modify] https://crrev.com/f8e7b55cbaf9147033f46e730a029347d3ae5baa/tools/metrics/histograms/histograms.xml

Project Member

Comment 15 by bugdroid1@chromium.org, Jan 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b34883d3328151ed5f81aa7ec8607b401d39d088

commit b34883d3328151ed5f81aa7ec8607b401d39d088
Author: lizeb <lizeb@chromium.org>
Date: Fri Jan 20 16:42:16 2017

android: Check GSA's broadcast permission.

GSA can either notify Chrome of account changes through a service, or
through a broadcast. The broadcast requires a permission to be granted
to Chrome in order to receive it. This commit adds a check in Chrome
that the permission is granted before switching to it.

BUG= 614388 

Review-Url: https://codereview.chromium.org/2639263002
Cr-Commit-Position: refs/heads/master@{#445075}

[modify] https://crrev.com/b34883d3328151ed5f81aa7ec8607b401d39d088/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAAccountChangeListener.java

Comment 16 by lizeb@chromium.org, Feb 17 2017

Cc: lizeb@chromium.org
 Issue 693145  has been merged into this issue.
Project Member

Comment 17 by bugdroid1@chromium.org, Feb 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/621821c9c2eeb41418191826528871b109ec5908

commit 621821c9c2eeb41418191826528871b109ec5908
Author: lizeb <lizeb@chromium.org>
Date: Tue Feb 21 12:37:03 2017

android: Tell GSA whether Chrome can listen to account change broadcasts.

For GSA to know that Chrome will receive the broadcasts, we need to tell
it that the matching permission is granted by the system.

BUG= 614388 

Review-Url: https://codereview.chromium.org/2701453002
Cr-Commit-Position: refs/heads/master@{#451740}

[modify] https://crrev.com/621821c9c2eeb41418191826528871b109ec5908/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAAccountChangeListener.java
[modify] https://crrev.com/621821c9c2eeb41418191826528871b109ec5908/chrome/android/java/src/org/chromium/chrome/browser/gsa/GSAServiceClient.java

Labels: sr-pm-7

Sign in to add a comment