Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(19)

Issue 2406083002: Crazylinker: remove the code to load component build library (Closed)

Created:
3 years, 10 months ago by michaelbai
Modified:
3 years, 9 months ago
Reviewers:
Torne
CC:
chromium-reviews, agrieve+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Crazylinker: remove the code to load component build library The code is broken, component build library will not be loaded if non-component build library is in system lib path, e.g. Chrome is preinstalled. BUG=657093 Committed: https://crrev.com/275c325caaec3cc65f0c3be9af0abd00963e9d01 Cr-Commit-Position: refs/heads/master@{#427809}

Patch Set 1 #

Total comments: 1

Patch Set 2 : remove the code #

Total comments: 2

Patch Set 3 : remove one more component code #

Total comments: 2

Patch Set 4 : change comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -13 lines) Patch
M base/android/java/src/org/chromium/base/library_loader/Linker.java View 1 2 1 chunk +3 lines, -11 lines 0 comments Download
M base/android/linker/config.gni View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 23 (8 generated)
michaelbai
3 years, 10 months ago (2016-10-11 00:56:48 UTC) #3
Torne
I don't understand what you mean here; can you try to explain differently?
3 years, 10 months ago (2016-10-11 12:03:45 UTC) #5
michaelbai
https://codereview.chromium.org/2406083002/diff/1/base/android/java/src/org/chromium/base/library_loader/Linker.java File base/android/java/src/org/chromium/base/library_loader/Linker.java (right): https://codereview.chromium.org/2406083002/diff/1/base/android/java/src/org/chromium/base/library_loader/Linker.java#newcode534 base/android/java/src/org/chromium/base/library_loader/Linker.java:534: System.loadLibrary(LINKER_JNI_LIBRARY + ".cr"); If you don't think crazy linker ...
3 years, 10 months ago (2016-10-11 18:26:37 UTC) #6
Torne
I don't get what the issue is here at all, so I'm not sure what ...
3 years, 10 months ago (2016-10-12 12:10:55 UTC) #7
michaelbai
On 2016/10/12 12:10:55, Torne wrote: > I don't get what the issue is here at ...
3 years, 10 months ago (2016-10-12 16:19:30 UTC) #8
Torne
https://codereview.chromium.org/2406083002/diff/20001/base/android/java/src/org/chromium/base/library_loader/Linker.java File base/android/java/src/org/chromium/base/library_loader/Linker.java (right): https://codereview.chromium.org/2406083002/diff/20001/base/android/java/src/org/chromium/base/library_loader/Linker.java#newcode513 base/android/java/src/org/chromium/base/library_loader/Linker.java:513: return library.equals(LINKER_JNI_LIBRARY) || library.equals(LINKER_JNI_LIBRARY + ".cr"); Seems like we ...
3 years, 9 months ago (2016-10-24 12:07:03 UTC) #10
michaelbai
PTAL https://codereview.chromium.org/2406083002/diff/20001/base/android/java/src/org/chromium/base/library_loader/Linker.java File base/android/java/src/org/chromium/base/library_loader/Linker.java (right): https://codereview.chromium.org/2406083002/diff/20001/base/android/java/src/org/chromium/base/library_loader/Linker.java#newcode513 base/android/java/src/org/chromium/base/library_loader/Linker.java:513: return library.equals(LINKER_JNI_LIBRARY) || library.equals(LINKER_JNI_LIBRARY + ".cr"); On 2016/10/24 ...
3 years, 9 months ago (2016-10-25 22:21:54 UTC) #11
Torne
LGTM with a comment nit https://codereview.chromium.org/2406083002/diff/40001/base/android/linker/config.gni File base/android/linker/config.gni (right): https://codereview.chromium.org/2406083002/diff/40001/base/android/linker/config.gni#newcode12 base/android/linker/config.gni:12: # and needs to ...
3 years, 9 months ago (2016-10-26 10:27:44 UTC) #12
michaelbai
https://codereview.chromium.org/2406083002/diff/40001/base/android/linker/config.gni File base/android/linker/config.gni (right): https://codereview.chromium.org/2406083002/diff/40001/base/android/linker/config.gni#newcode12 base/android/linker/config.gni:12: # and needs to cleanup. See crbug.com/657093. On 2016/10/26 ...
3 years, 9 months ago (2016-10-26 20:35:51 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2406083002/60001
3 years, 9 months ago (2016-10-26 20:36:49 UTC) #16
commit-bot: I haz the power
Committed patchset #4 (id:60001)
3 years, 9 months ago (2016-10-26 21:30:33 UTC) #18
commit-bot: I haz the power
Patchset 4 (id:??) landed as https://crrev.com/275c325caaec3cc65f0c3be9af0abd00963e9d01 Cr-Commit-Position: refs/heads/master@{#427809}
3 years, 9 months ago (2016-10-26 21:33:56 UTC) #20
Torne
On 2016/10/26 21:33:56, commit-bot: I haz the power wrote: > Patchset 4 (id:??) landed as ...
3 years, 9 months ago (2016-10-27 15:17:28 UTC) #21
michaelbai
On 2016/10/27 15:17:28, Torne wrote: > On 2016/10/26 21:33:56, commit-bot: I haz the power wrote: ...
3 years, 9 months ago (2016-10-27 16:54:13 UTC) #22
michaelbai
3 years, 9 months ago (2016-10-27 16:54:41 UTC) #23
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/2453683007/ by michaelbai@chromium.org.

The reason for reverting is: Wrong change about comment.

Powered by Google App Engine
This is Rietveld 408576698