1273227 - Paint dumping is broken again (assertion in TextureClient::Unlock())

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Botond Ballo [:botond]]

STR:
  1. Build in debug mode. I was testing on Linux.
  2. Run with MOZ_DUMP_PAINT=1 MOZ_DUMP_PAINT_TO_FILE=1

I get an assertion failure on startup, with the following backtrace:

#6  0x00007f90865c73c0 in mozilla::layers::TextureClient::Unlock (this=0x3e32df0) at /home/botond/dev/mozilla/central/gfx/layers/client/TextureClient.cpp:442
#7  0x00007f90865c99c9 in mozilla::layers::TextureClient::GetAsSurface (this=0x3e32df0) at /home/botond/dev/mozilla/central/gfx/layers/client/TextureClient.cpp:1055
#8  0x00007f90865aaa43 in mozilla::layers::CompositableClient::DumpTextureClient (aStream=..., aTexture=0x3e32df0, aCompress=mozilla::layers::DoNotCompress)
    at /home/botond/dev/mozilla/central/gfx/layers/client/CompositableClient.cpp:288
#9  0x00007f90865ac120 in mozilla::layers::ContentClientDoubleBuffered::Dump (this=0x3dd65b0, aStream=..., aPrefix=0x7ffc37a2c460 "    ", aDumpHtml=true,
    aCompress=mozilla::layers::DoNotCompress) at /home/botond/dev/mozilla/central/gfx/layers/client/ContentClient.cpp:427
#10 0x00007f90864ef53d in mozilla::layers::Layer::Dump (this=0x3dd5770, aStream=..., aPrefix=0x7ffc37a2c460 "    ", aDumpHtml=true, aSorted=false)
    at /home/botond/dev/mozilla/central/gfx/layers/Layers.cpp:1758
#11 0x00007f90864efae9 in mozilla::layers::Layer::Dump (this=0x3dd1470, aStream=..., aPrefix=0x7ffc37a2c820 "  ", aDumpHtml=true, aSorted=false)
    at /home/botond/dev/mozilla/central/gfx/layers/Layers.cpp:1812
#12 0x00007f90864f25f4 in mozilla::layers::LayerManager::Dump (this=0x2b812d0, aStream=..., aPrefix=0x7f908baba2b6 <.L.str.999> "", aDumpHtml=true, aSorted=false)
    at /home/botond/dev/mozilla/central/gfx/layers/Layers.cpp:2383
#13 0x00007f90890c8b66 in mozilla::FrameLayerBuilder::DumpRetainedLayerTree (aManager=0x2b812d0, aStream=..., aDumpHtml=true)
    at /home/botond/dev/mozilla/central/layout/base/FrameLayerBuilder.cpp:5943
#14 0x00007f90891a88d8 in nsLayoutUtils::PaintFrame (aRenderingContext=0x0, aFrame=0x2c289a0, aDirtyRegion=..., aBackstop=4293783021, aBuilderMode=nsDisplayListBuilderMode::PAINTING,
    aFlags=(nsLayoutUtils::PaintFrameFlags::PAINT_WIDGET_LAYERS | nsLayoutUtils::PaintFrameFlags::PAINT_EXISTING_TRANSACTION | nsLayoutUtils::PaintFrameFlags::PAINT_NO_COMPOSITE))
    at /home/botond/dev/mozilla/central/layout/base/nsLayoutUtils.cpp:3607
#15 0x00007f90891d785b in PresShell::Paint (this=0x2c271a0, aViewToPaint=0x2c26490, aDirtyRegion=..., aFlags=1) at /home/botond/dev/mozilla/central/layout/base/nsPresShell.cpp:6546
#16 0x00007f9088c93e7a in nsViewManager::ProcessPendingUpdatesPaint (this=0x2c263f0, aWidget=0x2b7e7f0) at /home/botond/dev/mozilla/central/view/nsViewManager.cpp:482
#17 0x00007f9088c93a45 in nsViewManager::ProcessPendingUpdatesForView (this=0x2c263f0, aView=0x2c26490, aFlushDirtyRegion=true)
    at /home/botond/dev/mozilla/central/view/nsViewManager.cpp:413
#18 0x00007f9088c94cce in nsViewManager::ProcessPendingUpdates (this=0x2c263f0) at /home/botond/dev/mozilla/central/view/nsViewManager.cpp:1117
#19 0x00007f9089096518 in nsRefreshDriver::Tick (this=0x2c60c20, aNowEpoch=1463426040484017, aNowTime=...) at /home/botond/dev/mozilla/central/layout/base/nsRefreshDriver.cpp:1903

Comment 1

[Botond Ballo [:botond]]

Nical, you added this assertion - do you know what might be going wrong here?

Comment 2

[Nicolas Silva [:nical]]

When a TextureClient is unlocked, all external strong references to a DrawTarget that wraps the texture's data must be released. This is because destroying the DT can cause it to access the texture's data and the later may be made inaccessible during unlock (getting unmapped or whatnot).
This assertion fires when there is still a RefPtr<DrawTarget> holding on to the borrowed DrawTarget externally.

In this case the solution is simply to place dt in GetAsSurface into a scope that terminates before Unlock(), or to null dt out before unlock.

By the way, it would be better to avoid calling BorrowDrawTarget and Unlock if Lock returned false in this method.

Comment 3

[Botond Ballo [:botond]]

(In reply to Nicolas Silva [:nical] from comment #2)

Thanks! I'll implement a fix based on what you said.

Comment 4

[Marcia Knous [:marcia]]

Tracking this regression for 49.

Comment 5

[Botond Ballo [:botond]]

Attachment: MozReview Request: Bug 1273227 - Fix an assertion in TextureClient::Unlock(). r=nical

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

Comment 6

[Nicolas Silva [:nical]]

Comment on attachment 8756546 [details]
MozReview Request: Bug 1273227 - Fix an assertion in TextureClient::Unlock(). r=nical

https://reviewboard.mozilla.org/r/55236/#review52168

Nice

Comment 7

[Botond Ballo [:botond]]

Try push that includes this patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=731f452ee7ee

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d667c7bfcc25

Comment 9

[Carsten Book [:Tomcat]]

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