Making BindingManager work on the launcher thread only.

Making BindingManager work on the launcher thread only.

BindingManager is now only accessed on the launcher thread and does not
require synchronization.
Also renamed BindingManager.determinedVisibility to onDeterminedVisibility.

BUG=715346
Review-Url: https://codereview.chromium.org/2843853002
Cr-Commit-Position: refs/heads/master@{#467434}
Committed: https://chromium.googlesource.com/chromium/src/+/2e1ea261b6194c8e14401f0a667927d4d32ac88f

[Jay Civelli, 2017-04-26 00:05:21]

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

[commit-bot: I haz the power, 2017-04-26 00:05:57]

Dry run: CQ is trying da patch.

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

[Jay Civelli, 2017-04-26 00:56:27]

Description was changed from

==========
Making BindingManager work on the Launcher thread only.

BindingManager now is only accessed on the launcher thread and does not
require synchronization.
Also renamed BindingManager.determinedVisibility to onDeterminedVisibility.

BUG=715346
==========

to

==========
Making BindingManager work on the Launcher thread only.

BindingManager is now only accessed on the launcher thread and does not
require synchronization.
Also renamed BindingManager.determinedVisibility to onDeterminedVisibility.

BUG=715346
==========

[Jay Civelli, 2017-04-26 00:56:27]

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

[Jay Civelli, 2017-04-26 01:01:52]

Description was changed from

==========
Making BindingManager work on the Launcher thread only.

BindingManager is now only accessed on the launcher thread and does not
require synchronization.
Also renamed BindingManager.determinedVisibility to onDeterminedVisibility.

BUG=715346
==========

to

==========
Making BindingManager work on the launcher thread only.

BindingManager is now only accessed on the launcher thread and does not
require synchronization.
Also renamed BindingManager.determinedVisibility to onDeterminedVisibility.

BUG=715346
==========

[commit-bot: I haz the power, 2017-04-26 01:07:06]

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

[commit-bot: I haz the power, 2017-04-26 01:07:07]

Dry run: This issue passed the CQ dry run.

[boliu, 2017-04-26 04:06:26]

https://codereview.chromium.org/2843853002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/BindingManagerImpl.java
(right):

https://codereview.chromium.org/2843853002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/BindingManagerImpl.java:54:
public void onTrimMemory(int level) {
these callbacks still happen on the UI thread afaik

https://codereview.chromium.org/2843853002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
(right):

https://codereview.chromium.org/2843853002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:226:
// This is called on the main thread.
assert these where possible instead of just commenting

[boliu, 2017-04-26 04:21:29]

https://codereview.chromium.org/2843853002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/BindingManagerImpl.java
(right):

https://codereview.chromium.org/2843853002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/BindingManagerImpl.java:435:
if (context != null) context.registerComponentCallbacks(mModerateBindingPool);
also, it's not totally clear to me if this register call is safe on a background
thread

[Jay Civelli, 2017-04-26 16:29:00]

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

[Jay Civelli, 2017-04-26 16:29:13]

https://codereview.chromium.org/2843853002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/BindingManagerImpl.java
(right):

https://codereview.chromium.org/2843853002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/BindingManagerImpl.java:54:
public void onTrimMemory(int level) {
On 2017/04/26 04:06:26, boliu wrote:
> these callbacks still happen on the UI thread afaik

Oh, right, fixed.

https://codereview.chromium.org/2843853002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/BindingManagerImpl.java:435:
if (context != null) context.registerComponentCallbacks(mModerateBindingPool);
On 2017/04/26 04:21:29, boliu wrote:
> also, it's not totally clear to me if this register call is safe on a
background
> thread

Looking at the source, it looks like it's thread-safe:
https://github.com/android/platform_frameworks_base/blob/4b1a8f46d6ec55796bf7...

https://codereview.chromium.org/2843853002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
(right):

https://codereview.chromium.org/2843853002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java:226:
// This is called on the main thread.
On 2017/04/26 04:06:26, boliu wrote:
> assert these where possible instead of just commenting

Done.

[commit-bot: I haz the power, 2017-04-26 16:29:25]

Dry run: CQ is trying da patch.

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

[boliu, 2017-04-26 16:31:37]

lgtm

https://codereview.chromium.org/2843853002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/BindingManagerImpl.java
(right):

https://codereview.chromium.org/2843853002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/BindingManagerImpl.java:435:
if (context != null) context.registerComponentCallbacks(mModerateBindingPool);
On 2017/04/26 16:29:12, Jay Civelli wrote:
> On 2017/04/26 04:21:29, boliu wrote:
> > also, it's not totally clear to me if this register call is safe on a
> background
> > thread
> 
> Looking at the source, it looks like it's thread-safe:
>
https://github.com/android/platform_frameworks_base/blob/4b1a8f46d6ec55796bf7...

Can you add a comment then?

[Jay Civelli, 2017-04-26 16:43:15]

jcivelli@chromium.org changed reviewers:
+ dtrainor@chromium.org

[Jay Civelli, 2017-04-26 16:43:16]

Adding dtrainor@ for OWNER review of super small change in:
chrome/android/javatests/src/org/chromium/chrome/browser/BindingManagerIntegrationTest.java

[Jay Civelli, 2017-04-26 16:55:19]

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

[commit-bot: I haz the power, 2017-04-26 16:55:48]

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-04-26 17:40:10]

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

[commit-bot: I haz the power, 2017-04-26 17:40:11]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2017-04-26 20:09:40]

lgtm

[Jay Civelli, 2017-04-26 20:10:06]

The CQ bit was checked by jcivelli@chromium.org

[Jay Civelli, 2017-04-26 20:10:06]

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/2843853002/#ps40001
(title: "Added a comment.")

[commit-bot: I haz the power, 2017-04-26 20:10:45]

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-04-26 20:19:41]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493237406225760,
"parent_rev": "8216b27dfdb3240900e60f2ccc67132eb5889507", "commit_rev":
"2e1ea261b6194c8e14401f0a667927d4d32ac88f"}

[commit-bot: I haz the power, 2017-04-26 20:19:48]

Description was changed from

==========
Making BindingManager work on the launcher thread only.

BindingManager is now only accessed on the launcher thread and does not
require synchronization.
Also renamed BindingManager.determinedVisibility to onDeterminedVisibility.

BUG=715346
==========

to

==========
Making BindingManager work on the launcher thread only.

BindingManager is now only accessed on the launcher thread and does not
require synchronization.
Also renamed BindingManager.determinedVisibility to onDeterminedVisibility.

BUG=715346

Review-Url: https://codereview.chromium.org/2843853002
Cr-Commit-Position: refs/heads/master@{#467434}
Committed:
https://chromium.googlesource.com/chromium/src/+/2e1ea261b6194c8e14401f0a6679...
==========

[commit-bot: I haz the power, 2017-04-26 20:19:49]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/2e1ea261b6194c8e14401f0a6679...