Linking arm and arm64 gvr static shim library

Linking arm and arm64 gvr static shim library.

This should reduce our binary size increase significantly for VR
related feature, as well as avoid issue with crazy linker which has
problem handling two shared libraries.

Note that the static library is mostly a shim lib. It will dynamically
load the correct library from VrCore service at runtime when needed.
Our initial target is Pixel and Pixel XL phones. And VrCore should exist
in both phones. If VrCore doesn't exist or the version is older than we
expect, VR will be disabled. Support for x86 and x64 will be added later.

BUG=644785
Committed: https://crrev.com/0907c2d02be6980427e4ecc372c47985e7dddffd
Cr-Commit-Position: refs/heads/master@{#431699}

[bshe, 2016-11-09 00:06:58]

The CQ bit was checked by bshe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-09 00:07:26]

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

[commit-bot: I haz the power, 2016-11-09 01:43:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-09 01:43:19]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[bshe, 2016-11-09 12:51:43]

The CQ bit was checked by bshe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-09 12:51:53]

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

[commit-bot: I haz the power, 2016-11-09 12:54:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-09 12:54:23]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[bshe, 2016-11-09 13:06:44]

The CQ bit was checked by bshe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-09 13:06:52]

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

[commit-bot: I haz the power, 2016-11-09 14:11:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-09 14:11:12]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[bshe, 2016-11-09 16:20:22]

The CQ bit was checked by bshe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-09 16:20:43]

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

[commit-bot: I haz the power, 2016-11-09 16:28:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-09 16:28:14]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[bshe, 2016-11-09 17:09:14]

The CQ bit was checked by bshe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-09 17:09:27]

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

[commit-bot: I haz the power, 2016-11-09 17:13:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-09 17:13:42]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[bshe, 2016-11-09 17:33:44]

The CQ bit was checked by bshe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-09 17:34:08]

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

[commit-bot: I haz the power, 2016-11-09 17:43:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-09 17:43:25]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[bshe, 2016-11-09 21:05:04]

The CQ bit was checked by bshe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-09 21:05:39]

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

[commit-bot: I haz the power, 2016-11-09 21:14:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-09 21:14:36]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[bshe, 2016-11-09 23:34:02]

The CQ bit was checked by bshe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-09 23:34:31]

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

[commit-bot: I haz the power, 2016-11-10 00:53:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-10 00:53:39]

Dry run: This issue passed the CQ dry run.

[bshe, 2016-11-10 14:59:01]

The CQ bit was checked by bshe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-10 14:59:19]

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

[commit-bot: I haz the power, 2016-11-10 15:36:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-10 15:36:57]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[bshe, 2016-11-10 16:07:14]

The CQ bit was checked by bshe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-10 16:07:27]

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

[commit-bot: I haz the power, 2016-11-10 17:13:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-10 17:13:58]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[bshe, 2016-11-10 18:23:52]

Patchset #15 (id:280001) has been deleted

[bshe, 2016-11-10 18:41:35]

Description was changed from

==========
Linking static lib


BUG=
==========

to

==========
Linking arm and arm64 gvr static shim library.

This should reduce our binary size increase significantly for VR
related feature, as well as avoid issue with crazy linker which has
problem handling two shared libraries.

Note that the static library is mostly a shim lib. It will dynamically
load the correct library from VrCore service at runtime when needed.
Our initial target is Pixel and Pixel XL phones. And VrCore should exist
in both phones. If VrCore doesn't exist or the version is older than we
expect, VR will be disabled. Support for x86 and x64 will be added later.


BUG=644785
==========

[bshe, 2016-11-10 18:42:38]

bshe@chromium.org changed reviewers:
+ agrieve@chromium.org, bajones@chromium.org, dtrainor@chromium.org,
thestig@chromium.org

[bshe, 2016-11-10 18:42:40]

PTAL. Thanks!

bajones@chromium.org: Please review changes in 
device/vr/*
chrome/browser/android/vr_shell/*
third_party/gvr-android-sdk/*

dtrainor@chromium.org: Please review changes in
chrome/android/*
chrome/browser/android/*

agrieve@chromium.org: Please review changes in 
build/*
and all build, deps related files

thestig@chromium.org: Please review changes in 
third_party/gvr-android-sdk/DEPS for base dependency

[Lei Zhang, 2016-11-10 18:54:35]

https://codereview.chromium.org/2467873004/diff/300001/third_party/gvr-androi...
File third_party/gvr-android-sdk/DEPS (right):

https://codereview.chromium.org/2467873004/diff/300001/third_party/gvr-androi...
third_party/gvr-android-sdk/DEPS:2: '+base',
You only need base/android, right? You can add that, or just the 3 headers you
need, and ask base/android/OWNERS for a review.

[agrieve, 2016-11-10 19:16:30]

https://codereview.chromium.org/2467873004/diff/300001/build/config/features.gni
File build/config/features.gni (right):

https://codereview.chromium.org/2467873004/diff/300001/build/config/features....
build/config/features.gni:86: enable_webvr = is_android && (current_cpu == "arm"
|| current_cpu == "arm64")
Can we put back is_component_build for now? Otherwise this change has the effect
of turning on webvr for release builds, which I think deserves a separate
commit.

https://codereview.chromium.org/2467873004/diff/300001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrCoreVersionCheckerImpl.java
(right):

https://codereview.chromium.org/2467873004/diff/300001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrCoreVersionCheckerImpl.java:26:
public VrCoreVersionCheckerImpl(Context context) {
Rather than pass in the context, I'd guess this could just use
ContextUtil.getApplicationContext()

https://codereview.chromium.org/2467873004/diff/300001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
(right):

https://codereview.chromium.org/2467873004/diff/300001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:88:
mVrCoreVersionCheckerClass = (Class<? extends VrCoreVersionChecker>)
Class.forName(
Lot of reflection here... How about changing this to use a builder, so that only
a single class needs to be created via reflection (the builder)?

https://codereview.chromium.org/2467873004/diff/300001/third_party/gvr-androi...
File third_party/gvr-android-sdk/display_synchronizer_jni.h (right):

https://codereview.chromium.org/2467873004/diff/300001/third_party/gvr-androi...
third_party/gvr-android-sdk/display_synchronizer_jni.h:11: // 1. Remove all
implementaiton, only keep definition.
I think it would be better to add a flag to the jni_generator.py to make it
output this file how you'd like it. Otherwise, this will fall out-of-date with
future changes to the generator.

https://codereview.chromium.org/2467873004/diff/300001/third_party/gvr-androi...
third_party/gvr-android-sdk/display_synchronizer_jni.h:131: bool
RegisterDisplaySynchronizerNatives(JNIEnv* env) {
As a non-static function, this shouldn't be in a .h file.

[bshe, 2016-11-10 20:55:57]

Patchset #16 (id:320001) has been deleted

[bshe, 2016-11-10 20:59:24]

bshe@chromium.org changed reviewers:
- thestig@chromium.org

[bshe, 2016-11-10 20:59:25]

-Lei Zhang as I have changed the DEPS to a more specific rules
Also address Andrew's review.

https://codereview.chromium.org/2467873004/diff/300001/build/config/features.gni
File build/config/features.gni (right):

https://codereview.chromium.org/2467873004/diff/300001/build/config/features....
build/config/features.gni:86: enable_webvr = is_android && (current_cpu == "arm"
|| current_cpu == "arm64")
On 2016/11/10 19:16:29, agrieve wrote:
> Can we put back is_component_build for now? Otherwise this change has the
effect
> of turning on webvr for release builds, which I think deserves a separate
> commit.

Done.
A follow up CL will simply remove the is_component_build I assume. So if there
is
a problem, a revert is easier and less intrusive?

https://codereview.chromium.org/2467873004/diff/300001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrCoreVersionCheckerImpl.java
(right):

https://codereview.chromium.org/2467873004/diff/300001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrCoreVersionCheckerImpl.java:26:
public VrCoreVersionCheckerImpl(Context context) {
On 2016/11/10 19:16:29, agrieve wrote:
> Rather than pass in the context, I'd guess this could just use
> ContextUtil.getApplicationContext()

Done.

https://codereview.chromium.org/2467873004/diff/300001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
(right):

https://codereview.chromium.org/2467873004/diff/300001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:88:
mVrCoreVersionCheckerClass = (Class<? extends VrCoreVersionChecker>)
Class.forName(
On 2016/11/10 19:16:29, agrieve wrote:
> Lot of reflection here... How about changing this to use a builder, so that
only
> a single class needs to be created via reflection (the builder)?

Agree that we shouldn't have so many reflections. It makes code ugly. All these
reflections should be gone once webvr is officially turned on. crbug.com/664269
is created to track it.

https://codereview.chromium.org/2467873004/diff/300001/third_party/gvr-androi...
File third_party/gvr-android-sdk/DEPS (right):

https://codereview.chromium.org/2467873004/diff/300001/third_party/gvr-androi...
third_party/gvr-android-sdk/DEPS:2: '+base',
On 2016/11/10 18:54:35, Lei Zhang (slow) wrote:
> You only need base/android, right? You can add that, or just the 3 headers you
> need, and ask base/android/OWNERS for a review.

Done.

https://codereview.chromium.org/2467873004/diff/300001/third_party/gvr-androi...
File third_party/gvr-android-sdk/display_synchronizer_jni.h (right):

https://codereview.chromium.org/2467873004/diff/300001/third_party/gvr-androi...
third_party/gvr-android-sdk/display_synchronizer_jni.h:11: // 1. Remove all
implementaiton, only keep definition.
On 2016/11/10 19:16:29, agrieve wrote:
> I think it would be better to add a flag to the jni_generator.py to make it
> output this file how you'd like it. Otherwise, this will fall out-of-date with
> future changes to the generator.

Do you see it as a blocking issue for this CL? Would it make sense to do it in a
separate CL? It may take me a long time to do so as I am not familiar with
python. I would prefer to not block this CL on it if possible. Will create a
crbug if you think it is fine to do it in a
follow up CL.

https://codereview.chromium.org/2467873004/diff/300001/third_party/gvr-androi...
third_party/gvr-android-sdk/display_synchronizer_jni.h:131: bool
RegisterDisplaySynchronizerNatives(JNIEnv* env) {
On 2016/11/10 19:16:30, agrieve wrote:
> As a non-static function, this shouldn't be in a .h file.

Changed it to static. It is simply a wrapper.

[bajones, 2016-11-10 21:35:16]

gvr-android-sdk/ and vr-shell/ LGTM

[agrieve, 2016-11-10 21:47:05]

lgtm

https://codereview.chromium.org/2467873004/diff/300001/build/config/features.gni
File build/config/features.gni (right):

https://codereview.chromium.org/2467873004/diff/300001/build/config/features....
build/config/features.gni:86: enable_webvr = is_android && (current_cpu == "arm"
|| current_cpu == "arm64")
On 2016/11/10 20:59:25, bshe wrote:
> On 2016/11/10 19:16:29, agrieve wrote:
> > Can we put back is_component_build for now? Otherwise this change has the
> effect
> > of turning on webvr for release builds, which I think deserves a separate
> > commit.
> 
> Done.
> A follow up CL will simply remove the is_component_build I assume. So if there
> is
> a problem, a revert is easier and less intrusive?

Right.

https://codereview.chromium.org/2467873004/diff/300001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
(right):

https://codereview.chromium.org/2467873004/diff/300001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:88:
mVrCoreVersionCheckerClass = (Class<? extends VrCoreVersionChecker>)
Class.forName(
On 2016/11/10 20:59:25, bshe wrote:
> On 2016/11/10 19:16:29, agrieve wrote:
> > Lot of reflection here... How about changing this to use a builder, so that
> only
> > a single class needs to be created via reflection (the builder)?
> 
> Agree that we shouldn't have so many reflections. It makes code ugly. All
these
> reflections should be gone once webvr is officially turned on.
crbug.com/664269
> is created to track it.

SG - this should be removed then when you remove the is_component_build
condition then?

https://codereview.chromium.org/2467873004/diff/300001/third_party/gvr-androi...
File third_party/gvr-android-sdk/display_synchronizer_jni.h (right):

https://codereview.chromium.org/2467873004/diff/300001/third_party/gvr-androi...
third_party/gvr-android-sdk/display_synchronizer_jni.h:11: // 1. Remove all
implementaiton, only keep definition.
On 2016/11/10 20:59:25, bshe wrote:
> On 2016/11/10 19:16:29, agrieve wrote:
> > I think it would be better to add a flag to the jni_generator.py to make it
> > output this file how you'd like it. Otherwise, this will fall out-of-date
with
> > future changes to the generator.
> 
> Do you see it as a blocking issue for this CL? Would it make sense to do it in
a
> separate CL? It may take me a long time to do so as I am not familiar with
> python. I would prefer to not block this CL on it if possible. Will create a
> crbug if you think it is fine to do it in a
> follow up CL.

That's fine. Please file a bug for a follow-up.

[bshe, 2016-11-11 17:43:03]

https://codereview.chromium.org/2467873004/diff/300001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
(right):

https://codereview.chromium.org/2467873004/diff/300001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:88:
mVrCoreVersionCheckerClass = (Class<? extends VrCoreVersionChecker>)
Class.forName(
On 2016/11/10 21:47:04, agrieve wrote:
> On 2016/11/10 20:59:25, bshe wrote:
> > On 2016/11/10 19:16:29, agrieve wrote:
> > > Lot of reflection here... How about changing this to use a builder, so
that
> > only
> > > a single class needs to be created via reflection (the builder)?
> > 
> > Agree that we shouldn't have so many reflections. It makes code ugly. All
> these
> > reflections should be gone once webvr is officially turned on.
> crbug.com/664269
> > is created to track it.
> 
> SG - this should be removed then when you remove the is_component_build
> condition then?

hum. Probably needs to be after x86 and x64 support. I am not sure if mipsel
arch
actually compiles this code. If it does, I will need to keep reflection as we
won't ship webvr
to mipsel as far as I know(gvr doesnt support mipsel either). In that case, I
will refactor the
code to use build patten as you suggested. I will make a note to the bug.

https://codereview.chromium.org/2467873004/diff/300001/third_party/gvr-androi...
File third_party/gvr-android-sdk/display_synchronizer_jni.h (right):

https://codereview.chromium.org/2467873004/diff/300001/third_party/gvr-androi...
third_party/gvr-android-sdk/display_synchronizer_jni.h:11: // 1. Remove all
implementaiton, only keep definition.
On 2016/11/10 21:47:04, agrieve wrote:
> On 2016/11/10 20:59:25, bshe wrote:
> > On 2016/11/10 19:16:29, agrieve wrote:
> > > I think it would be better to add a flag to the jni_generator.py to make
it
> > > output this file how you'd like it. Otherwise, this will fall out-of-date
> with
> > > future changes to the generator.
> > 
> > Do you see it as a blocking issue for this CL? Would it make sense to do it
in
> a
> > separate CL? It may take me a long time to do so as I am not familiar with
> > python. I would prefer to not block this CL on it if possible. Will create a
> > crbug if you think it is fine to do it in a
> > follow up CL.
> 
> That's fine. Please file a bug for a follow-up.

crbug.com/664306 is created.

[David Trainor- moved to gerrit, 2016-11-11 18:00:10]

lgtm with a few nits.

https://codereview.chromium.org/2467873004/diff/340001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
(right):

https://codereview.chromium.org/2467873004/diff/340001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:94:
mVrDaydreamApiClass = null;
Set mVrCoreVersionCheckerClass to null here too for consistency?

https://codereview.chromium.org/2467873004/diff/340001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:298:
mVrCoreVersionCheckerClass.getConstructor();
Can we check if this is null as well?  I know it'll be caught by
"maybeFindVrClasses()" but someone changing that code above could easily not
update this code here.

[bshe, 2016-11-11 19:33:16]

https://codereview.chromium.org/2467873004/diff/340001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
(right):

https://codereview.chromium.org/2467873004/diff/340001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:94:
mVrDaydreamApiClass = null;
On 2016/11/11 18:00:10, David Trainor wrote:
> Set mVrCoreVersionCheckerClass to null here too for consistency?

Done.

https://codereview.chromium.org/2467873004/diff/340001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:298:
mVrCoreVersionCheckerClass.getConstructor();
On 2016/11/11 18:00:10, David Trainor wrote:
> Can we check if this is null as well?  I know it'll be caught by
> "maybeFindVrClasses()" but someone changing that code above could easily not
> update this code here.

Done.

[bshe, 2016-11-11 22:02:42]

The CQ bit was checked by bshe@chromium.org

[bshe, 2016-11-11 22:02:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org, bajones@chromium.org, agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/2467873004/#ps380001
(title: "rebase")

[commit-bot: I haz the power, 2016-11-11 22:03:26]

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

[commit-bot: I haz the power, 2016-11-11 22:15:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-11 22:15:06]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[bshe, 2016-11-11 22:28:12]

bshe@chromium.org changed reviewers:
+ thestig@chromium.org

[bshe, 2016-11-11 22:28:13]

+thestig again.
Sorry, it turns out that I need owner for
chrome/browser/DEPS
It looks like no one in chrome/browser/OWNER could review DEPS file. So I added
you as OWNER. Hopefully it is right. Thanks!

[Lei Zhang, 2016-11-11 22:31:54]

https://codereview.chromium.org/2467873004/diff/380001/chrome/browser/DEPS
File chrome/browser/DEPS (right):

https://codereview.chromium.org/2467873004/diff/380001/chrome/browser/DEPS#ne...
chrome/browser/DEPS:80: "+third_party/gvr-android-sdk",
Can this move into chrome/browser/android/DEPS ? i.e. Will any other part of
chrome/browser need access to this SDK?

[bshe, 2016-11-11 22:39:19]

https://codereview.chromium.org/2467873004/diff/380001/chrome/browser/DEPS
File chrome/browser/DEPS (right):

https://codereview.chromium.org/2467873004/diff/380001/chrome/browser/DEPS#ne...
chrome/browser/DEPS:80: "+third_party/gvr-android-sdk",
On 2016/11/11 22:31:54, Lei Zhang (slow) wrote:
> Can this move into chrome/browser/android/DEPS ? i.e. Will any other part of
> chrome/browser need access to this SDK?

dooh. You are absolutely right. It is Android only. Sorry to bug twice for the
same
issue. Will remove you from reviewer list again after this email.

[bshe, 2016-11-11 22:39:53]

bshe@chromium.org changed reviewers:
- thestig@chromium.org

[bshe, 2016-11-11 22:40:19]

The CQ bit was checked by bshe@chromium.org

[bshe, 2016-11-11 22:40:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org, bajones@chromium.org, agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/2467873004/#ps400001
(title: "Move DEPS to android")

[commit-bot: I haz the power, 2016-11-11 22:41:17]

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

[Lei Zhang, 2016-11-11 22:50:00]

On 2016/11/11 22:39:19, bshe wrote:
> dooh. You are absolutely right. It is Android only. Sorry to bug twice for the
> same
> issue. Will remove you from reviewer list again after this email.

No worries, happy to help.

[commit-bot: I haz the power, 2016-11-12 00:25:25]

Description was changed from

==========
Linking arm and arm64 gvr static shim library.

This should reduce our binary size increase significantly for VR
related feature, as well as avoid issue with crazy linker which has
problem handling two shared libraries.

Note that the static library is mostly a shim lib. It will dynamically
load the correct library from VrCore service at runtime when needed.
Our initial target is Pixel and Pixel XL phones. And VrCore should exist
in both phones. If VrCore doesn't exist or the version is older than we
expect, VR will be disabled. Support for x86 and x64 will be added later.


BUG=644785
==========

to

==========
Linking arm and arm64 gvr static shim library.

This should reduce our binary size increase significantly for VR
related feature, as well as avoid issue with crazy linker which has
problem handling two shared libraries.

Note that the static library is mostly a shim lib. It will dynamically
load the correct library from VrCore service at runtime when needed.
Our initial target is Pixel and Pixel XL phones. And VrCore should exist
in both phones. If VrCore doesn't exist or the version is older than we
expect, VR will be disabled. Support for x86 and x64 will be added later.


BUG=644785
==========

[commit-bot: I haz the power, 2016-11-12 00:25:26]

Committed patchset #19 (id:400001)

[commit-bot: I haz the power, 2016-11-12 00:30:03]

Description was changed from

==========
Linking arm and arm64 gvr static shim library.

This should reduce our binary size increase significantly for VR
related feature, as well as avoid issue with crazy linker which has
problem handling two shared libraries.

Note that the static library is mostly a shim lib. It will dynamically
load the correct library from VrCore service at runtime when needed.
Our initial target is Pixel and Pixel XL phones. And VrCore should exist
in both phones. If VrCore doesn't exist or the version is older than we
expect, VR will be disabled. Support for x86 and x64 will be added later.


BUG=644785
==========

to

==========
Linking arm and arm64 gvr static shim library.

This should reduce our binary size increase significantly for VR
related feature, as well as avoid issue with crazy linker which has
problem handling two shared libraries.

Note that the static library is mostly a shim lib. It will dynamically
load the correct library from VrCore service at runtime when needed.
Our initial target is Pixel and Pixel XL phones. And VrCore should exist
in both phones. If VrCore doesn't exist or the version is older than we
expect, VR will be disabled. Support for x86 and x64 will be added later.

BUG=644785

Committed: https://crrev.com/0907c2d02be6980427e4ecc372c47985e7dddffd
Cr-Commit-Position: refs/heads/master@{#431699}
==========

[commit-bot: I haz the power, 2016-11-12 00:30:04]

Patchset 19 (id:??) landed as
https://crrev.com/0907c2d02be6980427e4ecc372c47985e7dddffd
Cr-Commit-Position: refs/heads/master@{#431699}