1395497 - WebGL video textures issues for Firefox 55 in android

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Audio/Video, defect)

Description

[Alaa El Boudali]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36

Steps to reproduce:

When visiting :

https://krpano.com/ios/bugs/ios8-webgl-video-performance/

The html5 video was playing normally on Firefox 54. Upon updating to Firefox 55 on android the video no longer works and displays a black screen. 


Actual results:

The video WebGL texture seems to no longer be playing on Firefox for Android. The sound is playing well, but the screen remains black.


Expected results:

The video WebGL texture should be able to be seen just.

Comment 1

[Sorina Florean [:sflorean]]

I was able to reproduce this on all branches with Oneplus Two (Android 6.0.1) and Motorola Nexus 6 (Android 7.1.1). Marking as New. 
Notes: on Chrome is not reproducible.

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1395497 - Modify ScopedBindTexture to allow save/restore

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

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1395497 - Create SurfaceTexture in detached state, attach on first use

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

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1395497 - Use mat3s to transform tex coords in GLBlitHelper. -

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

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1395497 - Regenerate JNI binding

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

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

These patches make the situation better. There are some caveats:

1) We do not attempt to handle the situation where both the compositor and canvas (either WebGL or 2d) are trying to consume a video. We know from past experience that trying to quickly switch between to GL contexts results in problems.

2) This won't work in E10s because we won't be able to access the SurfaceTexture in the content process. To fix that, we could create the SurfaceTexture for the video in the content process, then blit that to another SurfaceTexture that lives in the parent process.

Comment 7

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8912985 [details]
Bug 1395497 - Modify ScopedBindTexture to allow save/restore

https://reviewboard.mozilla.org/r/184346/#review189470

Modify ScopedBindTexture.

::: gfx/gl/ScopedGLHelpers.cpp:477
(Diff revision 1)
> +}
> +
> +void
> +ScopedTextureBindingState::UnwrapImpl()
> +{
> +    mGL->fBindTexture(mBinding, mValue);

This doesn't work at all!
LOCAL_GL_TEXTURE_BINDING_2D: 0x8069
LOCAL_GL_TEXTURE_2D: 0x0DE1

Comment 8

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8912986 [details]
Bug 1395497 - Create SurfaceTexture in detached state, attach on first use

https://reviewboard.mozilla.org/r/184348/#review189476

Comment 9

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8912987 [details]
Bug 1395497 - Use mat3s to transform tex coords in GLBlitHelper. -

https://reviewboard.mozilla.org/r/184350/#review189474

::: gfx/gl/GLBlitHelper.cpp:75
(Diff revision 1)
> +    uniform mat4 uTexTransform;                                              \n\
> +                                                                             \n\
> +    void main(void)                                                          \n\
> +    {                                                                        \n\
> +        FRAG_COLOR = TEXTURE(uTex0,                                          \n\
> +            (uTexTransform * vec4(vTexCoord0, 0.0, 1.0)).xy);                \n\

This should be in the vert shader.
We should just be uploading a matrix to the vs instead of the current mess of vec2s, though that does mean dealing with constructing matrices in c++.

I can do this for you.

::: gfx/gl/GLBlitHelper.cpp:687
(Diff revision 1)
> +
> +    ScopedTextureBindingState boundTex(mGL, LOCAL_GL_TEXTURE_BINDING_EXTERNAL);
> +    surfaceTexture->UpdateTexImage();
> +
> +    gfx::Matrix4x4 transform;
> +    AndroidSurfaceTexture::GetTransformMatrix(java::sdk::SurfaceTexture::Ref::From(surfaceTexture), transform);

GetTransformMatrix should take a gfx::Matrix4x4*, not gfx::Matrix4x4&.

Comment 10

[Jan Keromnes [:janx]]

Comment on attachment 8912985 [details]
Bug 1395497 - Modify ScopedBindTexture to allow save/restore

https://reviewboard.mozilla.org/r/184346/#review189658

Quick drive-by comments about two issues found by our static analysis bot.

::: gfx/gl/ScopedGLHelpers.h:344
(Diff revision 1)
> +protected:
> +    GLenum mBinding;
> +    GLuint mValue;
> +
> +public:
> +    ScopedTextureBindingState(GLContext* aGL, GLenum state = LOCAL_GL_TEXTURE_BINDING_2D);

Our static analysis bot found a problem at this line:

Error: Bad implicit conversion constructor for 'ScopedTextureBindingState' [clang-tidy: mozilla-implicit-constructor]
Note: Consider adding the explicit keyword to the constructor

::: gfx/gl/ScopedGLHelpers.cpp:467
(Diff revision 1)
>  }
>  
>  ////////////////////////////////////////////////////////////////////////
> +// ScopedTextureBindingState
> +
> +ScopedTextureBindingState::ScopedTextureBindingState(GLContext* gl, GLenum binding)

Our static analysis bot found a problem at this line:

Error: Bad implicit conversion constructor for 'ScopedTextureBindingState' [clang-tidy: mozilla-implicit-constructor]
Note: Consider adding the explicit keyword to the constructor

Comment 11

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912985 [details]
Bug 1395497 - Modify ScopedBindTexture to allow save/restore

https://reviewboard.mozilla.org/r/184346/#review189470

Ok, do you have a suggestion on what you would like to see? Maybe another constructor with no texture name for the save/restore case?

> This doesn't work at all!
> LOCAL_GL_TEXTURE_BINDING_2D: 0x8069
> LOCAL_GL_TEXTURE_2D: 0x0DE1

Whoops! You're right, of course. That's what I get for trying to clean up the manual fGetIntegerv/fBindTexture pair I had :)

Comment 12

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912987 [details]
Bug 1395497 - Use mat3s to transform tex coords in GLBlitHelper. -

https://reviewboard.mozilla.org/r/184350/#review189474

> GetTransformMatrix should take a gfx::Matrix4x4*, not gfx::Matrix4x4&.

Any particular reason why? I chose a reference because otherwise we should null check it. We seem to use both quite a bit in gfx code.

Comment 13

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912987 [details]
Bug 1395497 - Use mat3s to transform tex coords in GLBlitHelper. -

https://reviewboard.mozilla.org/r/184350/#review189474

> This should be in the vert shader.
> We should just be uploading a matrix to the vs instead of the current mess of vec2s, though that does mean dealing with constructing matrices in c++.
> 
> I can do this for you.

This is the same way we've handled this texture transform in the compositor since forever, and the same way it worked in the previous GLBlitHelper implementation. I guess doing it in the vs could perform better, but that's scope creep for this patch.

Comment 14

[Kelsey Gilbert [:jgilbert]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #13)
> Comment on attachment 8912987 [details]
> Bug 1395497 - Add SurfaceTexture impl in GLBlitHelper
> 
> https://reviewboard.mozilla.org/r/184350/#review189474
> 
> > This should be in the vert shader.
> > We should just be uploading a matrix to the vs instead of the current mess of vec2s, though that does mean dealing with constructing matrices in c++.
> > 
> > I can do this for you.
> 
> This is the same way we've handled this texture transform in the compositor
> since forever, and the same way it worked in the previous GLBlitHelper
> implementation. I guess doing it in the vs could perform better, but that's
> scope creep for this patch.

This is API/architecture creep otherwise. Since we have a bad track record about clean ups, I believe we should fix it immediately.

Comment 15

[Kelsey Gilbert [:jgilbert]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #12)
> Comment on attachment 8912987 [details]
> Bug 1395497 - Add SurfaceTexture impl in GLBlitHelper
> 
> https://reviewboard.mozilla.org/r/184350/#review189474
> 
> > GetTransformMatrix should take a gfx::Matrix4x4*, not gfx::Matrix4x4&.
> 
> Any particular reason why? I chose a reference because otherwise we should
> null check it. We seem to use both quite a bit in gfx code.

Because it makes it clear at the call site that we're expecting an out-var. A null check isn't needed. I don't even assert on it anymore. (it'll crash all on its own)

Out-vars being contractually non-null is a standard guarantee.

Comment 16

[Kelsey Gilbert [:jgilbert]]

WIP with matrices:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=04bd0395451e679a5dd0a1eb1aecec34829da825

Comment 17

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912986 [details]
Bug 1395497 - Create SurfaceTexture in detached state, attach on first use

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184348/diff/1-2/

Comment 18

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912987 [details]
Bug 1395497 - Use mat3s to transform tex coords in GLBlitHelper. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184350/diff/1-2/

Comment 19

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912985 [details]
Bug 1395497 - Modify ScopedBindTexture to allow save/restore

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184346/diff/1-2/

Comment 20

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912988 [details]
Bug 1395497 - Regenerate JNI binding

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184352/diff/1-2/

Comment 21

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8912985 [details]
Bug 1395497 - Modify ScopedBindTexture to allow save/restore

https://reviewboard.mozilla.org/r/184346/#review190220

::: gfx/gl/ScopedGLHelpers.cpp:202
(Diff revision 2)
>  ScopedBindTexture::ScopedBindTexture(GLContext* aGL, GLuint aNewTex, GLenum aTarget)
>          : ScopedGLWrapper<ScopedBindTexture>(aGL)
>          , mTarget(aTarget)
>          , mOldTex(GetBoundTexture(aGL, aTarget))
>  {
> +    if (aNewTex) {

Use -1. There are potential cases where binding 0 is desirable, so using zero as a sentenel could be surprising.

Comment 23

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1395497 - Add AndroidSurfaceTexture support to GLBlitHelper. -

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

Comment 24

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912986 [details]
Bug 1395497 - Create SurfaceTexture in detached state, attach on first use

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184348/diff/2-3/

Comment 25

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912985 [details]
Bug 1395497 - Modify ScopedBindTexture to allow save/restore

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184346/diff/2-3/

Comment 26

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912988 [details]
Bug 1395497 - Regenerate JNI binding

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184352/diff/2-3/

Comment 27

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912987 [details]
Bug 1395497 - Use mat3s to transform tex coords in GLBlitHelper. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184350/diff/2-3/

Comment 28

[Daosheng Mu[:daoshengmu]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #27)
> Comment on attachment 8912987 [details]
> Bug XXX - Use mat3s to transform tex coords in GLBlitHelper. -
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/184350/diff/2-3/

It seems :jgilbert has done some review work already. We should let him keep follow-up it.

Comment 29

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912985 [details]
Bug 1395497 - Modify ScopedBindTexture to allow save/restore

https://reviewboard.mozilla.org/r/184346/#review190220

> Use -1. There are potential cases where binding 0 is desirable, so using zero as a sentenel could be surprising.

Textures are GLuint, though, so using negative as a sentinel could also be surprising (and we'd have to change the interface to use GLint, which would be weird). I think I'm just going to drop this patch and bind to either 0 or whatever texture the SurfaceTexture is going to bind to.

Comment 30

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912988 [details]
Bug 1395497 - Regenerate JNI binding

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184352/diff/3-4/

Comment 31

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912987 [details]
Bug 1395497 - Use mat3s to transform tex coords in GLBlitHelper. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184350/diff/3-4/

Comment 32

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8914008 [details]
Bug 1395497 - Add AndroidSurfaceTexture support to GLBlitHelper. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/185270/diff/1-2/

Comment 33

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912987 [details]
Bug 1395497 - Use mat3s to transform tex coords in GLBlitHelper. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184350/diff/4-5/

Comment 34

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8914008 [details]
Bug 1395497 - Add AndroidSurfaceTexture support to GLBlitHelper. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/185270/diff/2-3/

Comment 35

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912987 [details]
Bug 1395497 - Use mat3s to transform tex coords in GLBlitHelper. -

https://reviewboard.mozilla.org/r/184350/#review190648

I think mozreview got screwed up here -- this patch has not been reviewed yet. Jeff wrote it, I'm just uploading it with the rest of the changes for this bug.

Comment 36

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8914008 [details]
Bug 1395497 - Add AndroidSurfaceTexture support to GLBlitHelper. -

https://reviewboard.mozilla.org/r/185270/#review190720

Comment 37

[Kelsey Gilbert [:jgilbert]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #29)
> Comment on attachment 8912985 [details]
> Bug 1395497 - Modify ScopedBindTexture to allow save/restore
> 
> https://reviewboard.mozilla.org/r/184346/#review190220
> 
> > Use -1. There are potential cases where binding 0 is desirable, so using zero as a sentenel could be surprising.
> 
> Textures are GLuint, though, so using negative as a sentinel could also be
> surprising (and we'd have to change the interface to use GLint, which would
> be weird). I think I'm just going to drop this patch and bind to either 0 or
> whatever texture the SurfaceTexture is going to bind to.

Sounds good.

Comment 38

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8912987 [details]
Bug 1395497 - Use mat3s to transform tex coords in GLBlitHelper. -

https://reviewboard.mozilla.org/r/184350/#review190872

LGTM. thanks.

::: gfx/gl/GLBlitHelper.cpp:128
(Diff revision 5)
> +Mat<N>::Zero()
> +{
> +    Mat<N> ret;
> +    for (auto& x : ret.m) {
> +        x = 0.0f;
> +    }

Why not consider to use memcpy instead of assigning by items.

::: gfx/gl/GLBlitHelper.cpp:1046
(Diff revision 5)
>          break;
>      default:
>          gfxCriticalError() << "Unexpected srcTarget: " << srcTarget;
>          return;
>      }
> -    const auto& prog = GetDrawBlitProg(key);
> +    const auto& prog = GetDrawBlitProg({ fragHeader, kFragBody_RGBA});

a redundant space in {} before 'f'.

Comment 39

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8912987 [details]
Bug 1395497 - Use mat3s to transform tex coords in GLBlitHelper. -

https://reviewboard.mozilla.org/r/184350/#review190222

::: gfx/gl/AndroidSurfaceTexture.cpp:21
(Diff revision 2)
>    auto jarray = jni::FloatArray::LocalRef::Adopt(env, env->NewFloatArray(16));
> -  aSurfaceTexture->GetTransformMatrix(jarray);
> +  surfaceTexture->GetTransformMatrix(jarray);
>  
>    jfloat* array = env->GetFloatArrayElements(jarray.Get(), nullptr);
>  
> -  aMatrix._11 = array[0];
> +  outMatrix->_11 = array[0];

Oh man, gross.

::: gfx/gl/GLBlitHelper.cpp:366
(Diff revision 2)
>          gl->fUniformMatrix4fv(mLoc_uColorMatrix, 1, false, colorMatrix);
>      }
>  
> +    MOZ_ASSERT(bool(argsTexTransform) == (mLoc_uTexTransform != -1));
> +    if (argsTexTransform) {
> +        gl->fUniformMatrix4fv(mLoc_uTexTransform, 1, false, &argsTexTransform->transform._11);

If this works, use memcpy in GetTransformMatrix.
If that isn't valid, this isn't valid.

Comment 42

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912986 [details]
Bug 1395497 - Create SurfaceTexture in detached state, attach on first use

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184348/diff/3-4/

Comment 43

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912988 [details]
Bug 1395497 - Regenerate JNI binding

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184352/diff/4-5/

Comment 44

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912987 [details]
Bug 1395497 - Use mat3s to transform tex coords in GLBlitHelper. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184350/diff/5-6/

Comment 45

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8914008 [details]
Bug 1395497 - Add AndroidSurfaceTexture support to GLBlitHelper. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/185270/diff/3-4/

Comment 46

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8914008 [details]
Bug 1395497 - Add AndroidSurfaceTexture support to GLBlitHelper. -

https://reviewboard.mozilla.org/r/185270/#review191466

Jeff, I made some changes to the y flip based on local testing. Can you look?

Comment 47

[Code Review Bot [:reviewbot]]

Comment on attachment 8912986 [details]
Bug 1395497 - Create SurfaceTexture in detached state, attach on first use

https://reviewboard.mozilla.org/r/184348/#review191474


C/C++ static analysis didn't find any defects in this patch. Hooray!

Comment 48

[Code Review Bot [:reviewbot]]

Comment on attachment 8912987 [details]
Bug 1395497 - Use mat3s to transform tex coords in GLBlitHelper. -

https://reviewboard.mozilla.org/r/184350/#review191480


C/C++ static analysis didn't find any defects in this patch. Hooray!

Comment 49

[Code Review Bot [:reviewbot]]

Comment on attachment 8914008 [details]
Bug 1395497 - Add AndroidSurfaceTexture support to GLBlitHelper. -

https://reviewboard.mozilla.org/r/185270/#review191484


C/C++ static analysis didn't find any defects in this patch. Hooray!

Comment 50

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8914008 [details]
Bug 1395497 - Add AndroidSurfaceTexture support to GLBlitHelper. -

https://reviewboard.mozilla.org/r/185270/#review192942

::: gfx/gl/GLBlitHelper.cpp:769
(Diff revision 4)
> +    // something wrong.
> +    MOZ_ASSERT(transform3.at(0,2) == 0);
> +    MOZ_ASSERT(transform3.at(1,2) == 0);
> +    MOZ_ASSERT(transform3.at(2,2) == 1);
> +
> +    const OriginPos srcOrigin = srcImage->GetOriginPos();

Prefer const auto& in cases like this, particularly since the type is extremely obvious.

Comment 51

[Kelsey Gilbert [:jgilbert]]

Sorry, I didn't see this re-request!

Comment 52

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8503893cc85
Create SurfaceTexture in detached state, attach on first use r=jgilbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9fbcd8bd4cb
Regenerate JNI binding r=me
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e3dd1e01908
Use mat3s to transform tex coords in GLBlitHelper. - r=daoshengmu
https://hg.mozilla.org/integration/mozilla-inbound/rev/c99dadd2df7d
Add AndroidSurfaceTexture support to GLBlitHelper. - r=snorp

Comment 53

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/1498abcee79d for https://treeherder.mozilla.org/logviewer.html#?job_id=137356665&repo=mozilla-inbound on every flavor of Linux.

Comment 54

[James Willcox (:snorp) ([e-mail]) (he/him)]

Jeff, the bustage in comment #53 is probably related to your mat3 patch, since none of the others should affect Linux. Can you take a look?

Comment 55

[Kelsey Gilbert [:jgilbert]]

Ok, I'm going to land bug 1404536 first. It looks green?

Comment 56

[Kelsey Gilbert [:jgilbert]]

Unblocked! Back to you to rebase and land. (I hope!)

Comment 57

[James Willcox (:snorp) ([e-mail]) (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9d7e4ca5e1abb8d4af840bcc679868ce7d11cd0d

Comment 58

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/31cc637a34aa
Create SurfaceTexture in detached state, attach on first use r=jgilbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/4046d781cacd
Regenerate JNI binding r=me
https://hg.mozilla.org/integration/mozilla-inbound/rev/b6aad6761527
Add AndroidSurfaceTexture support to GLBlitHelper. - r=snorp

Comment 59

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/31cc637a34aa
https://hg.mozilla.org/mozilla-central/rev/4046d781cacd
https://hg.mozilla.org/mozilla-central/rev/b6aad6761527

Comment 60

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/31cc637a34aa
https://hg.mozilla.org/mozilla-central/rev/4046d781cacd
https://hg.mozilla.org/mozilla-central/rev/b6aad6761527

Comment 61

[Sylvestre Ledru [:Sylvestre]]

Too late for 57

Comment 62

[u549602]

Verified as fixed on the latest Beta (58.0b3/build 4) and the latest Nightly (59.0a1 / 2017-11-14)
This issue was tested on a Pixel C (Android 8.0)

Comment 63

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out from beta on request from gchang and RyanVM:

https://hg.mozilla.org/releases/mozilla-beta/rev/b2dca24b1741e6089c7fc1080493e2e230b52cb3

Comment 64

[James Willcox (:snorp) ([e-mail]) (he/him)]

Ryan/gchang, why was this backed out? Was it due to bug 1413500? I would like to reland this in beta, along with bug 1413500 and 1421313.

Comment 65

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8912986 [details]
Bug 1395497 - Create SurfaceTexture in detached state, attach on first use

Approval Request Comment
[Feature/Bug causing the regression]: Unknown
[User impact if declined]: Crashes, this fix is necessary to uplift bug 1421313
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: Bug 1413500
[Is the change risky?]: No
[Why is the change risky/not risky?]: Been using on Nightly with no problems
[String changes made/needed]: None

Comment 66

[Ryan VanderMeulen [:RyanVM]]

Yes, it was to due to the high crash volume on Beta until a suitable fix for bug 1413500 could be found. Re-landing on Beta is Gerry's call, so leaving his NI set :)

Comment 67

[Gerry Chang [:gchang]]

Hi :snorp,
Yes, like Ryan said, it was due to the high crash volume on beta. Given this issue has been there since 55 and we only have 10 days before RC week (1/15) after we are back from time off. Can we let this ride the train on 59?

Comment 68

[James Willcox (:snorp) ([e-mail]) (he/him)]

If we're willing to accept the crash rate I guess so...

Comment 69

[Gerry Chang [:gchang]]

Comment on attachment 8912986 [details]
Bug 1395497 - Create SurfaceTexture in detached state, attach on first use

This patch is huge and it's a bit risky to uplift this to 58 (next week is RC). Let's let it ride the train on 59 and won't fix for 58.

Comment 70

[Johan]

Hi, I posted this bug from v56 back in September.
It has been moved forward I see to v59.
Has this been fixed in the current v59?

Comment 71

[Sylvestre Ledru [:Sylvestre]]

Yes, it should be fixed in 59.