[Cronet] Append version to libcronet library name.

[Cronet] Append version to libcronet library name.

In situations where shared objects fail to install when an app is updated,
this change prevents loading of out-of-date shared objects because the shared
object name now changes with every app update.  Loading out-of-date
shared objects can crash the app.  Failing to find the shared object
(because the name has changed) gives a LibraryLoader registered with
CronetEngine.Builder.setLibraryLoader() a chance to unzip it from the APK.

R=kapishnikov
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2741773003
Cr-Commit-Position: refs/heads/master@{#455971}
Committed: https://chromium.googlesource.com/chromium/src/+/8217d3c2acd05f9d5d5131b4f54743f231bc53a4

[pauljensen, 2017-03-09 18:07:26]

Description was changed from

==========
[Cronet] Append version to libcronet library name.

In situations where shared objects fail to install when an app is updated,
this change prevents loading of out-of-date shared objects because the shared
object name now changes with every app update.  Loading out-of-date
shared objects can crash the app.  Failing to find the shared object
(because the name has changed) gives a LibraryLoader registered with
CronetEngine.Builder.setLibraryLoader() a chance to unzip it from the APK.

R=kapishnikov
==========

to

==========
[Cronet] Append version to libcronet library name.

In situations where shared objects fail to install when an app is updated,
this change prevents loading of out-of-date shared objects because the shared
object name now changes with every app update.  Loading out-of-date
shared objects can crash the app.  Failing to find the shared object
(because the name has changed) gives a LibraryLoader registered with
CronetEngine.Builder.setLibraryLoader() a chance to unzip it from the APK.

R=kapishnikov
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[pauljensen, 2017-03-09 18:40:46]

Andrei, PTAL, thanks!

[kapishnikov, 2017-03-09 19:29:41]

Looks good and simple. Some minor comments. LGTM

https://codereview.chromium.org/2741773003/diff/1/components/cronet/android/B...
File components/cronet/android/BUILD.gn (right):

https://codereview.chromium.org/2741773003/diff/1/components/cronet/android/B...
components/cronet/android/BUILD.gn:259: output_name =
"cronet.$chrome_version_full"
Should we use '_' instead of '.' as the separator?  I couldn't find any .so file
that uses "." as the separator except for separating the version parts.

https://codereview.chromium.org/2741773003/diff/1/components/cronet/android/B...
components/cronet/android/BUILD.gn:1290:
"$root_out_dir/libcronet.$chrome_version_full.so",
Since we use "libcronet.$chrome_version_full.so" in four places, maybe we should
introduce an intermediate variable, like cronet_shared_library_file_name =
libcronet.$chrome_version_full.so

[pauljensen, 2017-03-10 01:55:34]

https://codereview.chromium.org/2741773003/diff/1/components/cronet/android/B...
File components/cronet/android/BUILD.gn (right):

https://codereview.chromium.org/2741773003/diff/1/components/cronet/android/B...
components/cronet/android/BUILD.gn:259: output_name =
"cronet.$chrome_version_full"
On 2017/03/09 19:29:40, kapishnikov wrote:
> Should we use '_' instead of '.' as the separator?  I couldn't find any .so
file
> that uses "." as the separator except for separating the version parts.

I think '.' is commonly used for this sort of thing; most libs in /usr/lib use
'.' to separate version parts.  It also makes the BUILD.gn and Java parts of
this change much simpler to use '.'

$ cd /usr/lib
$ ls -1d *\.0* | wc -l
235
$ ls -1d *_0*
ls: cannot access *_0*: No such file or directory

https://codereview.chromium.org/2741773003/diff/1/components/cronet/android/B...
components/cronet/android/BUILD.gn:1290:
"$root_out_dir/libcronet.$chrome_version_full.so",
On 2017/03/09 19:29:40, kapishnikov wrote:
> Since we use "libcronet.$chrome_version_full.so" in four places, maybe we
should
> introduce an intermediate variable, like cronet_shared_library_file_name =
> libcronet.$chrome_version_full.so

Done, I also made a variable for the simple name.

[pauljensen, 2017-03-10 01:58:07]

The CQ bit was checked by pauljensen@chromium.org

[pauljensen, 2017-03-10 01:58:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
kapishnikov@chromium.org
Link to the patchset: https://codereview.chromium.org/2741773003/#ps20001
(title: "address comment")

[commit-bot: I haz the power, 2017-03-10 01:58:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kapishnikov, 2017-03-10 02:03:22]

On 2017/03/10 01:55:34, pauljensen wrote:
>
https://codereview.chromium.org/2741773003/diff/1/components/cronet/android/B...
> File components/cronet/android/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2741773003/diff/1/components/cronet/android/B...
> components/cronet/android/BUILD.gn:259: output_name =
> "cronet.$chrome_version_full"
> On 2017/03/09 19:29:40, kapishnikov wrote:
> > Should we use '_' instead of '.' as the separator?  I couldn't find any .so
> file
> > that uses "." as the separator except for separating the version parts.
> 
> I think '.' is commonly used for this sort of thing; most libs in /usr/lib use
> '.' to separate version parts.  It also makes the BUILD.gn and Java parts of
> this change much simpler to use '.'
> 
> $ cd /usr/lib
> $ ls -1d *\.0* | wc -l
> 235
> $ ls -1d *_0*
> ls: cannot access *_0*: No such file or directory
> 
I used this query from the root src:

find . -printf "%f\n" | grep ".*\..*\.so$"

>
https://codereview.chromium.org/2741773003/diff/1/components/cronet/android/B...
> components/cronet/android/BUILD.gn:1290:
> "$root_out_dir/libcronet.$chrome_version_full.so",
> On 2017/03/09 19:29:40, kapishnikov wrote:
> > Since we use "libcronet.$chrome_version_full.so" in four places, maybe we
> should
> > introduce an intermediate variable, like cronet_shared_library_file_name =
> > libcronet.$chrome_version_full.so
> 
> Done, I also made a variable for the simple name.

[kapishnikov, 2017-03-10 02:07:06]

On 2017/03/10 02:03:22, kapishnikov wrote:
> On 2017/03/10 01:55:34, pauljensen wrote:
> >
>
https://codereview.chromium.org/2741773003/diff/1/components/cronet/android/B...
> > File components/cronet/android/BUILD.gn (right):
> > 
> >
>
https://codereview.chromium.org/2741773003/diff/1/components/cronet/android/B...
> > components/cronet/android/BUILD.gn:259: output_name =
> > "cronet.$chrome_version_full"
> > On 2017/03/09 19:29:40, kapishnikov wrote:
> > > Should we use '_' instead of '.' as the separator?  I couldn't find any
.so
> > file
> > > that uses "." as the separator except for separating the version parts.
> > 
> > I think '.' is commonly used for this sort of thing; most libs in /usr/lib
use
> > '.' to separate version parts.  It also makes the BUILD.gn and Java parts of
> > this change much simpler to use '.'
> > 
> > $ cd /usr/lib
> > $ ls -1d *\.0* | wc -l
> > 235
> > $ ls -1d *_0*
> > ls: cannot access *_0*: No such file or directory
> > 
> I used this query from the root src:
> 
> find . -printf "%f\n" | grep ".*\..*\.so$"

Looks like the common pattern is to use "-" before the version.

> 
> >
>
https://codereview.chromium.org/2741773003/diff/1/components/cronet/android/B...
> > components/cronet/android/BUILD.gn:1290:
> > "$root_out_dir/libcronet.$chrome_version_full.so",
> > On 2017/03/09 19:29:40, kapishnikov wrote:
> > > Since we use "libcronet.$chrome_version_full.so" in four places, maybe we
> > should
> > > introduce an intermediate variable, like cronet_shared_library_file_name =
> > > libcronet.$chrome_version_full.so
> > 
> > Done, I also made a variable for the simple name.

[commit-bot: I haz the power, 2017-03-10 03:03:23]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489111087422640,
"parent_rev": "a2181336154c12cb49601f515cb5dace1b06b1f2", "commit_rev":
"8217d3c2acd05f9d5d5131b4f54743f231bc53a4"}

[commit-bot: I haz the power, 2017-03-10 03:04:20]

Description was changed from

==========
[Cronet] Append version to libcronet library name.

In situations where shared objects fail to install when an app is updated,
this change prevents loading of out-of-date shared objects because the shared
object name now changes with every app update.  Loading out-of-date
shared objects can crash the app.  Failing to find the shared object
(because the name has changed) gives a LibraryLoader registered with
CronetEngine.Builder.setLibraryLoader() a chance to unzip it from the APK.

R=kapishnikov
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
[Cronet] Append version to libcronet library name.

In situations where shared objects fail to install when an app is updated,
this change prevents loading of out-of-date shared objects because the shared
object name now changes with every app update.  Loading out-of-date
shared objects can crash the app.  Failing to find the shared object
(because the name has changed) gives a LibraryLoader registered with
CronetEngine.Builder.setLibraryLoader() a chance to unzip it from the APK.

R=kapishnikov
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2741773003
Cr-Commit-Position: refs/heads/master@{#455971}
Committed:
https://chromium.googlesource.com/chromium/src/+/8217d3c2acd05f9d5d5131b4f547...
==========

[commit-bot: I haz the power, 2017-03-10 03:04:21]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/8217d3c2acd05f9d5d5131b4f547...