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

Issue 732591 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 2017
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

ChildProcessServiceImpl lock refactor

Project Member Reported by jcivelli@chromium.org, Jun 12 2017

Issue description

ChildProcessServiceImpl.java uses one lock to synchronize various unrelated variables and synchronize when not needed in some cases.
It should be cleaned-up.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 13 2017

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

commit 1085bed87de2bbd43fbf5b3520634ca045b764e6
Author: Jay Civelli <jcivelli@google.com>
Date: Tue Jun 13 15:48:11 2017

ChildProcessServiceImpl lock refactor.

Using GuardedBy for the mBinderLock's protected members in
ChildProcessServiceImpl.

Bug:  732591 
Change-Id: I82185c920f29cfc1c92f42d7e63f341bf1681107
Reviewed-on: https://chromium-review.googlesource.com/532253
Reviewed-by: Bo Liu <boliu@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479032}
[modify] https://crrev.com/1085bed87de2bbd43fbf5b3520634ca045b764e6/content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java

Project Member

Comment 2 by bugdroid1@chromium.org, Jun 13 2017

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

commit a8874ee752dc6a9e8250640952bfb3a8055c2764
Author: Jay Civelli <jcivelli@google.com>
Date: Tue Jun 13 19:45:27 2017

ChildProcessServiceImpl: using a specific lock for library init var.

Using a specific lock for the library initialized member variable in
ChildProcessServiceImpl instead of using the main thread member.

Bug:  732591 
Change-Id: I2973f250aaa1d03eebea75810a06ef4f6925537a
Reviewed-on: https://chromium-review.googlesource.com/532313
Reviewed-by: Bo Liu <boliu@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479104}
[modify] https://crrev.com/a8874ee752dc6a9e8250640952bfb3a8055c2764/content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 13 2017

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

commit 79acd3169c325a4fb28f848c2a0cdd94eae62947
Author: Jay Civelli <jcivelli@google.com>
Date: Tue Jun 13 21:10:00 2017

ChildProcessServiceImpl: removing unnecessary synchronizations.

The Android framework only calls onBind() once for a bound service. As a
result we can remove synchronizations for member variables that are set
in the bind() method and then accessed for the returned binder (since
we are guaranteed these variables cannot be used before they are set).

Bug:  732591 
Change-Id: Ib04955e3f45721c3a01e82332d99f1950c14b5ae
Reviewed-on: https://chromium-review.googlesource.com/532234
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479150}
[modify] https://crrev.com/79acd3169c325a4fb28f848c2a0cdd94eae62947/content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 13 2017

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

commit 0bb515d00576c78a0b74b6154f665f1ed509fee5
Author: Jay Civelli <jcivelli@google.com>
Date: Tue Jun 13 23:43:23 2017

ChildProcessServiceImpl: minor clean-up.

Renaming getServiceInfo to processConnectionBundle and using a local var
to simplify code.

Bug:  732591 
Change-Id: I89f2e767832f1f0c34327c45abe20de550216310
Reviewed-on: https://chromium-review.googlesource.com/532255
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479207}
[modify] https://crrev.com/0bb515d00576c78a0b74b6154f665f1ed509fee5/content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java

Status: Fixed (was: Untriaged)

Sign in to add a comment