Simplify ShSurf passing for WebGL

RESOLVED FIXED in Firefox 36

Status

()

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jgilbert, Assigned: jgilbert)

Tracking

(Blocks 1 bug)

unspecified
mozilla35
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox35 disabled, firefox36 fixed)

Details

Attachments

(9 attachments, 5 obsolete attachments)

3.01 KB, patch
jgilbert
: review+
Details | Diff | Splinter Review
7.68 KB, patch
jgilbert
: review+
Details | Diff | Splinter Review
51.84 KB, patch
jgilbert
: review+
Details | Diff | Splinter Review
38.19 KB, patch
kamidphish
: review+
mattwoodrow
: review+
Details | Diff | Splinter Review
85.73 KB, patch
kamidphish
: review+
mattwoodrow
: review+
Details | Diff | Splinter Review
17.21 KB, patch
kamidphish
: review+
mattwoodrow
: review+
Details | Diff | Splinter Review
11.41 KB, patch
jgilbert
: review+
Details | Diff | Splinter Review
871 bytes, patch
mattwoodrow
: review+
Details | Diff | Splinter Review
51.40 KB, application/zip
Details
In order to allow layers to manipulate TexClients, we need to switch from subverting the layers mechanisms via SurfaceStream to embracing them with a SharedSurfaceTextureClient.

In the short term, this is just a simplification of what we already do, but it will allow for doing more sophisticated things with layers transactions in the long run. (As well as just being a great simplification)
Posted patch patch 1: Add ShSurfHandle (obsolete) — Splinter Review
r=kamidphish from bug 1052234
Attachment #8488167 - Flags: review+
r=kamidphish in bug 1052234
Attachment #8488168 - Flags: review+
Attachment #8488170 - Flags: review?(matt.woodrow)
Attachment #8488170 - Flags: review?(dglastonbury)
Comment on attachment 8488170 [details] [diff] [review]
patch 3: Add TexClient for ShSurf

Review of attachment 8488170 [details] [diff] [review]:
-----------------------------------------------------------------

I've looked over the GL changes. Matt can you cast your expert eye over the CanvasClient, TextureClient/Host, and Layer changes?

::: gfx/2d/HelpersCairo.h
@@ +140,5 @@
>      case SurfaceFormat::R5G6B5:
>        return CAIRO_FORMAT_RGB16_565;
>      default:
>        gfxWarning() << "Unknown image format";
> +      MOZ_ASSERT(false, "Unknown image format");

Good.

::: gfx/gl/GLContext.h
@@ +1516,5 @@
>          BEFORE_GL_CALL;
>          mSymbols.fReadBuffer(mode);
>          AFTER_GL_CALL;
>      }
> +	

To the god of WS?
Attachment #8488170 - Flags: review?(dglastonbury) → review+
Comment on attachment 8488170 [details] [diff] [review]
patch 3: Add TexClient for ShSurf

Review of attachment 8488170 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/gl/GLContext.cpp
@@ +1706,2 @@
>  
> +    return nullptr;

Might as well just remove the callers of this then and kill it?

::: gfx/layers/client/CanvasClient.cpp
@@ +216,5 @@
>      bool bufferCreated = false;
>      if (!mBuffer) {
> +      // We need to dealloc in the client.
> +      TextureFlags flags = GetTextureFlags() |
> +                           TextureFlags::DEALLOCATE_CLIENT;

CreateCanvasClient should have set DEALLOCATE_CLIENT already.

::: gfx/layers/client/CanvasClient.h
@@ +44,5 @@
>     */
>    enum CanvasClientType {
>      CanvasClientSurface,
>      CanvasClientGLContext,
> +    CanvasClientTypeShSurf,

Once we convert to creating the specific TextureClients we can probably get rid of the other two types.

::: gfx/layers/client/TextureClient.h
@@ +681,5 @@
>  
> +/**
> + * A TextureClient implementation to share SharedSurfaces.
> + */
> +class ShSurfTexClient : public TextureClient

SharedSurfaceTextureClient

::: gfx/layers/composite/TextureHost.h
@@ +653,5 @@
>  
> +/**
> + * A TextureHost for SharedSurfaces
> + */
> +class ShSurfTexHost : public TextureHost

SharedSurfaceTextureHost
Attachment #8488170 - Flags: review?(matt.woodrow) → review+
(In reply to Dan Glastonbury :djg :kamidphish from comment #4)
> Comment on attachment 8488170 [details] [diff] [review]
> patch 3: Add TexClient for ShSurf
> 
> Review of attachment 8488170 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I've looked over the GL changes. Matt can you cast your expert eye over the
> CanvasClient, TextureClient/Host, and Layer changes?
> 
> ::: gfx/2d/HelpersCairo.h
> @@ +140,5 @@
> >      case SurfaceFormat::R5G6B5:
> >        return CAIRO_FORMAT_RGB16_565;
> >      default:
> >        gfxWarning() << "Unknown image format";
> > +      MOZ_ASSERT(false, "Unknown image format");
> 
> Good.
> 
> ::: gfx/gl/GLContext.h
> @@ +1516,5 @@
> >          BEFORE_GL_CALL;
> >          mSymbols.fReadBuffer(mode);
> >          AFTER_GL_CALL;
> >      }
> > +	
> 
> To the god of WS?
Not today!
Blocks: 1066271
(In reply to Matt Woodrow (:mattwoodrow) from comment #5)
> Comment on attachment 8488170 [details] [diff] [review]
> patch 3: Add TexClient for ShSurf
> 
> Review of attachment 8488170 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/gl/GLContext.cpp
> @@ +1706,2 @@
> >  
> > +    return nullptr;
> 
> Might as well just remove the callers of this then and kill it?
Right, done.
> 
> ::: gfx/layers/client/CanvasClient.cpp
> @@ +216,5 @@
> >      bool bufferCreated = false;
> >      if (!mBuffer) {
> > +      // We need to dealloc in the client.
> > +      TextureFlags flags = GetTextureFlags() |
> > +                           TextureFlags::DEALLOCATE_CLIENT;
> 
> CreateCanvasClient should have set DEALLOCATE_CLIENT already.
We actually don't need this, so I'll remove it.
> 
> ::: gfx/layers/client/CanvasClient.h
> @@ +44,5 @@
> >     */
> >    enum CanvasClientType {
> >      CanvasClientSurface,
> >      CanvasClientGLContext,
> > +    CanvasClientTypeShSurf,
> 
> Once we convert to creating the specific TextureClients we can probably get
> rid of the other two types.
Yep!
> 
> ::: gfx/layers/client/TextureClient.h
> @@ +681,5 @@
> >  
> > +/**
> > + * A TextureClient implementation to share SharedSurfaces.
> > + */
> > +class ShSurfTexClient : public TextureClient
> 
> SharedSurfaceTextureClient
So many letters! :P
> 
> ::: gfx/layers/composite/TextureHost.h
> @@ +653,5 @@
> >  
> > +/**
> > + * A TextureHost for SharedSurfaces
> > + */
> > +class ShSurfTexHost : public TextureHost
> 
> SharedSurfaceTextureHost
Yeah, ok.
Any ETA on landing now that everything is reviewed?
It is failing on try and I don't have access to the android/b2gemu setups to test it myself. (I'm still at the Khronos F2F)

These are the two most recent Try runs:
https://tbpl.mozilla.org/?tree=Try&rev=8a67d53ea1e7
https://tbpl.mozilla.org/?tree=Try&rev=e705fef5899b
New run, forgot to restrict it to b2gemu.
 https://tbpl.mozilla.org/?tree=Try&rev=684951085fba
Perhaps, I'm misunderstanding but does this patch set move the WaitSync call from compositor side to client side? If so, won't that cause performance regressions for b2g etc?
(In reply to Jeff Muizelaar [:jrmuizel] from comment #11)
> Perhaps, I'm misunderstanding but does this patch set move the WaitSync call
> from compositor side to client side? If so, won't that cause performance
> regressions for b2g etc?

It does not do that.
(In reply to Jeff Gilbert [:jgilbert] from comment #12)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #11)
> > Perhaps, I'm misunderstanding but does this patch set move the WaitSync call
> > from compositor side to client side? If so, won't that cause performance
> > regressions for b2g etc?
> 
> It does not do that.

Where is the new call to WaitSync()?
(In reply to Jeff Muizelaar [:jrmuizel] from comment #13)
> (In reply to Jeff Gilbert [:jgilbert] from comment #12)
> > (In reply to Jeff Muizelaar [:jrmuizel] from comment #11)
> > > Perhaps, I'm misunderstanding but does this patch set move the WaitSync call
> > > from compositor side to client side? If so, won't that cause performance
> > > regressions for b2g etc?
> > 
> > It does not do that.
> 
> Where is the new call to WaitSync()?

In SharedSurfaceTextureHost::EnsureTexSource().
Blocks: 1070308
Blocks: 1077722
No longer blocks: 1028859
r=kamidphish
Attachment #8488167 - Attachment is obsolete: true
Attachment #8500785 - Flags: review+
r=kamidphish
Attachment #8488168 - Attachment is obsolete: true
Attachment #8500786 - Flags: review+
r=kamidphish,mattwoodrow
Attachment #8488170 - Attachment is obsolete: true
Attachment #8500787 - Flags: review+
Attachment #8500788 - Flags: review?(matt.woodrow)
Attachment #8500788 - Flags: review?(dglastonbury)
I got frustrated by canvas2d issues, and removing SurfaceStream simplifies things, so I did that.
Attachment #8500789 - Flags: review?(snorp)
Attachment #8500789 - Flags: review?(matt.woodrow)
Attachment #8500789 - Flags: review?(dglastonbury)
Attachment #8500790 - Flags: review?(snorp)
Attachment #8500790 - Flags: review?(matt.woodrow)
Attachment #8500790 - Flags: review?(dglastonbury)
Comment on attachment 8500788 [details] [diff] [review]
patch 3.5: Fixes to patch 3

Review of attachment 8500788 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/canvas/WebGLContext.cpp
@@ +1416,4 @@
>  
>      gl->MakeCurrent();
> +
> +    auto screen = gl->Screen();

Please don't use auto unless the type is otherwise immediately obvious the reader.

::: gfx/layers/opengl/GrallocTextureClient.cpp
@@ +347,5 @@
>    return 0;
>  }
>  
> +/*static*/ TemporaryRef<TextureClient>
> +GrallocTextureClientOGL::FromShSurf(gl::SharedSurface* abstractSurf,

FromSharedSurface
Attachment #8500788 - Flags: review?(matt.woodrow) → review+
Comment on attachment 8500788 [details] [diff] [review]
patch 3.5: Fixes to patch 3

Review of attachment 8500788 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/gl/GLScreenBuffer.cpp
@@ +43,5 @@
>      UniquePtr<SurfaceFactory> factory;
>  
>  #ifdef MOZ_WIDGET_GONK
>      /* On B2G, we want a Gralloc factory, and we want one right at the start */
> +    auto allocator = caps.surfaceAllocator;

auto is the new var?

::: gfx/gl/GLUploadHelpers.cpp
@@ +461,5 @@
> +        case SurfaceFormat::R8G8B8A8:
> +            if (gl->GetPreferredARGB32Format() == LOCAL_GL_BGRA) {
> +              format = LOCAL_GL_BGRA;
> +              type = LOCAL_GL_UNSIGNED_INT_8_8_8_8_REV;
> +              surfaceFormat = SurfaceFormat::B8G8R8A8;

Is this saying to swap the BGRA order to get RGBA?

::: gfx/layers/opengl/GrallocTextureClient.cpp
@@ +112,5 @@
>       android::sp<Fence> fence = mReleaseFenceHandle.mFence;
>  #if ANDROID_VERSION == 17
>       fence->waitForever(1000, "GrallocTextureClientOGL::Lock");
>       // 1000 is what Android uses. It is warning timeout ms.
> +     // This timeous is removed since ANDROID_VERSION 18.

timeout?
Attachment #8500788 - Flags: review?(dglastonbury) → review+
Comment on attachment 8500789 [details] [diff] [review]
patch 4: Remove SurfaceStream

Review of attachment 8500789 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/canvas/CanvasRenderingContext2D.cpp
@@ +4833,5 @@
> +  data.mSize = nsIntSize(mWidth, mHeight);
> +  data.mHasAlpha = !mOpaque;
> +
> +  SkiaGLGlue* glue = gfxPlatform::GetPlatform()->GetSkiaGLGlue();
> +  GLuint skiaGLTex = (GLuint)(uintptr_t)mTarget->GetNativeSurface(NativeSurfaceType::OPENGL_TEXTURE);

Wat?! double cast.
Attachment #8500789 - Flags: review?(dglastonbury) → review+
Comment on attachment 8500790 [details] [diff] [review]
patch 4.5: Fixes for patch 4

Review of attachment 8500790 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/canvas/CanvasRenderingContext2D.cpp
@@ +4795,2 @@
>      SkiaGLGlue* glue = gfxPlatform::GetPlatform()->GetSkiaGLGlue();
> +    GLuint skiaGLTex = (GLuint)(uintptr_t)mTarget->GetNativeSurface(NativeSurfaceType::OPENGL_TEXTURE);

Double cast?

::: gfx/2d/DrawTargetSkia.cpp
@@ +704,5 @@
>    TempBitmap bitmap = GetBitmapForSurface(aSurface);
>  
>    // This is a fast path that is disabled for now to mimimize risk
>    if (false && !bitmap.mBitmap.getTexture() && mCanvas->imageInfo() == bitmap.mBitmap.info()) {
> +  SkBitmap bm(bitmap.mBitmap);

Indent.

::: gfx/gl/SharedSurfaceGralloc.cpp
@@ +67,5 @@
>      gfxContentType type = hasAlpha ? gfxContentType::COLOR_ALPHA
>                                     : gfxContentType::COLOR;
>  
> +    auto platform = gfxPlatform::GetPlatform();
> +    gfxImageFormat format = platform->OptimalFormatForContent(type);

why is platform auto, yet format is gfxImageFormat?

::: gfx/layers/CopyableCanvasLayer.cpp
@@ +95,5 @@
>      }
>      return;
>    }
>  
> +  if (mDrawTarget) {

Don't need { }.
Attachment #8500790 - Flags: review?(dglastonbury) → review+
Comment on attachment 8500789 [details] [diff] [review]
patch 4: Remove SurfaceStream

Review of attachment 8500789 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/gl/SurfaceTypes.cpp
@@ +46,5 @@
>      bpp16 = false;
>      depth = false;
>      stencil = false;
>      antialias = false;
> +    premultAlpha = true;

Why this change?

::: gfx/layers/CopyableCanvasLayer.cpp
@@ +109,5 @@
> +  if (mGLFrontbuffer) {
> +    frontbuffer = mGLFrontbuffer.get();
> +  } else {
> +    auto screen = mGLContext->Screen();
> +    auto front = screen->Front();

no auto please

::: gfx/layers/client/CanvasClient.cpp
@@ +140,5 @@
>                                                   aFormat, aSize, backend,
>                                                   mTextureInfo.mTextureFlags | aFlags);
>  #endif
>  }
> +/*

Remove this code instead of commenting it out.
Attachment #8500789 - Flags: review?(matt.woodrow) → review+
Comment on attachment 8500790 [details] [diff] [review]
patch 4.5: Fixes for patch 4

Review of attachment 8500790 [details] [diff] [review]:
-----------------------------------------------------------------

It's nice seeing indent and whitespace being cleaned up, but please put those changes into a separate patch from functional changes.

The CopyableCanvasLayer change worries me, will wait to hear back about that.

::: gfx/2d/DrawTargetSkia.cpp
@@ +715,5 @@
> +    // adjust pixels for the source offset
> +    pixels += aSourceRect.x + aSourceRect.y*bm.rowBytes();
> +    mCanvas->writePixels(info, pixels, bm.rowBytes(), aDestination.x, aDestination.y);
> +    return;
> +  }

Indent all the way

::: gfx/gl/SharedSurfaceGralloc.cpp
@@ +72,3 @@
>  
> +    typedef GrallocTextureClientOGL ptrT;
> +    RefPtr<ptrT> grallocTC = new ptrT(allocator,

I'm not convinced these changes made anything more readable :)

::: gfx/layers/CopyableCanvasLayer.cpp
@@ -81,5 @@
>  CopyableCanvasLayer::UpdateTarget(DrawTarget* aDestTarget)
>  {
> -  if (!IsDirty())
> -    return;
> -  Painted();

Why was this removed? We don't want to readback from the GL texture every frame if it hasn't changed. I see that ClientCanvasLayer checks this itself, but BasicCanvasLayer will be regressed.
(In reply to Matt Woodrow (:mattwoodrow) from comment #27)
> Comment on attachment 8500790 [details] [diff] [review]
> patch 4.5: Fixes for patch 4
> 
> Review of attachment 8500790 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> It's nice seeing indent and whitespace being cleaned up, but please put
> those changes into a separate patch from functional changes.
> 
> The CopyableCanvasLayer change worries me, will wait to hear back about that.
> 
> ::: gfx/2d/DrawTargetSkia.cpp
> @@ +715,5 @@
> > +    // adjust pixels for the source offset
> > +    pixels += aSourceRect.x + aSourceRect.y*bm.rowBytes();
> > +    mCanvas->writePixels(info, pixels, bm.rowBytes(), aDestination.x, aDestination.y);
> > +    return;
> > +  }
> 
> Indent all the way
Oops, ok.
> 
> ::: gfx/gl/SharedSurfaceGralloc.cpp
> @@ +72,3 @@
> >  
> > +    typedef GrallocTextureClientOGL ptrT;
> > +    RefPtr<ptrT> grallocTC = new ptrT(allocator,
> 
> I'm not convinced these changes made anything more readable :)
Fair. I can cut the decl and definition to different lines and still keep it short.
> 
> ::: gfx/layers/CopyableCanvasLayer.cpp
> @@ -81,5 @@
> >  CopyableCanvasLayer::UpdateTarget(DrawTarget* aDestTarget)
> >  {
> > -  if (!IsDirty())
> > -    return;
> > -  Painted();
> 
> Why was this removed? We don't want to readback from the GL texture every
> frame if it hasn't changed. I see that ClientCanvasLayer checks this itself,
> but BasicCanvasLayer will be regressed.

Ok, we need to change BasicCanvasLayer to manage its own dirtyness. This bug was a lot of trouble to track down. Dirtying should always happen at the same layer.
The issue here was that we mark the layer clean, and then after marking it clean, call UpdateTarget. Since the layer's clean, UpdateTarget exits early, and does no work.
Comment on attachment 8500789 [details] [diff] [review]
patch 4: Remove SurfaceStream

Snorp is on PTO, and two sets of eyes is probably enough.
Attachment #8500789 - Flags: review?(snorp)
Comment on attachment 8500790 [details] [diff] [review]
patch 4.5: Fixes for patch 4

Snorp is on PTO, and two sets of eyes is probably enough.
Attachment #8500790 - Flags: review?(snorp)
(In reply to Dan Glastonbury :djg :kamidphish from comment #23)
> Comment on attachment 8500788 [details] [diff] [review]
> patch 3.5: Fixes to patch 3
> 
> Review of attachment 8500788 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/gl/GLScreenBuffer.cpp
> @@ +43,5 @@
> >      UniquePtr<SurfaceFactory> factory;
> >  
> >  #ifdef MOZ_WIDGET_GONK
> >      /* On B2G, we want a Gralloc factory, and we want one right at the start */
> > +    auto allocator = caps.surfaceAllocator;
> 
> auto is the new var?
Yep. I've gone off the deep end for them. We still have static types, but I just don't have to look up the exact namespaced name when I can have the compiler handle that for me.

I added the type back in for this case, and can add it back for any others.
> 
> ::: gfx/gl/GLUploadHelpers.cpp
> @@ +461,5 @@
> > +        case SurfaceFormat::R8G8B8A8:
> > +            if (gl->GetPreferredARGB32Format() == LOCAL_GL_BGRA) {
> > +              format = LOCAL_GL_BGRA;
> > +              type = LOCAL_GL_UNSIGNED_INT_8_8_8_8_REV;
> > +              surfaceFormat = SurfaceFormat::B8G8R8A8;
> 
> Is this saying to swap the BGRA order to get RGBA?
Yep. We're uploading RGBA data to GL as BGRA, and then just remembering that we need to sample from B to get R. I added a comment saying this.
> 
> ::: gfx/layers/opengl/GrallocTextureClient.cpp
> @@ +112,5 @@
> >       android::sp<Fence> fence = mReleaseFenceHandle.mFence;
> >  #if ANDROID_VERSION == 17
> >       fence->waitForever(1000, "GrallocTextureClientOGL::Lock");
> >       // 1000 is what Android uses. It is warning timeout ms.
> > +     // This timeous is removed since ANDROID_VERSION 18.
> 
> timeout?
Yep, fixed.
(In reply to Dan Glastonbury :djg :kamidphish from comment #24)
> Comment on attachment 8500789 [details] [diff] [review]
> patch 4: Remove SurfaceStream
> 
> Review of attachment 8500789 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/canvas/CanvasRenderingContext2D.cpp
> @@ +4833,5 @@
> > +  data.mSize = nsIntSize(mWidth, mHeight);
> > +  data.mHasAlpha = !mOpaque;
> > +
> > +  SkiaGLGlue* glue = gfxPlatform::GetPlatform()->GetSkiaGLGlue();
> > +  GLuint skiaGLTex = (GLuint)(uintptr_t)mTarget->GetNativeSurface(NativeSurfaceType::OPENGL_TEXTURE);
> 
> Wat?! double cast.

Yep, the existing code already does it. In some ways it makes sense, first stepping from a void* to a uintptr_t, but then truncating to a GLuint.
(In reply to Dan Glastonbury :djg :kamidphish from comment #25)
> Comment on attachment 8500790 [details] [diff] [review]
> patch 4.5: Fixes for patch 4
> 
> Review of attachment 8500790 [details] [diff] [review]:
> -----------------------------------------------------------------
> ::: gfx/2d/DrawTargetSkia.cpp
> @@ +704,5 @@
> >    TempBitmap bitmap = GetBitmapForSurface(aSurface);
> >  
> >    // This is a fast path that is disabled for now to mimimize risk
> >    if (false && !bitmap.mBitmap.getTexture() && mCanvas->imageInfo() == bitmap.mBitmap.info()) {
> > +  SkBitmap bm(bitmap.mBitmap);
> 
> Indent.
Oops.
> 
> ::: gfx/gl/SharedSurfaceGralloc.cpp
> @@ +67,5 @@
> >      gfxContentType type = hasAlpha ? gfxContentType::COLOR_ALPHA
> >                                     : gfxContentType::COLOR;
> >  
> > +    auto platform = gfxPlatform::GetPlatform();
> > +    gfxImageFormat format = platform->OptimalFormatForContent(type);
> 
> why is platform auto, yet format is gfxImageFormat?
I think it's relevant which of our many format types `format` is here, but it's not really relevant what type `platform` has, only that it has this function call. I'm primarily defining `platform` separately to help with line length.
> 
> ::: gfx/layers/CopyableCanvasLayer.cpp
> @@ +95,5 @@
> >      }
> >      return;
> >    }
> >  
> > +  if (mDrawTarget) {
> 
> Don't need { }.
gfx/layers likes to use {} everywhere.
(In reply to Matt Woodrow (:mattwoodrow) from comment #26)
> Comment on attachment 8500789 [details] [diff] [review]
> patch 4: Remove SurfaceStream
> 
> Review of attachment 8500789 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/gl/SurfaceTypes.cpp
> @@ +46,5 @@
> >      bpp16 = false;
> >      depth = false;
> >      stencil = false;
> >      antialias = false;
> > +    premultAlpha = true;
> 
> Why this change?
It's pretty arbitrary, but since I understand the canvas2d paths less, and canvas2d is always premult, I figure this is safer.
> 
> ::: gfx/layers/CopyableCanvasLayer.cpp
> @@ +109,5 @@
> > +  if (mGLFrontbuffer) {
> > +    frontbuffer = mGLFrontbuffer.get();
> > +  } else {
> > +    auto screen = mGLContext->Screen();
> > +    auto front = screen->Front();
> 
> no auto please
Ok.
> 
> ::: gfx/layers/client/CanvasClient.cpp
> @@ +140,5 @@
> >                                                   aFormat, aSize, backend,
> >                                                   mTextureInfo.mTextureFlags | aFlags);
> >  #endif
> >  }
> > +/*
> 
> Remove this code instead of commenting it out.
Thanks for reminding me.
patch 5: Review fixes
Attachment #8502019 - Flags: review+
Attachment #8502019 - Attachment description: fixes.diff → patch 5: Review fixes
Comment on attachment 8500790 [details] [diff] [review]
patch 4.5: Fixes for patch 4

Review of attachment 8500790 [details] [diff] [review]:
-----------------------------------------------------------------

It's nice seeing indent and whitespace being cleaned up, but please put those changes into a separate patch from functional changes.

The CopyableCanvasLayer change worries me, will wait to hear back about that.

::: gfx/2d/DrawTargetSkia.cpp
@@ +715,5 @@
> +    // adjust pixels for the source offset
> +    pixels += aSourceRect.x + aSourceRect.y*bm.rowBytes();
> +    mCanvas->writePixels(info, pixels, bm.rowBytes(), aDestination.x, aDestination.y);
> +    return;
> +  }

Indent all the way

::: gfx/gl/SharedSurfaceGralloc.cpp
@@ +72,3 @@
>  
> +    typedef GrallocTextureClientOGL ptrT;
> +    RefPtr<ptrT> grallocTC = new ptrT(allocator,

I'm not convinced these changes made anything more readable :)

::: gfx/layers/CopyableCanvasLayer.cpp
@@ -81,5 @@
>  CopyableCanvasLayer::UpdateTarget(DrawTarget* aDestTarget)
>  {
> -  if (!IsDirty())
> -    return;
> -  Painted();

Why was this removed? We don't want to readback from the GL texture every frame if it hasn't changed. I see that ClientCanvasLayer checks this itself, but BasicCanvasLayer will be regressed.
Attachment #8500790 - Flags: review?(matt.woodrow) → review-
Attachment #8502087 - Flags: review?(matt.woodrow)
Attachment #8502087 - Attachment is obsolete: true
Attachment #8502087 - Flags: review?(matt.woodrow)
Attachment #8502091 - Flags: review?(matt.woodrow)
Attachment #8502091 - Attachment is obsolete: true
Attachment #8502091 - Flags: review?(matt.woodrow)
Attachment #8502093 - Flags: review?(matt.woodrow)
Comment on attachment 8502093 [details] [diff] [review]
patch 4.6: 0001-Handle-dirtying-in-BasicCanvasLayer.patch

Review of attachment 8502093 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the transaction callbacks moved.

::: gfx/layers/basic/BasicCanvasLayer.cpp
@@ +32,5 @@
> +    Painted();
> +
> +    FirePreTransactionCallback();
> +    UpdateTarget();
> +    FireDidTransactionCallback();

I think we want to fire the pre/post transaction callbacks unconditionally, not inside the IsDirty scope.
Attachment #8502093 - Flags: review?(matt.woodrow) → review+
Attachment #8500790 - Flags: review- → review+
(In reply to Jeff Gilbert [:jgilbert] from comment #33)
> (In reply to Dan Glastonbury :djg :kamidphish from comment #25)
> > Don't need { }.
> gfx/layers likes to use {} everywhere.

WWBED? :-)
Blocks: 1081363
Depends on: 1081495
Patch 1 seems to have introduced this crash: [@ _moz_cairo_surface_destroy ]
bp-bb81c8a8-708b-4a4e-bac3-1ef702141011

A brief description of the problem is here: http://forums.mozillazine.org/viewtopic.php?p=13814603#p13814603

When I have more time I can look into opening a new bug with an STR, if still necessary by then.
Depends on: 1081535
Blocks: 1081694
The crashkill team believes that this patch is likely the cause of several new nightly crashes: bug 1081790 and a spike in the rate of the _moz_cairo_surface_destroy signature tracked by bug 1071079. We believe that this should be backed out of Aurora/35 before updates are enabled, and unless an obvious fix is in place, we also recommend backing this out of nightly/36.
Flags: needinfo?(jgilbert)
Depends on: 1081790
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #48)
> The crashkill team believes that this patch is likely the cause of several
> new nightly crashes: bug 1081790 and a spike in the rate of the
> _moz_cairo_surface_destroy signature tracked by bug 1071079. We believe that
> this should be backed out of Aurora/35 before updates are enabled, and
> unless an obvious fix is in place, we also recommend backing this out of
> nightly/36.

No fix, so skipping Aurora uplift is probably a good idea.
I need STRs or ideas for the issues before removing it from nightly. Without a better idea of the issues, I can't hope to fix them. They did not come up in testing.
Flags: needinfo?(jgilbert)
Depends on: 1082754
To be clear, this already got uplifted to Aurora (automatically on merge day) and so the request is to back it out.

If you don't know what's causing the nightly issues, dmajor or I can help you navigate the crash-stats, but we should probably back it out in the meantime.
(In reply to IU from comment #46)
> Patch 1 seems to have introduced this crash: [@ _moz_cairo_surface_destroy ]
> bp-bb81c8a8-708b-4a4e-bac3-1ef702141011
> 
> A brief description of the problem is here:
> http://forums.mozillazine.org/viewtopic.php?p=13814603#p13814603
> 
> When I have more time I can look into opening a new bug with an STR, if
> still necessary by then.

Patch 1 doesn't change anything, though. Did you mean a different patch, or just the patches in general?
(In reply to Jeff Gilbert [:jgilbert] from comment #51)
> Patch 1 doesn't change anything, though. Did you mean a different patch, or
> just the patches in general?

It was a quick look for a candidate -- not that I understand the code. :-)

Also, after the crashes occurred, my GPU stopped being able to display videos and I had to reboot my computer.

If I'm still able to reproduce it, is there some type of output I should collect that might help?
(In reply to IU from comment #52)
> (In reply to Jeff Gilbert [:jgilbert] from comment #51)
> > Patch 1 doesn't change anything, though. Did you mean a different patch, or
> > just the patches in general?
> 
> It was a quick look for a candidate -- not that I understand the code. :-)
> 
> Also, after the crashes occurred, my GPU stopped being able to display
> videos and I had to reboot my computer.
> 
> If I'm still able to reproduce it, is there some type of output I should
> collect that might help?

Yes, please! An about:support would be helpful.
Flags: needinfo?(fehe)
I used the Oct 11 nightly build, since that's the build with which I first experience the problem.

I ran Firefox with WinDbg and visited the youtube video I originally experience the problem with.  Once I notice the following appearing in the log, over and over again, and the WinDbg log window flashing and glitching, I felt that was enough (I did not wait for the crash):

Failed to create GLContext for SkiaGL!
Can't find symbol 'wglCreatePbufferARB'.
Can't find symbol 'wglDestroyPbufferARB'.
Can't find symbol 'wglGetPbufferDCARB'.
Can't find symbol 'wglBindTexImageARB'.
Can't find symbol 'wglReleaseTexImageARB'.
Can't find symbol 'wglChoosePixelFormatARB'.
Can't find symbol 'wglGetPixelFormatAttribivARB'.
Can't find symbol 'wglGetExtensionsStringARB'.
Failed to create GLContext for SkiaGL!
Failed to create GLContext for SkiaGL!
Failed to create GLContext for SkiaGL!
Failed to create GLContext for SkiaGL!
Failed to create GLContext for SkiaGL!


I have the attached the WinDbg log
-----------------------------------
Youtube video link: https://www.youtube.com/watch?v=SlDdjtLincE&hd=1


Here is my about:support information:
--------------------------------------

Application Basics
------------------

Name: Firefox
Version: 35.0a1
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0
Multiprocess Windows: 0/4

Crash Reports for the Last 3 Days
---------------------------------

All Crash Reports

Extensions
----------

Name: Adblock Plus Pop-up Addon
Version: 0.9.2
Enabled: true
ID: adblockpopups@jessehakanen.net

Name: Add Bookmark Here ²
Version: 29.0.20140506
Enabled: true
ID: abhere2@moztw.org

Name: Add to Search Bar
Version: 2.5
Enabled: true
ID: add-to-searchbox@maltekraus.de

Name: Add-ons Manager Context Menu
Version: 0.4.2
Enabled: true
ID: amcontextmenu@loucypher

Name: Addons Manager Hilite
Version: 2.1.1
Enabled: true
ID: addonsmgrhilte@cfl

Name: Ageless
Version: 0.7
Enabled: true
ID: 2341n4m3@gmail.com

Name: BetterPrivacy
Version: 1.68
Enabled: true
ID: {d40f5e7b-d2cf-4856-b441-cc613eeffbe3}

Name: Browser View
Version: 34
Enabled: true
ID: {82129504-17c6-4fec-b132-9c17e61879ca}

Name: Change Bookmark Recent Folder List Length
Version: 1.1.0
Enabled: true
ID: change-bookmark-folder-list-length@makyen.foo

Name: Classic Theme Restorer
Version: 1.2.4
Enabled: true
ID: ClassicThemeRestorer@ArisT2Noia4dev

Name: Click to Play per-element
Version: 0.0.7
Enabled: true
ID: ClickToPlayPerElement@uaSad.addons.mozilla.org

Name: Click-to-Play Manager
Version: 1.3.1
Enabled: true
ID: click-to-play-manager@xulforge.com

Name: Configuration Mania
Version: 1.19.2014091501
Enabled: true
ID: {c4d362ec-1cff-4ca0-9031-99a8fad7995a}

Name: Context Search X
Version: 0.4.6.16
Enabled: true
ID: contextsearch2@lwz.addons.mozilla.org

Name: Cookie Controller
Version: 3.7
Enabled: true
ID: {ac2cfa60-bc96-11e0-962b-0800200c9a66}

Name: Distill Web Monitor - AlertBox
Version: 1.1.1
Enabled: true
ID: alertbox@ajitk.com

Name: DownloadHelper
Version: 4.9.24
Enabled: true
ID: {b9db16a4-6edc-47ec-a1f4-b86292ed211d}

Name: Duplicate in Tab Context Menu
Version: 1.0.9
Enabled: true
ID: DuplicateInTabContext@schuzak.jp

Name: GPU Accelerated Flash Player
Version: 1.32
Enabled: true
ID: gpuacceleratedflashplayer@stas

Name: Greasemonkey
Version: 2014.10.10.nightly
Enabled: true
ID: {e4a8a97b-f2ed-450b-b12d-ee082ba24781}

Name: HttpFox
Version: 0.8.14
Enabled: true
ID: {4093c4de-454a-4329-8aff-c6b0b123c386}

Name: Link Properties Plus
Version: 1.5.3pre6
Enabled: true
ID: linkPropertiesPlus@infocatcher

Name: Open in Browser
Version: 1.15
Enabled: true
ID: openinbrowser@www.spasche.net

Name: OPIE
Version: 5.0.1
Enabled: true
ID: OPIE@guid.customsoftwareconsult.com

Name: Precise Clear History
Version: 1.3
Enabled: true
ID: preciseclearhistory@vano

Name: Print Edit
Version: 12.10
Enabled: true
ID: printedit@DW-dev

Name: Private Tab
Version: 0.1.7.3
Enabled: true
ID: privateTab@infocatcher

Name: ProfileSwitcher
Version: 1.6.2
Enabled: true
ID: {fa8476cf-a98c-4e08-99b4-65a69cb4b7d4}

Name: QuickJava
Version: 2.0.4
Enabled: true
ID: {E6C1199F-E687-42da-8C24-E7770CC3AE66}

Name: Redirect Cleaner
Version: 2.4.0
Enabled: true
ID: redirectcleaner@example.net

Name: Redirector
Version: 2.8.3
Enabled: true
ID: redirector@einaregilsson.com

Name: RefControl
Version: 0.8.16
Enabled: true
ID: {455D905A-D37C-4643-A9E2-F6FEFAA0424A}

Name: RequestPolicy
Version: 0.5.28
Enabled: true
ID: requestpolicy@requestpolicy.com

Name: Restartless Restart
Version: 9
Enabled: true
ID: restartless.restart@erikvold.com

Name: ScrapBook
Version: 1.5.11
Enabled: true
ID: {53A03D43-5363-4669-8190-99061B2DEBA5}

Name: Session Exporter
Version: 0.9
Enabled: true
ID: {943b5589-7808-4a70-acdc-7b6ee21e7cce}

Name: Session Manager
Version: 0.8.1.6pre20140929b
Enabled: true
ID: {1280606b-2510-4fe0-97ef-9b5a22eafe30}

Name: SettingSanity
Version: 0.8.2
Enabled: true
ID: {12A60D0F-0077-4F41-81B2-1286DDD278BB}

Name: Sidebars List
Version: 0.2.1
Enabled: true
ID: sidebarsList@infocatcher

Name: Silent Block
Version: 1.2.2
Enabled: true
ID: SilentBlock@schuzak.jp

Name: Star-Button In Urlbar
Version: 1.1
Enabled: true
ID: Starbuttoninurlbar@ArisT2Noia4dev

Name: Tab in Textarea
Version: 0.10.2
Enabled: true
ID: {df9815d2-ca91-4e8f-aaa3-066d1de2c7a2}

Name: Table2Clipboard
Version: 1.5.3.1
Enabled: true
ID: {9ab67d74-ec41-4cb2-b417-df5d93ba1beb}

Name: Textarea Cache
Version: 0.9.3.2
Enabled: true
ID: {578e7caa-210f-4967-a0d3-88fe5b59a39f}

Name: Tile Tabs
Version: 11.12
Enabled: true
ID: tiletabs@DW-dev

Name: UAControl
Version: 0.1.3.1
Enabled: true
ID: uacontrol@qz.tsugumi.org

Name: URL Alias
Version: 2.3.2
Enabled: true
ID: urlalias@zibada.xgm.ru

Name: Video Blocker
Version: 1.0.0
Enabled: true
ID: jid1-3OQ5HY7YsLBV7Q@jetpack

Name: What about:..
Version: 4.1.1
Enabled: true
ID: jid0-IPSuVKD0J7yL1cIBwQAdoHTCWmY@jetpack

Name: YesScript
Version: 2.0
Enabled: true
ID: yesscript@userstyles.org

Name: YouTube Grid Search and Preview Player
Version: 3.7.0
Enabled: true
ID: gridtube@gridtube.txt

Name: YouTube High Definition
Version: 32.1
Enabled: true
ID: {7b1bf0b6-a1b9-42b0-b75d-252036438bdc}

Name: 1-Click YouTube Video Downloader
Version: 2.3.6
Enabled: false
ID: YoutubeDownloader@PeterOlayev.com

Name: Adblock Edge
Version: 2.1.5
Enabled: false
ID: {fe272bd1-5f76-4ea4-8501-a05d35d823fc}

Name: Adblock Plus
Version: 2.6.4.3867
Enabled: false
ID: {d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d}

Name: Auto Shutdown NG
Version: 0.9.16
Enabled: false
ID: jid0-HZ5UvAEiWWAxT9TKLuhEgUCARqo@jetpack

Name: BehindTheOverlay
Version: 0.1.1
Enabled: false
ID: jid1-Y3WfE7td45aWDw@jetpack

Name: Bluhell Firewall
Version: 2.4.0
Enabled: false
ID: {6BB5760D-F97E-421B-AF5B-8457A90C3CED}

Name: ChatZilla
Version: 0.9.91
Enabled: false
ID: {59c81df5-4b7a-477b-912d-4e0fdf64e5f2}

Name: Click to Play per-element CE
Version: 0.0.5
Enabled: false
ID: ClickToPlayPerElementCE@rpzrpz123.addons.mozilla.org

Name: Download Plan
Version: 1.3
Enabled: false
ID: downloadplan@firefoxmania.uci.cu

Name: Element Hiding Helper for Adblock Plus
Version: 1.3
Enabled: false
ID: elemhidehelper@adblockplus.org

Name: FEBE
Version: 8.0.5
Enabled: false
ID: {4BBDD651-70CF-4821-84F8-2B918CF89CA3}

Name: FireSSH
Version: 0.94.5
Enabled: false
ID: firessh@nightlight.ws

Name: FireX Proxy
Version: 3.3
Enabled: false
ID: divanproger@gmail.com

Name: Flash and Video Download
Version: 1.62
Enabled: false
ID: {bee6eb20-01e0-ebd1-da83-080329fb9a3a}

Name: Flash Video Downloader - YouTube Full HD Download
Version: 6.1.2
Enabled: false
ID: artur.dubovoy@gmail.com

Name: Flashblock
Version: 1.5.17
Enabled: false
ID: {3d7eb24f-2740-49df-8937-200b1cc08f8a}

Name: FlashGot
Version: 1.5.6.7
Enabled: false
ID: {19503e42-ca3c-4c27-b1e2-9cdb2170ee34}

Name: FlashStopper
Version: 1.2.3
Enabled: false
ID: flashstopper@byo.co.il

Name: Form History Control
Version: 1.3.3.0
Enabled: false
ID: formhistory@yahoo.com

Name: FoxReplace
Version: 0.16.1
Enabled: false
ID: fox@replace.fx

Name: Free Download Manager plugin
Version: 1.6.0.6
Enabled: false
ID: fdm_ffext@freedownloadmanager.org

Name: Globefish
Version: 1.4.1
Enabled: false
ID: globefish@projects.6831.courses.csail.mit.edu

Name: Google Reverse Image Search
Version: 32.0
Enabled: false
ID: {95322c08-05ff-4f3c-85fd-8ceb821988dd}

Name: Google search link fix
Version: 1.4.9
Enabled: false
ID: jid0-XWJxt5VvCXkKzQK99PhZqAn7Xbg@jetpack

Name: Hide Plugin Notifications
Version: 0.2
Enabled: false
ID: suppress-plugin-infobar@benjamin.smedbergs.us

Name: Hola Better Internet
Version: 1.4.995
Enabled: false
ID: jid1-4P0kohSJxU1qGg@jetpack

Name: HTTP UserAgent cleaner
Version: 0.6.0.6
Enabled: false
ID: HTTPUserAgentcleaner@addons.8vs.ru

Name: Integrated Authentication for Firefox
Version: 3.0.1
Enabled: false
ID: extension@firefox-ntlmauth.googlecode.com

Name: JavaScript Deobfuscator
Version: 1.6.4.196
Enabled: false
ID: jsdeobfuscator@adblockplus.org

Name: Kill Infinite Scroll
Version: 0.4
Enabled: false
ID: killInfiniteScroll@jetpack

Name: KillSpinners
Version: 1.2.0
Enabled: false
ID: killspinners@byo.co.il

Name: LeechBlock
Version: 0.6.6
Enabled: false
ID: {a95d8332-e4b4-6e7f-98ac-20b733364387}

Name: OneTab
Version: 1.9
Enabled: false
ID: extension@one-tab.com

Name: Open Link in Silent Tab
Version: 0.0.6
Enabled: false
ID: {d4c46ca0-999d-11da-a72b-0800200c9a66}

Name: Referrer Control
Version: 0.4.5
Enabled: false
ID: referrercontrol@qixinglu.com

Name: ReloadEvery
Version: 28.0.2
Enabled: false
ID: {888d99e7-e8b5-46a3-851e-1ec45da1e644}

Name: Screengrab (fix version)
Version: 0.98.02c
Enabled: false
ID: {02450914-cdd9-410f-b1da-db004e18c671}

Name: Search term highlighter
Version: 7.1
Enabled: false
ID: {458482f0-90fb-4257-855f-0ba2790584f9}

Name: SpeedView
Version: 1.0
Enabled: false
ID: jid1-MmDjnsjlez2Sdw@jetpack

Name: Sticky Notes
Version: 0.7.2
Enabled: false
ID: sticky@filenamezero.dip.jp

Name: Stylish
Version: 1.4.3
Enabled: false
ID: {46551EC9-40F0-4e47-8E18-8E5CF550CFB8}

Name: Tab Grenade
Version: 0.8.5
Enabled: false
ID: jid1-gzlHTgBCb5hzkA@jetpack

Name: Tab Utilities
Version: 1.5.28.1
Enabled: false
ID: tabutils@ithinc.cn

Name: TheSage one-click lookup
Version: 5.0.4406
Enabled: false
ID: wcapturex@deskperience.com

Name: UnMHT
Version: 7.2.0.2
Enabled: false
ID: {f759ca51-3a91-4dd1-ae78-9db5eee9ebf0}

Name: YouTube Center
Version: 2.1.1
Enabled: false
ID: jid1-cwbvBTE216jjpg@jetpack

Graphics
--------

Adapter Description: ATI Radeon HD 4290 (Microsoft Corporation - WDDM v1.1)
Adapter Drivers: aticfx64 aticfx64 aticfx32 aticfx32 atiumd64 atidxx64 atiumdag atidxx32 atiumdva atiumd6a atitmm64
Adapter RAM: 368
Device ID: 0x9714
Direct2D Enabled: true
DirectWrite Enabled: true (6.3.9600.17111)
Driver Date: 6-20-2012
Driver Version: 8.97.10.6
GPU #2 Active: false
GPU Accelerated Windows: 4/4 Direct3D 11 (OMTC)
Subsys ID: 00000000
Vendor ID: 0x1002
WebGL Renderer: Google Inc. -- ANGLE (ATI Radeon HD 4290 (Microsoft Corporation - WDDM v1.1) Direct3D9Ex vs_3_0 ps_3_0)
windowLayerManagerRemote: true
AzureCanvasBackend: direct2d
AzureContentBackend: direct2d
AzureFallbackCanvasBackend: cairo
AzureSkiaAccelerated: 0

Important Modified Preferences
------------------------------

accessibility.typeaheadfind.flashBar: 0
browser.cache.disk.capacity: 358400
browser.cache.disk.smart_size_cached_value: 358400
browser.cache.disk.smart_size.first_run: false
browser.cache.disk.smart_size.use_old_max: false
browser.cache.frecency_experiment: 3
browser.newtab.url: about:blank
browser.places.smartBookmarksVersion: 7
browser.sessionstore.max_tabs_undo: 30
browser.sessionstore.max_windows_undo: 10
browser.sessionstore.upgradeBackup.latestBuildID: 20141011030203
browser.startup.homepage_override.buildID: 20141011030203
browser.startup.homepage_override.mstone: 35.0a1
browser.tabs.defaultDrawInTitlebar: false
browser.tabs.drawInTitlebar: false
browser.urlbar.autoFill: false
browser.urlbar.unifiedcomplete: false
dom.mozApps.used: true
extensions.lastAppVersion: 35.0a1
gfx.canvas.azure.backends: direct2d,skia,cairo
gfx.content.azure.backends: direct2d,cairo
gfx.direct2d.use1_1: false
gfx.direct3d.last_used_feature_level_idx: 0
image.animation_mode: none
media.autoplay.enabled: false
media.gmp-gmpopenh264.lastUpdate: 1412184805
media.gmp-gmpopenh264.path: D:\Downloads\debug\tmp\gmp-gmpopenh264
media.gmp-gmpopenh264.version: 1.1
media.gmp-manager.lastCheck: 1413295659
network.cookie.lifetimePolicy: 2
network.cookie.prefsMigrated: true
network.dns.disablePrefetch: true
network.prefetch-next: false
places.database.lastMaintenance: 1413338394
places.history.expiration.transient_current_max_pages: 104858
plugin.disable_full_page_plugin_for_types: application/pdf,application/vnd.adobe.xfdf,application/vnd.fdf,application/vnd.adobe.xdp+xml
plugin.importedState: true
plugin.persistentPermissionAlways.intervalInDays: 0
plugin.sessionPermissionNow.intervalInMinutes: 0
plugin.state.flash: 1
plugin.state.java: 0
privacy.cpd.cookies: false
privacy.cpd.extensions-betterprivacy: true
privacy.cpd.extensions-sessionmanager: false
privacy.cpd.historysession: false
privacy.cpd.sessions: false
privacy.sanitize.migrateFx3Prefs: true
storage.vacuum.last.index: 1
storage.vacuum.last.places.sqlite: 1412626038

Important Locked Preferences
----------------------------

JavaScript
----------

Incremental GC: true

Accessibility
-------------

Activated: false
Prevent Accessibility: 0

Library Versions
----------------

NSPR
Expected minimum version: 4.10.7
Version in use: 4.10.7

NSS
Expected minimum version: 3.17.2 Basic ECC
Version in use: 3.17.2 Basic ECC

NSSSMIME
Expected minimum version: 3.17.2 Basic ECC
Version in use: 3.17.2 Basic ECC

NSSSSL
Expected minimum version: 3.17.2 Basic ECC
Version in use: 3.17.2 Basic ECC

NSSUTIL
Expected minimum version: 3.17.2
Version in use: 3.17.2

Experimental Features
---------------------

Name: Invisible test of the experiment branching system.
ID: experiment-branch-test-nightly@experiments.mozilla.org
Description: An experiment using branches just to test whether branches get saved correctly.
Active: false
End Date: 1409591246458
Homepage:
Flags: needinfo?(fehe)
The console spew is related to bug 1082850. If that's the problem, the patch we get there should solve it. We should have a build we can test of that tomorrow.
Backed out of Aurora:
 https://hg.mozilla.org/releases/mozilla-aurora/rev/d56d0157891e
Target Milestone: mozilla35 → mozilla36
TM tracks trunk landing, so leave that as-is unless you backout from m-c :)
Target Milestone: mozilla36 → mozilla35
Blocks: 1082850
Flags: needinfo?(fehe)
I'll look at it tomorrow (might not been till the evening), as I'm going to bed now.

Meanwhile, if I won't be able to run the debug or opt builds without special configurations, is there a regular build available?  In the past I have never succeeded in getting a debug build to start (it always required something I couldn't figure out -- even after reading the confusing documentation), so I stay away from them.

Supposedly their easier to use now, but I haven't touched one for probably three years now.

Thanks
Flags: needinfo?(fehe)
(In reply to IU from comment #59)
> I'll look at it tomorrow (might not been till the evening), as I'm going to
> bed now.
> 
> Meanwhile, if I won't be able to run the debug or opt builds without special
> configurations, is there a regular build available?  In the past I have
> never succeeded in getting a debug build to start (it always required
> something I couldn't figure out -- even after reading the confusing
> documentation), so I stay away from them.
> 
> Supposedly their easier to use now, but I haven't touched one for probably
> three years now.
> 
> Thanks

I run them similar to:

> mkdir foo 2>/dev/null
> 
> export JS_DISABLE_SLOW_SCRIPT_SIGNALS=1
> export MOZ_QUIET=1
> ./dist/bin/firefox -console -profile foo -no-remote

But even just running `firefox` should work, if you don't have another browser window open.
Blocks: 1083621
Before leaving for work, I managed to test the debug build with the same video I mentioned earlier.

There is no more crashing or spewing of errors.  The fix seems to work, as my GPU did not become unstable afterwards (as was the case with the buggy builds).

One thing though, I used my default profile to run the debug build and now debugging in permanently turned on and I get crashing when subsequently launching Firefox.  How do I completely disable debugging from my profile?
Forget the last part, something strange was happening -- even after reverting to a non-debug build.  So I completely deleted and extracted a new build and now my profile is fine.
See Also: → 1052234
Depends on: 1095639
Depends on: 1123573
Depends on: 1137251
Depends on: 1142071
Depends on: 1157990
Blocks: 1057554
You need to log in before you can comment on or make changes to this bug.