The difference is caused by NativeLibraries.java in component build, NativeLibraries has a member LIBRARIES which defines a list of libraries need to be loaded, there are a number of libraries in component build.

- For WebView, the order of libraries are different, this should be easy to fix.
- For Monochrome, the libraries are different because 64-bit libraries are webview only.

We could move the libraries list to a asset file, but this will slow down startup, we shouldn't do it, thought?

It is also interesting, why 32-bit Chrome in 64-bit Monochrome works? the libraries list should come from 64-bit libraries, we probably just haven't run into the missing shared libraries.

Basically, this libraries list doesn't work on component build of 64-bit Monochrome.

Are you saying that in a component build, NativeLibraries.LIBRARIES is a list that includes all the different components?

If so that's *really* surprising to me, and seems like it's wrong in the first place (though I'm not really sure how this mechanism is supposed to work) - it should only be necessary to load the "root" library and the others will be picked up automatically via the DT_NEEDED dependency tree. Maybe there's a reason I'm not thinking of though?

I suspect this is why it works fine anyway, though - it loads libmonochrome.so and that just pulls in all the dependencies.

Possibly-related question: webview still appears to have a pregenerated copy of NativeLibraries.java checked into the sources; is that actually used or is it obsolete? Either way we should probably try to get rid of it ;)

I checked the deps graph with write_ordered_libraries.py, but found there are multiple root libs, this seemed not right, otherwise JNI won't register correctly, maybe the script has issue, I will take another look today.

What does the list actually look like? Does it include all the libraries in the apk, or just some of them? (if it's not all of them, which ones?)

This list includes all libraries, write_ordered_libraries.py intentionally sorts all those libraries according the dependent tree, it obviously assumes the shared libraries that current library depends on wouldn't be loaded automatically, I haven't checked git log, don't know why order is important at first place.

There are multiple root libraries in all the libraries generated by component build, except the one has JNI entry point, others most likely are not used at all, e.g. 32-bit Monochrome build has below root libraries

"captive_portal.cr","compositor.cr","device_vibration.cr","devices.cr","gles2_c_lib.cr","monochrome.cr","onc.cr","surface.cr"

I removed compositor.cr from the deps graph and build flat libmonochrome, there is no error. I also only loaded libmonchrome.cr.so in 64-bit Monochrome, both WebView and Chrome worked fine, the other libraries (except above unused root libraries) are loaded automatically. 

cc to yfriedman if he still checks email, he should know the background.

Hm. Just to confirm I understand what you mean:

1) In a component build, those components (captive_portal, compositor, etc) have no other components that depend on them via DT_NEEDED, and so they are separate roots in the dependency graph

2) In a regular static build, those components are linked in, but removing them from the build does not cause linker errors (implying that they are actually unused)

Is that correct? If so then we can probably just remove them. The linker will probably be dropping the actual code from them anyway due to --gc-sections...

I'm still not sure why write_ordered_libraries.py actually has to include any libraries other than "root" libraries in the list in the first place, though. The only thing that occurs to me is that it's required when using the crazy linker; have you tested that configuration? (on regular chrome, rather than monochrome, of course)

RE #7 

Yes, both for 1) and 2).

I don't think crazy linker works in component build, I took a few hours to make it work, but still failed with


10-10 17:21:30.054 19847 19868 I cr_LibraryLoader: Chromium Linker Loading c++_shared from libc++_shared.so
10-10 17:21:30.057 19847 19868 E cr_ChromiumAndroidLinker: GetLibraryLoadSize: Failed to find library at address 0x63035000
10-10 17:21:30.057 19847 19868 E cr_ChromiumAndroidLinker: LoadLibrary: Unable to find size for load at 0x63035000
10-10 17:21:30.057 19847 19868 E cr_LibraryLoader: Unable to load library: libc++_shared.so

Once crazy linker supports component build, it should have ability to load libraries according DT_NEEDED. 

Back to the native libraries list, I am planing to find the root library by finding out which share library exposes JNI_OnLoad() method, and use this library as the single entry of NativeLibraries.LIBRARIES.

I'm not sure whether the crazy linker was ever expected to work for component builds or not, and if it doesn't currently then I doubt it ever will; it's not something we're putting a lot of effort into now that it's obsolete on current OS versions.

Doesn't the build graph already imply which library is the main one? (i.e. the one(s) that the APK depends on directly) - is there no way to just get this information from the build setup, rather than actually by looking at the libraries? I'm not sure we want to depend on which ones have JNI_OnLoad - it's perfectly *possible* for an APK to contain more than one actual root library (even though ours generally don't), and it's not required that a library actually have a JNI_OnLoad.

Separately to changing NativeLibraries.LIBRARIES, it seems like it would be useful if we actually checked the libraries produced by a component build to see if they are all necessary or not, and if not, complained, since those other components that aren't used should really just be removed from the build entirely..

Re questions in #9

1) Crazy linker once worked for component build, but hasn't failed for a long time, see http://b/11379966.

2) Currently to build android APK doesn't need to pass shared library name, I didn't find a way to get the shared library name from deps unless manually pass the library name, it was what we done, but was removed. Using JNI_Onload is the easy way now.

3) I were thinking about it, but it requires to fix the wrong deps first - a large work, we could ends up with having 'if (!is_android)' in a lot of places, you can feel about it by checking how widely gles2_c_lib is used, we could file another bug to track it.

If using a component build with the crazy linker has been broken for a long time and nobody has complained about it or fixed it then probably nobody cares about this configuration, and it seems like we should just explicitly not support it - is there a way to actually disable it such it'll be clear to anyone who tries that it doesn't work, rather than just having it fail in an unclear way at runtime?
Also, have you actually checked if it works currently? That bug didn't go anywhere, it's not obvious whether it was resolved or not.

I'm still confused about why this write_ordered_libraries step is there in the first place. Is it actually because of the crazy linker? Have you looked at the history/talked to people to confirm? I think we should talk with the GN experts about whether there's a better way to handle the library dependency graph here so that we aren't in the position of having to scan the actual libraries to work out how to generate the list; it really seems like it should be possible and would be much saner to figure it out from the graph.

I don't think fixing the incorrect deps is a lot of work, and the benefit is significant: developer productivity (it will speed up all builds by not compiling unused code). We can definitely do that in a separate bug though, yes.

When write_ordered_libraries.py was initially added (without transitive dependencies) the bug says:

This is designed to also support the component build, where, we will
have to calculate the list of libraries at build time, and sort them in
dependency order (because Android's linker, for some reason, doesn't do
that itself).

And then when the transitive deps were added:

ake write_library_dependencies.py find all transitive dependencies

For the component build, it is impractical/impossible to explicitly
list all library dependencies. This list is required (in dependency
order) for several of the apk-building steps.

For now, we will generate this list as follows:
Use readelf to find all transitive dependencies
Topologically sort those dependencies

Once we can expose this information from gyp
(http://crbug.com/2255588), it is straightforward to update this action
to use the gyp-exposed list of libraries.

RE#12, I didn't see anywhere said Android linker has issue, I might overlook it, what's the bug?(the URL doesn't work for me.)

These are commit descriptions Toby copied (you can probably find them with git log --grep) - the bug number is a typo in the original commit description unfortunately, I'm not sure what the right number is.

Looks like the right bug is https://bugs.chromium.org/p/chromium/issues/detail?id=225558

The android linker issue seems not correct to me, if linker failed to load lib according DT_NEEDED, how the system libs was loaded, does Android loads all system libs in its zygote?

Anyway, I tried Android J,K,L,M,N, there is no issue to explicitly load libchrome.cr.so only.

I broken the ordering of the libraries somewhat recently, and it did actually cause bad things to happen with incremental install:

https://bugs.chromium.org/p/chromium/issues/detail?id=624791

Is creating component versions of monochrome_apk that have both 64-bit and 32-bit a thing that we even need to support? Component build generally means debug build, and there's no need to create fat apks (64 bit + 32 bit) for debug mode, is there?

Component build of monochrome that has both 32-bit and 64-bit libraries is already supported, the question is do we need to support merging component(debug) build APK? The reason what I can see is to find the bug like this.

If we don't need to support it though, then this isn't a bug.

Re #16: indeed, it's definitely not the case that Android's linker fails here. I can only assume that the description is incorrect or that it's actually referring to some more specific situation where it has a problem that we aren't encountering. The system linker loads everything according to DT_NEEDED and always has.

Re #17: I have no idea how incremental installs work and am definitely not sure why loading all the component libraries separately would be required there. Understanding that would be useful I think.

There is definitely a need to be able to make fat APKs for debug/component builds: the fat APKs are the only ones that actually work for all local dev purposes on 64-bit devices (the 64-bit only APK causes all 32-bit apps to be broken, which can either be a direct problem with dev work, or can just be annoying/distracting by causing 32-bit apps to crash in the background unrelated to what you're trying to debug).

I don't think it's sufficient to say that the current state is okay, because even though it does all actually work in a regular component build, it sounds like it won't for an incremental install, and the library list is still actually different between 32-bit and 64-bit: if it just happens to work when using the wrong list because of the linker loading the right things via DT_NEEDED that's convenient for now, but isn't guaranteed to always be true, and if it causes Monochrome to load a different set of components than other builds that might be weird/confusing. This is not just a problem when merging the APKs; the library list is still actually wrong in a monochrome apk built without merging, no?

How incremental install works:
1. It copies all .so files to the app's data directory
2. It modifies the Classloader's native library search path via reflection to point at it

The reflection bit is here: https://cs.chromium.org/chromium/src/build/android/incremental_install/java/org/chromium/incrementalinstall/ClassLoaderPatcher.java?q=classloaderpatcher&sq=package:chromium&dr=CSs&l=151

Perhaps some part of the linker is not picking up the changed search path when loading DT_NEEDED entries.

The way System.loadLibrary is implemented is pretty unintuitive:

1) It uses ClassLoader.findLibrary to determine the absolute path of the native library name you asked for.

2) It figures out what the library search path is supposed to be and calls a special function exposed in the native linker that modifies the effective LD_LIBRARY_PATH for the current process to be that.

3) It then calls dlopen() with the *absolute* path found by step 1. The linker will resolve DT_NEEDED entries using the search path set by step 2.

So it's possible that the incremental install logic is only affecting step 1 and not step 2, because the actual linker's search path is not involved in finding the main library; the main library is already an absolute path.

Also: it's possible that on N and later the exact behaviour here has become more complicated, because the addition of linker namespaces means that there's now more different places keeping track of what library paths are supposed to be (the namespace itself also has a list of library directories that are permitted to be used).