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

Issue 766248 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

CronetEngine.Builder.setLibraryLoader not safe with different ClassLoaders

Project Member Reported by pauljensen@chromium.org, Sep 18 2017

Issue description

LibraryLoaders may not use the same ClassLoader as the NativeCronetEngineBuilderImpl (and related Java classes in cronet_impl_native_java.jar).  This can cause failures to load the native libraries because the native library search path is contained in the ClassLoader.  We should fix this by offering a different CronetEngine.Builder implementation that ignores LibraryLoaders for use in cases where ClassLoaders may differ.

Internal bug b/65757004
 
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 19 2017

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

commit a9edc7a85bc9643a99610051b526d608f2acbec6
Author: kapishnikov <kapishnikov@chromium.org>
Date: Tue Sep 19 01:17:32 2017

CronetEngine.Builder.setLibraryLoader not safe with different ClassLoaders

Do not enforce ICronetEngineBuilder.setLibraryLoader() in Java and
Native builder implementations by default. Instead introduce a new
NativeCronetEngineBuilderWithLibraryLoaderImpl class that enforces
the library loader.

BUG= 766248 

Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: Ie78383084cd68b31f4a87463763891bd30368d21
Reviewed-on: https://chromium-review.googlesource.com/671416
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502749}
[modify] https://crrev.com/a9edc7a85bc9643a99610051b526d608f2acbec6/components/cronet/android/BUILD.gn
[modify] https://crrev.com/a9edc7a85bc9643a99610051b526d608f2acbec6/components/cronet/android/java/src/org/chromium/net/impl/CronetEngineBuilderImpl.java
[add] https://crrev.com/a9edc7a85bc9643a99610051b526d608f2acbec6/components/cronet/android/java/src/org/chromium/net/impl/NativeCronetEngineBuilderWithLibraryLoaderImpl.java
[modify] https://crrev.com/a9edc7a85bc9643a99610051b526d608f2acbec6/components/cronet/android/java/src/org/chromium/net/impl/NativeCronetProvider.java
[modify] https://crrev.com/a9edc7a85bc9643a99610051b526d608f2acbec6/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java

Status: Fixed (was: Started)
The change should be ported to M61 & M62.
Labels: Merge-Request-62
This merge cannot affect Chrome as it's in components/cronet/ only.
Project Member

Comment 7 by sheriffbot@chromium.org, Sep 19 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: M62 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: amineer@chromium.org
Labels: Merge-Request-61
Requesting the approval to merge to M61. The fix only contains changes in "components/cronet" code base. It doesn't affect Chromium since none of Chromium code has dependencies on the Cronet component.
Labels: -Merge-Request-61 -Merge-Review-62 Merge-Approved-61 Merge-Approved-62
Approved for M61 branch 3163 and M62 branch 3202.
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 20 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7f9912d02d7396b7b62805a3cfef7099c215c09f

commit 7f9912d02d7396b7b62805a3cfef7099c215c09f
Author: kapishnikov <kapishnikov@chromium.org>
Date: Wed Sep 20 21:06:01 2017

[Merge M61] CronetEngine.Builder.setLibraryLoader not safe

Do not enforce ICronetEngineBuilder.setLibraryLoader() in Java and
Native builder implementations by default. Instead introduce a new
NativeCronetEngineBuilderWithLibraryLoaderImpl class that enforces
the library loader.

BUG= 766248 

(cherry picked from commit a9edc7a85bc9643a99610051b526d608f2acbec6)

Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: Ie78383084cd68b31f4a87463763891bd30368d21
Reviewed-on: https://chromium-review.googlesource.com/671416
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#502749}
Reviewed-on: https://chromium-review.googlesource.com/675712
Cr-Commit-Position: refs/branch-heads/3163@{#1249}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/7f9912d02d7396b7b62805a3cfef7099c215c09f/components/cronet/android/BUILD.gn
[modify] https://crrev.com/7f9912d02d7396b7b62805a3cfef7099c215c09f/components/cronet/android/java/src/org/chromium/net/impl/CronetEngineBuilderImpl.java
[add] https://crrev.com/7f9912d02d7396b7b62805a3cfef7099c215c09f/components/cronet/android/java/src/org/chromium/net/impl/NativeCronetEngineBuilderWithLibraryLoaderImpl.java
[modify] https://crrev.com/7f9912d02d7396b7b62805a3cfef7099c215c09f/components/cronet/android/java/src/org/chromium/net/impl/NativeCronetProvider.java
[modify] https://crrev.com/7f9912d02d7396b7b62805a3cfef7099c215c09f/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java

Project Member

Comment 11 by bugdroid1@chromium.org, Sep 20 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f92b8c5061a9d9be6a77be73a7d1cde91520c440

commit f92b8c5061a9d9be6a77be73a7d1cde91520c440
Author: kapishnikov <kapishnikov@chromium.org>
Date: Wed Sep 20 21:40:06 2017

[Merge M62] CronetEngine.Builder.setLibraryLoader not safe

Do not enforce ICronetEngineBuilder.setLibraryLoader() in Java and
Native builder implementations by default. Instead introduce a new
NativeCronetEngineBuilderWithLibraryLoaderImpl class that enforces
the library loader.

BUG= 766248 

(cherry picked from commit a9edc7a85bc9643a99610051b526d608f2acbec6)

Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: Ie78383084cd68b31f4a87463763891bd30368d21
Reviewed-on: https://chromium-review.googlesource.com/671416
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#502749}
Reviewed-on: https://chromium-review.googlesource.com/676168
Cr-Commit-Position: refs/branch-heads/3202@{#363}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/f92b8c5061a9d9be6a77be73a7d1cde91520c440/components/cronet/android/BUILD.gn
[modify] https://crrev.com/f92b8c5061a9d9be6a77be73a7d1cde91520c440/components/cronet/android/java/src/org/chromium/net/impl/CronetEngineBuilderImpl.java
[add] https://crrev.com/f92b8c5061a9d9be6a77be73a7d1cde91520c440/components/cronet/android/java/src/org/chromium/net/impl/NativeCronetEngineBuilderWithLibraryLoaderImpl.java
[modify] https://crrev.com/f92b8c5061a9d9be6a77be73a7d1cde91520c440/components/cronet/android/java/src/org/chromium/net/impl/NativeCronetProvider.java
[modify] https://crrev.com/f92b8c5061a9d9be6a77be73a7d1cde91520c440/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java

Sign in to add a comment