Android WebVR implementation, Cardboard 0.5.5 SDK

Android WebVR implementation, Cardboard 0.5.5 SDK

Previously part of https://codereview.chromium.org/829803003/

BUG=389343
Committed: https://crrev.com/1d9c45547709e77d599c218d76317895389db129
Cr-Commit-Position: refs/heads/master@{#342496}

Committed: https://crrev.com/d9be58be03170c7c82f9c55abc2d21f0f7447c7f
Cr-Commit-Position: refs/heads/master@{#345356}

[bajones, 2015-06-23 20:06:30]

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

[commit-bot: I haz the power, 2015-06-23 20:09:10]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1200303002/1

[commit-bot: I haz the power, 2015-06-23 20:19:07]

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

[commit-bot: I haz the power, 2015-06-23 20:19:08]

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

[bajones, 2015-07-21 17:44:44]

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

[commit-bot: I haz the power, 2015-07-21 17:45:25]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1200303002/20001

[commit-bot: I haz the power, 2015-07-21 17:53:20]

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

[commit-bot: I haz the power, 2015-07-21 17:53:20]

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

[bajones, 2015-07-21 20:07:06]

bajones@chromium.org changed reviewers:
+ jdduke@chromium.org, jochen@chromium.org, sievers@chromium.org,
tedchoc@chromium.org

[bajones, 2015-07-21 20:07:07]

All of the code in this CL has previously been LGTMed as part of
https://codereview.chromium.org/829803003/, but was split out when some
incompatibilities between the Cardboard SDK and Android M were discovered. The
Cardboard SDK has been updated and rolled, which should resolve this issue, so
I'm taking another crack at landing this.

Please take another look at the code and let me know if you see any issues that
slipped by previous reviews! Thanks!

tedchoc@/jduke@: Please re-review the Android-side code
jochen@: Please re-review build/
sievers@: Could you please take another look at content/ in general?

[jdduke (slow), 2015-07-21 20:36:27]

Just some minor comments, you've probably addressed them and I've since
forgotten (and was too lazy to scan the previous review =/). lgtm per previous
review (I'm assuming this is still experimental? is it behind a flag?)

https://codereview.chromium.org/1200303002/diff/20001/content/browser/vr/andr...
File content/browser/vr/android/cardboard/cardboard_vr_device.cc (right):

https://codereview.chromium.org/1200303002/diff/20001/content/browser/vr/andr...
content/browser/vr/android/cardboard/cardboard_vr_device.cc:29: float root;
Hmm, there's some transform->quaternion code in ui/gfx/transform_util.cc, not
sure if it's worth pulling that into a utility routing and re-using? Don't have
a strong preference.

https://codereview.chromium.org/1200303002/diff/20001/content/browser/vr/andr...
content/browser/vr/android/cardboard/cardboard_vr_device.cc:98:
VREyeParametersPtr& leftEye = hmdInfo->leftEye;
I guess calling this leftEye (instead of left_eye) keeps the style from the
referenced variable, I'm OK with it not sure about our style policy though.

https://codereview.chromium.org/1200303002/diff/20001/content/browser/vr/andr...
content/browser/vr/android/cardboard/cardboard_vr_device.cc:141:
leftEye->renderRect = VRRect::New();
Just confirming, looks like Mojo does some fanciness here so we're not actually
heap allocating?

https://codereview.chromium.org/1200303002/diff/20001/content/browser/vr/andr...
content/browser/vr/android/cardboard/cardboard_vr_device.cc:143:
leftEye->renderRect->y = 0;
If the orientation changes (or the window dimensions change for whatever other
reason), do we recreate the device?

https://codereview.chromium.org/1200303002/diff/20001/content/browser/vr/andr...
content/browser/vr/android/cardboard/cardboard_vr_device.cc:159:
state->timestamp = base::Time::Now().ToJsTime();
Hmm, do they really want wallclock time and not system time? (I may have asked
this before =/). For other input events we populate the timestamp with
TimeTicks::Now(), though Blink eventually ignores that and feeds Time::Now() to
the the web-exposed event.

https://codereview.chromium.org/1200303002/diff/20001/content/browser/vr/andr...
content/browser/vr/android/cardboard/cardboard_vr_device.cc:163:
ScopedJavaLocalRef<jfloatArray> j_head_matrix(env, env->NewFloatArray(16));
Have you considered storing a persistent Java array in this object, and re-using
it? Would save garbage generation each frame.

https://codereview.chromium.org/1200303002/diff/20001/content/browser/vr/andr...
content/browser/vr/android/cardboard/cardboard_vr_device.cc:178: frame_index_++;
Maybe increment this up where it's used?

[bajones, 2015-07-21 23:27:30]

Thanks for the feedback! Made some tweaks to the code.

https://codereview.chromium.org/1200303002/diff/20001/content/browser/vr/andr...
File content/browser/vr/android/cardboard/cardboard_vr_device.cc (right):

https://codereview.chromium.org/1200303002/diff/20001/content/browser/vr/andr...
content/browser/vr/android/cardboard/cardboard_vr_device.cc:29: float root;
On 2015/07/21 20:36:27, jdduke wrote:
> Hmm, there's some transform->quaternion code in ui/gfx/transform_util.cc, not
> sure if it's worth pulling that into a utility routing and re-using? Don't
have
> a strong preference.

Might be worth doing, though I would probably defer to another patch.

https://codereview.chromium.org/1200303002/diff/20001/content/browser/vr/andr...
content/browser/vr/android/cardboard/cardboard_vr_device.cc:98:
VREyeParametersPtr& leftEye = hmdInfo->leftEye;
On 2015/07/21 20:36:27, jdduke wrote:
> I guess calling this leftEye (instead of left_eye) keeps the style from the
> referenced variable, I'm OK with it not sure about our style policy though.

Changed it to left_eye/right_eye anyway.

https://codereview.chromium.org/1200303002/diff/20001/content/browser/vr/andr...
content/browser/vr/android/cardboard/cardboard_vr_device.cc:143:
leftEye->renderRect->y = 0;
On 2015/07/21 20:36:27, jdduke wrote:
> If the orientation changes (or the window dimensions change for whatever other
> reason), do we recreate the device?

For the purposes of VR it's assumed that you will always render stereo scenes in
landscape (which is how your phone goes into Cardboard).

https://codereview.chromium.org/1200303002/diff/20001/content/browser/vr/andr...
content/browser/vr/android/cardboard/cardboard_vr_device.cc:159:
state->timestamp = base::Time::Now().ToJsTime();
On 2015/07/21 20:36:27, jdduke wrote:
> Hmm, do they really want wallclock time and not system time? (I may have asked
> this before =/). For other input events we populate the timestamp with
> TimeTicks::Now(), though Blink eventually ignores that and feeds Time::Now()
to
> the the web-exposed event.

This is an area of the WebVR spec that's underdeveloped, as right now the
concept of a timestamp is just whatever the VR SDK most conveniently provides. I
need to tighten that down with the Mozilla folks, but until then this should be
sufficient.

https://codereview.chromium.org/1200303002/diff/20001/content/browser/vr/andr...
content/browser/vr/android/cardboard/cardboard_vr_device.cc:163:
ScopedJavaLocalRef<jfloatArray> j_head_matrix(env, env->NewFloatArray(16));
On 2015/07/21 20:36:27, jdduke wrote:
> Have you considered storing a persistent Java array in this object, and
re-using
> it? Would save garbage generation each frame.

Done

https://codereview.chromium.org/1200303002/diff/20001/content/browser/vr/andr...
content/browser/vr/android/cardboard/cardboard_vr_device.cc:178: frame_index_++;
On 2015/07/21 20:36:27, jdduke wrote:
> Maybe increment this up where it's used?

Done

[bajones, 2015-07-21 23:30:20]

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

[commit-bot: I haz the power, 2015-07-21 23:31:06]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1200303002/40001

[commit-bot: I haz the power, 2015-07-22 04:02:21]

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

[commit-bot: I haz the power, 2015-07-22 04:02:22]

Dry run: This issue passed the CQ dry run.

[jochen (gone - plz use gerrit), 2015-07-22 09:38:56]

https://codereview.chromium.org/1200303002/diff/40001/content/browser/vr/andr...
File content/browser/vr/android/cardboard/cardboard_vr_device.cc (right):

https://codereview.chromium.org/1200303002/diff/40001/content/browser/vr/andr...
content/browser/vr/android/cardboard/cardboard_vr_device.cc:26: VRVector4Ptr
MatrixToOrientationQuat(const float m[16]) {
can we add a unit test for this?

https://codereview.chromium.org/1200303002/diff/40001/content/browser/vr/andr...
File content/browser/vr/android/cardboard/cardboard_vr_device_provider.h
(right):

https://codereview.chromium.org/1200303002/diff/40001/content/browser/vr/andr...
content/browser/vr/android/cardboard/cardboard_vr_device_provider.h:22: void
GetDevices(std::vector<VRDevice*>& devices) override;
devices should be a pointer

[bajones, 2015-07-22 20:28:17]

On 2015/07/22 09:38:56, jochen wrote:
>
https://codereview.chromium.org/1200303002/diff/40001/content/browser/vr/andr...
> content/browser/vr/android/cardboard/cardboard_vr_device.cc:26: VRVector4Ptr
> MatrixToOrientationQuat(const float m[16]) {
> can we add a unit test for this?

Done. Tweaked the signature a bit and moved it out into a utils file as well,
since future VR backends will need this function as well.

>
https://codereview.chromium.org/1200303002/diff/40001/content/browser/vr/andr...
> content/browser/vr/android/cardboard/cardboard_vr_device_provider.h:22: void
> GetDevices(std::vector<VRDevice*>& devices) override;
> devices should be a pointer

Done.

[jochen (gone - plz use gerrit), 2015-07-23 09:16:43]

lgtm

[Ted C, 2015-07-23 18:45:27]

On 2015/07/23 09:16:43, jochen wrote:
> lgtm

Per jdduke's comment, this seems to be enabled by default for android with
this change.

Is there a launch bug for this?  The same yada yada about apk size and 
additional methods.  Would be good to get something tracking this at the
browser level.

[bajones, 2015-07-23 19:07:02]

On 2015/07/23 18:45:27, Ted C wrote:
> On 2015/07/23 09:16:43, jochen wrote:
> > lgtm
> 
> Per jdduke's comment, this seems to be enabled by default for android with
> this change.
> 
> Is there a launch bug for this?  The same yada yada about apk size and 
> additional methods.  Would be good to get something tracking this at the
> browser level.

This is not enabled by default. It's behind the enable-webvr flag, which was
added in https://codereview.chromium.org/829803003/ and defaults to false. It
currently adds 42K to the APK (Down from 52k at last check, likely due to
Cardboard SDK internal changes). What sort of browser-level tracking are you
looking for?

[jdduke (slow), 2015-07-23 19:10:57]

https://codereview.chromium.org/1200303002/diff/60001/content/browser/vr/andr...
File content/browser/vr/android/cardboard/cardboard_vr_device.cc (right):

https://codereview.chromium.org/1200303002/diff/60001/content/browser/vr/andr...
content/browser/vr/android/cardboard/cardboard_vr_device.cc:41: VRDeviceInfoPtr
device = VRDeviceInfo::New();
Oh, could you add a TRACE_EVENT entry for this function and GetSensorState (or
is there tracing at a slightly higher level in the VR stack?). Not sure what the
best category would be, maybe "input" or "browser"?

[bajones, 2015-07-24 19:52:01]

On 2015/07/23 19:10:57, jdduke wrote:
> Oh, could you add a TRACE_EVENT entry for this function and GetSensorState (or
> is there tracing at a slightly higher level in the VR stack?). Not sure what
the
> best category would be, maybe "input" or "browser"?

Good idea, done. Chose "input" because it is indeed a form of input.

[jdduke (slow), 2015-07-24 22:15:52]

On 2015/07/24 19:52:01, bajones wrote:
> On 2015/07/23 19:10:57, jdduke wrote:
> > Oh, could you add a TRACE_EVENT entry for this function and GetSensorState
(or
> > is there tracing at a slightly higher level in the VR stack?). Not sure what
> the
> > best category would be, maybe "input" or "browser"?
> 
> Good idea, done. Chose "input" because it is indeed a form of input.

Thanks, still lgtm.

[bajones, 2015-07-24 22:29:11]

On 2015/07/24 22:15:52, jdduke wrote:
> Thanks, still lgtm.

Thank you! tedchoc@, I'd still appreciate a LG from you before I commit this.
Any remaining concerns?

[Ted C, 2015-07-24 22:40:48]

On 2015/07/23 19:07:02, bajones wrote:
> On 2015/07/23 18:45:27, Ted C wrote:
> > On 2015/07/23 09:16:43, jochen wrote:
> > > lgtm
> > 
> > Per jdduke's comment, this seems to be enabled by default for android with
> > this change.
> > 
> > Is there a launch bug for this?  The same yada yada about apk size and 
> > additional methods.  Would be good to get something tracking this at the
> > browser level.
> 
> This is not enabled by default. It's behind the enable-webvr flag, which was
> added in https://codereview.chromium.org/829803003/ and defaults to false. It
> currently adds 42K to the APK (Down from 52k at last check, likely due to
> Cardboard SDK internal changes). What sort of browser-level tracking are you
> looking for?

The launch review would be for APK size increase.

In particular, this change:
https://codereview.chromium.org/1200303002/patch/80001/90001

To me, that looks like it means that regardless of the command line flag,
we are including the jar and associated bits on all versions of Chrome going
forward.

It would be good to have sign off that the extra size increase is indeed
accepted (kerz@ would be the person to appease).

[kerz_chromium, 2015-07-24 22:48:32]

On 2015/07/24 22:40:48, Ted C wrote:
> On 2015/07/23 19:07:02, bajones wrote:
> > On 2015/07/23 18:45:27, Ted C wrote:
> > > On 2015/07/23 09:16:43, jochen wrote:
> > > > lgtm
> > > 
> > > Per jdduke's comment, this seems to be enabled by default for android with
> > > this change.
> > > 
> > > Is there a launch bug for this?  The same yada yada about apk size and 
> > > additional methods.  Would be good to get something tracking this at the
> > > browser level.
> > 
> > This is not enabled by default. It's behind the enable-webvr flag, which was
> > added in https://codereview.chromium.org/829803003/ and defaults to false.
It
> > currently adds 42K to the APK (Down from 52k at last check, likely due to
> > Cardboard SDK internal changes). What sort of browser-level tracking are you
> > looking for?
> 
> The launch review would be for APK size increase.
> 
> In particular, this change:
> https://codereview.chromium.org/1200303002/patch/80001/90001
> 
> To me, that looks like it means that regardless of the command line flag,
> we are including the jar and associated bits on all versions of Chrome going
> forward.
> 
> It would be good to have sign off that the extra size increase is indeed
> accepted (kerz@ would be the person to appease).

Have we done a launch review for this, or a web platform review? Given this is
launching new features/functionality, we should do at least one of these. I
guess the size increase is pretty small, but it's unclear who's going to
actually use this, and how often. Seems pretty niche.

[bajones, 2015-07-24 23:00:48]

On 2015/07/24 22:40:48, Ted C wrote:
> The launch review would be for APK size increase.
> 
> In particular, this change:
> https://codereview.chromium.org/1200303002/patch/80001/90001
> 
> To me, that looks like it means that regardless of the command line flag,
> we are including the jar and associated bits on all versions of Chrome going
> forward.

Ah, thanks for clarifying. Yes, the WebVR code would be in the build by default
with this change. It would not actually be accessible to the web (what I meant
by "enabled") without the flag.
 
On 2015/07/24 22:48:32, kerz_chromium wrote:
> Have we done a launch review for this, or a web platform review? Given this is
> launching new features/functionality, we should do at least one of these. I
> guess the size increase is pretty small, but it's unclear who's going to
> actually use this, and how often. Seems pretty niche.

I've done a review with chrome-eng-review (Unfortunately I can't provide a link,
since the group is private.) I'm not sure if that covers the case you're looking
for?

Also, I'd like some clarity on what is considered "launching" a feature. The
intent of making this accessible in Chrome behind a flag is to allow developers
to experiment with the feature and provide feedback. It's not going to be
exposed to the general web for a long time while I and Mozilla continue to
iterate on the design.

[kerz_chromium, 2015-07-27 18:07:51]

On 2015/07/24 23:00:48, bajones wrote:
> On 2015/07/24 22:40:48, Ted C wrote:
> > The launch review would be for APK size increase.
> > 
> > In particular, this change:
> > https://codereview.chromium.org/1200303002/patch/80001/90001
> > 
> > To me, that looks like it means that regardless of the command line flag,
> > we are including the jar and associated bits on all versions of Chrome going
> > forward.
> 
> Ah, thanks for clarifying. Yes, the WebVR code would be in the build by
default
> with this change. It would not actually be accessible to the web (what I meant
> by "enabled") without the flag.
>  
> On 2015/07/24 22:48:32, kerz_chromium wrote:
> > Have we done a launch review for this, or a web platform review? Given this
is
> > launching new features/functionality, we should do at least one of these. I
> > guess the size increase is pretty small, but it's unclear who's going to
> > actually use this, and how often. Seems pretty niche.
> 
> I've done a review with chrome-eng-review (Unfortunately I can't provide a
link,
> since the group is private.) I'm not sure if that covers the case you're
looking
> for?
> 
> Also, I'd like some clarity on what is considered "launching" a feature. The
> intent of making this accessible in Chrome behind a flag is to allow
developers
> to experiment with the feature and provide feedback. It's not going to be
> exposed to the general web for a long time while I and Mozilla continue to
> iterate on the design.

Details are at http://goto.ext.google.com/newChromeFeature.

[bajones, 2015-07-27 18:43:31]

On 2015/07/27 18:07:51, kerz_chromium wrote:
> Details are at http://goto.ext.google.com/newChromeFeature.

Thanks for the link. You'll have to forgive my unfamiliarity with the current
process, I've been working on this feature in some capacity for over a year (see
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/m32Yy87Ggt0)
and feel like the approval process has been shifting under my feet.

I do have a "launch bug"
(https://code.google.com/p/chromium/issues/detail?id=389343) but as it was
created back in June of 2014 it doesn't follow the format prescribed by
go/newChromeFeature. Would you recommend a new bug be created with the more
recent template?

I believe I'm at the "Develop and Dogfood" stage of the document, which seems to
precede the launch review. It's a little confusing, however, since the document
seems geared more towards UI or other user-facing changes rather than new
developer APIs. (I'm not sure that "rolling out to 100% of Googlers" is a
meaningful thing in this case.) I could also see this possibly fitting under the
"Experimentation" stage, but again the talk of turning features on for XX% of
users doesn't really mean much here, since the feature will only be of interest
(currently) to a self-selecting group of developers.

I'd love to get some more clarity on the process at this point. Maybe it would
be easier to chat in person or over GVC briefly?

[bajones, 2015-07-29 18:39:42]

Just read through the updated go/newChromeFeature. My compliments to the authors
for making the process far more clear and less UI-centric!

In light of that I've created a launch bug (crbug.com/515192) so that when the
time comes we can track all the necessary approvals. I don't expect that to be
for a while, though. This feature is definitely still in the "Early development"
stage, and as such is hidden away behind the --enable-webvr flag (as suggested
by the document). For the record the original intent to implement email is here:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/m32Yy87Ggt0

Do you have any remaining concerns about this CL that I can address?

[bajones, 2015-08-03 21:05:50]

Post weekend review ping!

[bajones, 2015-08-05 20:29:46]

I haven't received any response on this CL for over a week, but technically have
all the LGTMs needed to land it and have previously had it reviewed and landed
it before. (Only rolling back due to a minor Android M incompatibility.) I also
feel that the most recent update to go/newChromeFeature establishes this feature
as not being in a state which requires a launch review. As such, to avoid
getting stuck in limbo I'm going to move forward with landing this. I still
welcome any comments, and am happy to halt the commit if there are further
concerns.

[bajones, 2015-08-05 20:29:59]

The CQ bit was checked by bajones@chromium.org

[bajones, 2015-08-05 20:30:00]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1200303002/#ps80001
(title: "Added Traces")

[commit-bot: I haz the power, 2015-08-05 20:31:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1200303002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1200303002/80001

[jdduke (slow), 2015-08-05 20:32:40]

On 2015/08/05 20:29:46, bajones wrote:
> I haven't received any response on this CL for over a week, but technically
have
> all the LGTMs needed to land it and have previously had it reviewed and landed
> it before. (Only rolling back due to a minor Android M incompatibility.) I
also
> feel that the most recent update to go/newChromeFeature establishes this
feature
> as not being in a state which requires a launch review. As such, to avoid
> getting stuck in limbo I'm going to move forward with landing this. I still
> welcome any comments, and am happy to halt the commit if there are further
> concerns.

tedchoc@ is on paternity leave, you might try reaching out to him directly but
we can always roll back if necessary I suppose.

[commit-bot: I haz the power, 2015-08-05 20:52:51]

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

[commit-bot: I haz the power, 2015-08-05 20:52:52]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  mac_chromium_10.10_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_10.1...)

[bajones, 2015-08-07 00:35:30]

The CQ bit was checked by bajones@chromium.org

[bajones, 2015-08-07 00:35:31]

The patchset sent to the CQ was uploaded after l-g-t-m from jdduke@chromium.org,
jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1200303002/#ps140001
(title: "Add missing files to BUILD.gn")

[commit-bot: I haz the power, 2015-08-07 00:37:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1200303002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1200303002/140001

[commit-bot: I haz the power, 2015-08-07 01:43:45]

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

[commit-bot: I haz the power, 2015-08-07 01:43:46]

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

[bajones, 2015-08-07 23:10:05]

The CQ bit was checked by bajones@chromium.org

[bajones, 2015-08-07 23:10:06]

The patchset sent to the CQ was uploaded after l-g-t-m from jdduke@chromium.org,
jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1200303002/#ps180001
(title: "Fix build dependencies")

[commit-bot: I haz the power, 2015-08-07 23:10:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1200303002/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1200303002/180001

[commit-bot: I haz the power, 2015-08-08 00:57:51]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-08-08 00:58:23]

Patchset 10 (id:??) landed as
https://crrev.com/1d9c45547709e77d599c218d76317895389db129
Cr-Commit-Position: refs/heads/master@{#342496}

[Nico, 2015-08-08 22:54:15]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2015-08-08 22:54:16]

https://codereview.chromium.org/1200303002/diff/180001/content/browser/vr/vr_...
File content/browser/vr/vr_transform_util.h (right):

https://codereview.chromium.org/1200303002/diff/180001/content/browser/vr/vr_...
content/browser/vr/vr_transform_util.h:12: VRVector4Ptr
MatrixToOrientationQuaternion(
This probably needs a CONTENT_EXPORT

[Nico, 2015-08-08 22:55:05]

On 2015/08/08 22:54:16, Nico (hiding) wrote:
>
https://codereview.chromium.org/1200303002/diff/180001/content/browser/vr/vr_...
> File content/browser/vr/vr_transform_util.h (right):
> 
>
https://codereview.chromium.org/1200303002/diff/180001/content/browser/vr/vr_...
> content/browser/vr/vr_transform_util.h:12: VRVector4Ptr
> MatrixToOrientationQuaternion(
> This probably needs a CONTENT_EXPORT

(Also, content/browser seems kind of like the wrong place for a math library…)

[bajones, 2015-08-08 23:13:10]

A revert of this CL (patchset #10 id:180001) has been created in
https://codereview.chromium.org/1274913004/ by bajones@chromium.org.

The reason for reverting is: Build config issue on ASAN bots..

[bajones, 2015-08-25 03:51:59]

The CQ bit was checked by bajones@chromium.org

[bajones, 2015-08-25 03:51:59]

The patchset sent to the CQ was uploaded after l-g-t-m from jdduke@chromium.org,
jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1200303002/#ps220001
(title: "Removed vr_transform_utils, using ui/gfx/transform_utils instead")

[commit-bot: I haz the power, 2015-08-25 03:52:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1200303002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1200303002/220001

[commit-bot: I haz the power, 2015-08-25 04:51:33]

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

[commit-bot: I haz the power, 2015-08-25 04:51:34]

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

[bajones, 2015-08-25 16:24:35]

The CQ bit was checked by bajones@chromium.org

[commit-bot: I haz the power, 2015-08-25 16:25:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1200303002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1200303002/220001

[commit-bot: I haz the power, 2015-08-25 16:59:59]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2015-08-25 17:00:31]

Patchset 12 (id:??) landed as
https://crrev.com/d9be58be03170c7c82f9c55abc2d21f0f7447c7f
Cr-Commit-Position: refs/heads/master@{#345356}

[bajones, 2015-09-01 16:38:04]

A revert of this CL (patchset #12 id:220001) has been created in
https://codereview.chromium.org/1321383002/ by bajones@chromium.org.

The reason for reverting is: Android team found an incompatibility with the
0.5.5 Cardboard SDK and non-ARM devices. Rolling back until the Cardboard team
can address that..