Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(579)

Issue 200753002: [Android] Workaround of an android platform bug. (Closed)

Created:
6 years, 9 months ago by Feng Qian
Modified:
5 years, 1 month ago
CC:
chromium-reviews, darin (slow to review), craigdh+watch_chromium.org, viettrungluu+watch_chromium.org, jam, abarth-chromium, joi+watch-content_chromium.org, Aaron Boodman, bulach+watch_chromium.org, yfriedman+watch_chromium.org, ilevy-cc_chromium.org, darin-cc_chromium.org, klundberg+watch_chromium.org, erikwright+watch_chromium.org, ben+mojo_chromium.org, android-webview-reviews_chromium.org, jochen+watch_chromium.org
Visibility:
Public.

Description

[Android] Workaround of an android platform bug. On some Android devices (e.g., Sony Xperia), package manager may fail to extract native libraries when updating Chrome. The change tries alleviate the situation by: 1) name libchrome with version number; 2) when failed to load library, try to extract native libraies and load them. BUG=311644 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=258546

Patch Set 1 #

Patch Set 2 : fix a build error. #

Total comments: 5

Patch Set 3 : fix Linker.java so content_linker works with new naming scheme. #

Total comments: 8

Patch Set 4 : move logic of removing old libchrome .so into clank/ #

Patch Set 5 : rebase #

Patch Set 6 : address klobag' #

Total comments: 8

Patch Set 7 : Add a test of LibraryLoaderHelper class #

Total comments: 14

Patch Set 8 : take reviewer's comments #

Total comments: 1

Patch Set 9 : take reviewers' comments #

Total comments: 1

Patch Set 10 : rebase #

Total comments: 1

Patch Set 11 : move uma recording from clank/ into base #

Total comments: 2

Patch Set 12 : public -> private of static field #

Patch Set 13 : make a temp fix of webview build failure #

Total comments: 5

Patch Set 14 : fix webview issue #

Patch Set 15 : ChildProcessService needs to use the hack #

Unified diffs Side-by-side diffs Delta from patch set Stats (+425 lines, -15 lines) Patch
M android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -1 line 0 comments Download
M base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 7 chunks +51 lines, -6 lines 0 comments Download
A base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +259 lines, -0 lines 0 comments Download
A base/android/javatests/src/org/chromium/base/LibraryLoaderHelperTest.java View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +53 lines, -0 lines 0 comments Download
M base/android/library_loader/library_loader_hooks.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +4 lines, -0 lines 0 comments Download
A build/android/gyp/delete_files.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +45 lines, -0 lines 0 comments Download
M build/common.gypi View 1 2 3 4 5 6 7 8 9 1 chunk +2 lines, -0 lines 0 comments Download
M build/util/version.gypi View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/app/ChildProcessService.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -1 line 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/BrowserStartupController.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -1 line 0 comments Download
M content/public/android/javatests/src/org/chromium/content/browser/ContentCommandLineTest.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -1 line 0 comments Download
M content/public/android/javatests/src/org/chromium/content/browser/ImportantFileWriterAndroidTest.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -1 line 0 comments Download
M content/shell/android/browsertests_apk/src/org/chromium/content_browsertests_apk/ContentBrowserTestsActivity.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -1 line 0 comments Download
M content/shell/android/linker_test_apk/src/org/chromium/chromium_linker_test_apk/ChromiumLinkerTestActivity.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -1 line 0 comments Download
M content/shell/android/shell_apk/src/org/chromium/content_shell_apk/ContentShellActivity.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -1 line 0 comments Download
M mojo/shell/android/apk/src/org/chromium/mojo_shell_apk/MojoShellActivity.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 58 (1 generated)
Feng Qian
Sorry guys, This is another try to workaround issue 311644. The cause of problem is ...
6 years, 9 months ago (2014-03-14 19:14:50 UTC) #1
Feng Qian
oops, forgot to include the prev CL link: https://codereview.chromium.org/180273005/ On 2014/03/14 19:14:50, Feng Qian wrote: ...
6 years, 9 months ago (2014-03-14 19:15:29 UTC) #2
cjhopman
https://codereview.chromium.org/200753002/diff/20001/build/common.gypi File build/common.gypi (right): https://codereview.chromium.org/200753002/diff/20001/build/common.gypi#newcode3783 build/common.gypi:3783: 'android_product_extension': '<(lib_version).so', This is going to apply to all ...
6 years, 9 months ago (2014-03-17 18:32:35 UTC) #3
Feng Qian
https://codereview.chromium.org/200753002/diff/20001/build/common.gypi File build/common.gypi (right): https://codereview.chromium.org/200753002/diff/20001/build/common.gypi#newcode3783 build/common.gypi:3783: 'android_product_extension': '<(lib_version).so', Strictly speaking, it applies to all libraries ...
6 years, 9 months ago (2014-03-18 00:06:59 UTC) #4
cjhopman
https://codereview.chromium.org/200753002/diff/20001/build/java_apk.gypi File build/java_apk.gypi (right): https://codereview.chromium.org/200753002/diff/20001/build/java_apk.gypi#newcode187 build/java_apk.gypi:187: ['native_lib_target != "" and component != "shared_library" and use_chromium_linker ...
6 years, 9 months ago (2014-03-18 00:16:54 UTC) #5
klobag.chromium
https://codereview.chromium.org/200753002/diff/40001/base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java File base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java (right): https://codereview.chromium.org/200753002/diff/40001/base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java#newcode55 base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java:55: * If named library is not in backup directory, ...
6 years, 9 months ago (2014-03-18 19:17:23 UTC) #6
Feng Qian
https://codereview.chromium.org/200753002/diff/40001/base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java File base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java (right): https://codereview.chromium.org/200753002/diff/40001/base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java#newcode55 base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java:55: * If named library is not in backup directory, ...
6 years, 9 months ago (2014-03-18 21:59:27 UTC) #7
klobag.chromium
https://codereview.chromium.org/200753002/diff/40001/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java File base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java (right): https://codereview.chromium.org/200753002/diff/40001/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java#newcode167 base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java:167: LibraryLoaderHelper.cleanupOldLibraries(context, !sNativeLibraryHack); Should we move this after line 132 ...
6 years, 9 months ago (2014-03-19 00:32:27 UTC) #8
Feng Qian
https://codereview.chromium.org/200753002/diff/40001/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java File base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java (right): https://codereview.chromium.org/200753002/diff/40001/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java#newcode167 base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java:167: LibraryLoaderHelper.cleanupOldLibraries(context, !sNativeLibraryHack); Sure, sounds a good plan. On 2014/03/19 ...
6 years, 9 months ago (2014-03-19 16:08:26 UTC) #9
Feng Qian
Darin, the CL has small changes in mojo code, which requires mojo owner's approval. Can ...
6 years, 9 months ago (2014-03-19 16:47:51 UTC) #10
klobag.chromium
https://codereview.chromium.org/200753002/diff/100001/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java File base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java (right): https://codereview.chromium.org/200753002/diff/100001/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java#newcode61 base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java:61: public static void ensureInitialized(Context context, boolean isBrowserProcess) can we ...
6 years, 9 months ago (2014-03-19 17:23:45 UTC) #11
Feng Qian
https://codereview.chromium.org/200753002/diff/100001/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java File base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java (right): https://codereview.chromium.org/200753002/diff/100001/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java#newcode61 base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java:61: public static void ensureInitialized(Context context, boolean isBrowserProcess) Per discussion, ...
6 years, 9 months ago (2014-03-19 17:52:50 UTC) #12
Yaron
https://codereview.chromium.org/200753002/diff/120001/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java File base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java (right): https://codereview.chromium.org/200753002/diff/120001/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java#newcode48 base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java:48: public static boolean sNativeLibraryHack = false; nit: sNativeLibraryHackWasUsed (to ...
6 years, 9 months ago (2014-03-20 01:24:48 UTC) #13
Feng Qian
https://codereview.chromium.org/200753002/diff/120001/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java File base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java (right): https://codereview.chromium.org/200753002/diff/120001/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java#newcode48 base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java:48: public static boolean sNativeLibraryHack = false; Good suggestion, done. ...
6 years, 9 months ago (2014-03-20 15:28:47 UTC) #14
klobag.chromium
https://codereview.chromium.org/200753002/diff/140001/base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java File base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java (right): https://codereview.chromium.org/200753002/diff/140001/base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java#newcode127 base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java:127: if (sLibrariesWereUnpacked) { hmm, will this cause loadNativeLibrariesUsingWorkaroundForTesting() fail ...
6 years, 9 months ago (2014-03-20 16:26:44 UTC) #15
klobag.chromium
https://codereview.chromium.org/200753002/diff/160001/base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java File base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java (right): https://codereview.chromium.org/200753002/diff/160001/base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java#newcode43 base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java:43: */ add @VisibleForTesting, then you don't need to make ...
6 years, 9 months ago (2014-03-20 16:55:22 UTC) #16
klobag.chromium
lgtm
6 years, 9 months ago (2014-03-20 17:01:12 UTC) #17
sky
mojo/shell/android/apk/src/org/chromium/mojo_shell_apk/MojoShellActivity.java LGTM
6 years, 9 months ago (2014-03-20 17:21:45 UTC) #18
Feng Qian
The CQ bit was checked by feng@chromium.org
6 years, 9 months ago (2014-03-20 17:46:08 UTC) #19
Yaron
https://codereview.chromium.org/200753002/diff/120001/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java File base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java (right): https://codereview.chromium.org/200753002/diff/120001/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java#newcode144 base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java:144: if (LibraryLoaderHelper.tryLoadLibraryUsingWorkaround( On 2014/03/20 15:28:47, Feng Qian wrote: > ...
6 years, 9 months ago (2014-03-20 17:48:44 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/feng@chromium.org/200753002/180001
6 years, 9 months ago (2014-03-20 17:50:39 UTC) #21
Yaron
https://codereview.chromium.org/200753002/diff/180001/base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java File base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java (right): https://codereview.chromium.org/200753002/diff/180001/base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java#newcode131 base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java:131: return false; This doesn't seem right You want L93 ...
6 years, 9 months ago (2014-03-20 17:51:54 UTC) #22
Yaron
lgtm based on off-thread explanation. thanks
6 years, 9 months ago (2014-03-20 18:11:47 UTC) #23
commit-bot: I haz the power
Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=56514
6 years, 9 months ago (2014-03-20 18:31:39 UTC) #24
Yaron
lgtm https://codereview.chromium.org/200753002/diff/200001/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java File base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java (right): https://codereview.chromium.org/200753002/diff/200001/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java#newcode48 base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java:48: public static boolean sNativeLibraryHackWasUsed = false; Does this ...
6 years, 9 months ago (2014-03-20 18:34:46 UTC) #25
Feng Qian
The CQ bit was checked by feng@chromium.org
6 years, 9 months ago (2014-03-20 18:36:09 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/feng@chromium.org/200753002/200001
6 years, 9 months ago (2014-03-20 18:40:43 UTC) #27
Feng Qian
The CQ bit was checked by feng@chromium.org
6 years, 9 months ago (2014-03-20 18:43:05 UTC) #28
commit-bot: I haz the power
Failed to trigger a try job on chromium_presubmit HTTP Error 400: Bad Request
6 years, 9 months ago (2014-03-20 18:55:27 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/feng@chromium.org/200753002/220001
6 years, 9 months ago (2014-03-20 18:55:32 UTC) #30
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-20 19:25:58 UTC) #31
commit-bot: I haz the power
Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=56538
6 years, 9 months ago (2014-03-20 19:25:59 UTC) #32
michaelbai
lgtm
6 years, 9 months ago (2014-03-20 20:38:23 UTC) #33
Feng Qian
The CQ bit was checked by feng@chromium.org
6 years, 9 months ago (2014-03-20 20:38:51 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/feng@chromium.org/200753002/220001
6 years, 9 months ago (2014-03-20 20:39:31 UTC) #35
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-20 22:36:38 UTC) #36
commit-bot: I haz the power
Retried try job too often on android_aosp for step(s) compile http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_aosp&number=57026
6 years, 9 months ago (2014-03-20 22:36:39 UTC) #37
Feng Qian
The CQ bit was checked by feng@chromium.org
6 years, 9 months ago (2014-03-20 23:03:55 UTC) #38
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/feng@chromium.org/200753002/240001
6 years, 9 months ago (2014-03-20 23:04:46 UTC) #39
boliu
lgtm since I don't want to block cq... https://codereview.chromium.org/200753002/diff/240001/android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java File android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java (right): https://codereview.chromium.org/200753002/diff/240001/android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java#newcode37 android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java:37: LibraryLoader.loadNow(null, ...
6 years, 9 months ago (2014-03-20 23:27:11 UTC) #40
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-20 23:36:11 UTC) #41
commit-bot: I haz the power
Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=56623
6 years, 9 months ago (2014-03-20 23:36:12 UTC) #42
boliu
https://codereview.chromium.org/200753002/diff/240001/build/android/gyp/delete_files.py File build/android/gyp/delete_files.py (right): https://codereview.chromium.org/200753002/diff/240001/build/android/gyp/delete_files.py#newcode12 build/android/gyp/delete_files.py:12: import shutil I think presubmit is complaining you don't ...
6 years, 9 months ago (2014-03-20 23:50:11 UTC) #43
Feng Qian
I changed the API to loadLibrary(Context context), and it accepts null context. In the case ...
6 years, 9 months ago (2014-03-20 23:59:48 UTC) #44
Feng Qian
https://codereview.chromium.org/200753002/diff/240001/android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java File android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java (right): https://codereview.chromium.org/200753002/diff/240001/android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java#newcode37 android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java:37: LibraryLoader.loadNow(null, false); ok, will make it LibraryLoader.loadNow(null), and the ...
6 years, 9 months ago (2014-03-21 00:01:41 UTC) #45
Feng Qian
The CQ bit was checked by feng@chromium.org
6 years, 9 months ago (2014-03-21 00:17:43 UTC) #46
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/feng@chromium.org/200753002/280001
6 years, 9 months ago (2014-03-21 00:19:06 UTC) #47
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-21 00:23:50 UTC) #48
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.chromium on linux_chromium_clang_dbg
6 years, 9 months ago (2014-03-21 00:23:50 UTC) #49
Feng Qian
The CQ bit was checked by feng@chromium.org
6 years, 9 months ago (2014-03-21 00:27:57 UTC) #50
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/feng@chromium.org/200753002/280001
6 years, 9 months ago (2014-03-21 00:30:39 UTC) #51
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-21 00:34:45 UTC) #52
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.chromium on linux_chromium_clang_dbg
6 years, 9 months ago (2014-03-21 00:34:46 UTC) #53
Feng Qian
The CQ bit was checked by feng@chromium.org
6 years, 9 months ago (2014-03-21 00:49:47 UTC) #54
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/feng@chromium.org/200753002/280001
6 years, 9 months ago (2014-03-21 00:51:45 UTC) #55
commit-bot: I haz the power
Change committed as 258546
6 years, 9 months ago (2014-03-21 12:39:58 UTC) #56
Hezley Khez
5 years, 1 month ago (2015-11-13 18:55:13 UTC) #58
Message was sent while issue was closed.
Issues 200753002 [Android] workaround of an android platform bug

Powered by Google App Engine
This is Rietveld 408576698