1465816 - Add Android support to gfxVRExternal

Status: RESOLVED FIXED

(Core :: WebVR, enhancement)

Description

[Randall Barker [:rbarker]]

gfxVRExternal needs Android support so it may be used by FxR

Comment 1

[Randall Barker [:rbarker]]

Attachment: Bug 1465816 - Add initial code needed to support gfxVRExternal on android

Review commit: https://reviewboard.mozilla.org/r/249142/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/249142/

Comment 2

[Randall Barker [:rbarker]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=62d43ad30c062c9f888053c6d20bf8987814c213&selectedJob=181816181

Comment 3

[:kip (Kearwood Gilbert)]

Comment on attachment 8983253 [details]
Bug 1465816 - Add initial code needed to support gfxVRExternal on android

https://reviewboard.mozilla.org/r/249142/#review255382

This LGTM, thanks!

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8983253 [details]
Bug 1465816 - Add initial code needed to support gfxVRExternal on android

https://reviewboard.mozilla.org/r/249142/#review255508

r+ for Java bits

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoVRManager.java:20
(Diff revision 1)
> -                }
> -            });
> -            return;
> -        }
> +    }
>  
> -        if (mGVRDelegate == null) {
> +    public static synchronized void setExternalContext(final long aExternalContext) {

No a-prefix for parameters in Java code

::: widget/android/GeckoVRManager.h:18
(Diff revision 1)
> -    : public mozilla::java::GeckoVRManager::Natives<mozilla::GeckoVRManager>
>  {
> -  typedef mozilla::java::GeckoVRManager::Natives<mozilla::GeckoVRManager> Super;
>  public:
> -  static void Init()
> -  {
> +  static void* GetExternalContext() {
> +    return (void*)mozilla::java::GeckoVRManager::GetExternalContext();

static_cast

Comment 5

[Randall Barker [:rbarker]]

(In reply to Jim Chen [:jchen] [:darchons] from comment #4)
> Comment on attachment 8983253 [details]
> ::: widget/android/GeckoVRManager.h:18
> (Diff revision 1)
> > -    : public mozilla::java::GeckoVRManager::Natives<mozilla::GeckoVRManager>
> >  {
> > -  typedef mozilla::java::GeckoVRManager::Natives<mozilla::GeckoVRManager> Super;
> >  public:
> > -  static void Init()
> > -  {
> > +  static void* GetExternalContext() {
> > +    return (void*)mozilla::java::GeckoVRManager::GetExternalContext();
> 
> static_cast

static_cast won't work on a jlong, so changed it to a reinterpret_cast.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/66a3146a7fce
Add initial code needed to support gfxVRExternal on android r=kip,jchen

Comment 7

[Cosmin Sabou [:CosminS]]

Backed out for frequent reftest failures on draw_rect.html.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&fromchange=66a3146a7fce62c513aa0fec48a0a4b62a839881&tochange=4b22431dee8391865ba5be637c12216148f124c6&filter-searchStr=reftest&selectedJob=181945216

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=181945216&repo=mozilla-inbound&lineNumber=29854

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/4b22431dee8391865ba5be637c12216148f124c6

Comment 8

[Randall Barker [:rbarker]]

I don't see how this patch is related. It is only for android and the pref isn't even enabled.

Comment 9

[Randall Barker [:rbarker]]

Okay, I think I see a potential problem. Will do a new try push.

Comment 10

[Randall Barker [:rbarker]]

Looks like it passes now:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6b6318c01c3e7493349615effd79e440f0a9c2f0

Will push updated patch once inbound opens again.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b077ffd9a84d
Add initial code needed to support gfxVRExternal on android r=kip,jchen

Comment 12

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/b077ffd9a84d