1293759 - RGBX Assertion with basic layers

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Mason Chang [Inactive] [:mchang]]

+++ This bug was initially created as a clone of Bug #1279063 +++

Comment 1

[Mason Chang [Inactive] [:mchang]]

Attachment: basic.patch

From https://bugzilla.mozilla.org/show_bug.cgi?id=1279063#c18

Does this patch work for you if you build locally?

Comment 2

[Jorg K (CEST = GMT+2)]

If push comes to shove, we can run a Thunderbird try with an M-C change included.

Comment 3

[alexander :surkov (:asurkov)]

(In reply to Mason Chang [:mchang] from comment #1)
> Created attachment 8779476 [details] [diff] [review]
> basic.patch
> 
> From https://bugzilla.mozilla.org/show_bug.cgi?id=1279063#c18
> 
> Does this patch work for you if you build locally?

seems helps

Comment 4

[Mason Chang [Inactive] [:mchang]]

Comment on attachment 8779476 [details] [diff] [review]
basic.patch

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

If we're in Basic layers, we allocate a data surface for textures. This bypassed our usual skia check to allocate textures and clear them for RGBX surfaces. This adds the check for Skia for basic layers.

Comment 5

[Jorg K (CEST = GMT+2)]

Thanks for solving this so quickly!

Comment 6

[Jorg K (CEST = GMT+2)]

Can you please get this landed. Thanking you in advance.

Comment 7

[Mason Chang [Inactive] [:mchang]]

I think try is ok - https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea473b744f9b

(In reply to Jorg K (GMT+2, PTO during summer) from comment #6)
> Can you please get this landed. Thanking you in advance.

Was waiting on tests to run first.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fbf4eba5db4
Memset RGBX surfaces with basic layers and a skia backend. r=lsalzman

Comment 9

[Jorg K (CEST = GMT+2)]

Comment on attachment 8779476 [details] [diff] [review]
basic.patch

Approval Request Comment
[Feature/regressing bug #]: bug 1279063 (landed on mozilla50)
[User impact if declined]: Mozmill tests fail for Thunderbird on Aurora.
[Describe test coverage new/current, TreeHerder]: ?
[Risks and why]: ?
[String/UUID change made/needed]: None.

Mason, can you please fill in the missing details.

Comment 11

[Milan Sreckovic [:milan] (needinfo for best results)]

As a follow up, the asserts that trigger in DrawTargetSkia are using local variables only used in the debug build - perhaps use DebugOnly<> for them to avoid warnings?

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7fbf4eba5db4

Comment 13

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Milan Sreckovic [:milan] from comment #11)
> As a follow up, the asserts that trigger in DrawTargetSkia are using local
> variables only used in the debug build - perhaps use DebugOnly<> for them to
> avoid warnings?

Never mind, this is debug only function in the first place.

Comment 14

[Mason Chang [Inactive] [:mchang]]

(In reply to Jorg K (GMT+2, PTO during summer) from comment #9)
> Comment on attachment 8779476 [details] [diff] [review]
> basic.patch
> 
> Approval Request Comment
> [Feature/regressing bug #]: bug 1279063 (landed on mozilla50)
> [User impact if declined]: Mozmill tests fail for Thunderbird on Aurora.
> [Describe test coverage new/current, TreeHerder]: Treeherder with basic layers
> [Risks and why]: Low, this would trigger an assertion on debug builds only. This fixes the assertion.
> [String/UUID change made/needed]: None.
> 
> Mason, can you please fill in the missing details.

Comment 15

[Mason Chang [Inactive] [:mchang]]

Approval Request Comment
[Feature/regressing bug #]: bug 1279063 (landed on mozilla50)
[User impact if declined]: Mozmill tests fail for Thunderbird on Aurora.
[Describe test coverage new/current, TreeHerder]: Treeherder with basic layers
[Risks and why]: Low, this would trigger an assertion on debug builds only. This fixes the assertion.
[String/UUID change made/needed]: None.

Comment 16

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8779476 [details] [diff] [review]
basic.patch

Fixes a test failure on Thunderbird, Aurora50+

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f23573c8eff0

Comment 18

[Jorg K (CEST = GMT+2)]

Thanks for the uplift but sadly we're still seeing the crash here:

https://treeherder.mozilla.org/#/jobs?repo=comm-aurora&revision=ed85c9257236a59d81ded1ac7501c18f2d36077e&selectedJob=29046

Assertion failure: aData[topLeft] == 0xFF, at /builds/slave/tb-c-aurora-m64-d-000000000000/build/mozilla/gfx/2d/DrawTargetSkia.cpp:162

The stack doesn't appear very conclusive to me:
https://archive.mozilla.org/pub/thunderbird/tinderbox-builds/comm-aurora-macosx64-debug/1472292373/comm-aurora_yosemite_r7-debug_test-mozmill-bm134-tests1-macosx-build4.txt.gz
(look for DrawTargetSkia.cpp:162, I won't copy it here).

Aleth, could you please download the debug build from here
https://archive.mozilla.org/pub/thunderbird/tinderbox-builds/comm-aurora-macosx64-debug/1472292373/
and run it in the debugger to get a proper stack.

The test that will cause the crash is:
mozmake SOLO_TEST=message-header/test-message-header.js mozmill-one

Comment 19

[Jorg K (CEST = GMT+2)]

I think it's premature to ask Mason, since we need a stack dump to see where the call is coming from.

Comment 20

[aleth [:aleth]]

I thought this was exactly the same issue that was fixed on central? In which case you'd guess some additional patch needs uplift.

Comment 21

[Jorg K (CEST = GMT+2)]

Maybe. My guess is that the MOZ_ASSERT from bug 1279063 can be triggered via multiple code paths, only one of which was fixed here. So it would be good to find the other one(s) via a stack dump.

Comment 22

[Mason Chang [Inactive] [:mchang]]

It'd be nice to get a full stack trace since the one in comment 18 doesn't have line numbers :/.

Comment 23

[aleth [:aleth]]

TEST-START | /mail/test/mozmill/message-header/test-message-header.js | test_clicking_star_opens_inline_contact_editor
++DOMWINDOW == 34 (0x11f760000) [pid = 15618] [serial = 34] [outer = 0x11ee8a400]
[15618] WARNING: NS_ENSURE_TRUE(document) failed: file /mozilla/extensions/cookie/nsPermissionManager.cpp, line 2002
[15618] WARNING: NS_ENSURE_SUCCESS(rv, false) failed with result 0x80004002: file /mozilla/dom/base/nsDocument.cpp, line 5949
[15618] WARNING: NS_ENSURE_TRUE(standardURL) failed: file /mozilla/caps/nsPrincipal.cpp, line 176
[15618] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file /mozilla/caps/BasePrincipal.cpp, line 311
[15618] WARNING: 'NS_FAILED(rv)', file /mozilla/dom/workers/ServiceWorkerManager.cpp, line 1771
[15618] WARNING: Failed to retarget HTML data delivery to the parser thread.: file /mozilla/parser/html/nsHtml5StreamParser.cpp, line 967
Assertion failure: aData[topLeft] == 0xFF, at /mozilla/gfx/2d/DrawTargetSkia.cpp:162
#01: mozilla::gfx::DrawTargetSkia::FillRect(mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::Pattern const&, mozilla::gfx::DrawOptions const&)[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0x129247e]
#02: mozilla::layers::FillRectWithMask(mozilla::gfx::DrawTarget*, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::SourceSurface*, mozilla::gfx::SamplingFilter, mozilla::gfx::DrawOptions const&, mozilla::gfx::ExtendMode, mozilla[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0x141d5ec]
#03: mozilla::layers::DrawSurfaceWithTextureCoords(mozilla::gfx::DrawTarget*, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::SourceSurface*, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::Samp[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0x141061b]
#04: mozilla::layers::BasicCompositor::DrawQuad(mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::EffectChain const&, float, mozilla::gfx::Matrix4x4Typed<mozilla::[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0x140f590]
#05: mozilla::layers::TiledContentHost::RenderTile(mozilla::layers::TileHost&, mozilla::layers::EffectChain&, float, mozilla::gfx::Matrix4x4Typed<mozilla::gfx::UnknownUnits, mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SamplingFilter, mozilla::gfx::IntRec[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0x1477c91]
#06: mozilla::layers::TiledContentHost::RenderLayerBuffer(mozilla::layers::TiledLayerBufferComposite&, mozilla::gfx::Color const*, mozilla::layers::EffectChain&, float, mozilla::gfx::SamplingFilter, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0x147778a]
#07: mozilla::layers::TiledContentHost::Composite(mozilla::layers::LayerComposite*, mozilla::layers::EffectChain&, float, mozilla::gfx::Matrix4x4Typed<mozilla::gfx::UnknownUnits, mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SamplingFilter, mozilla::gfx::I[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0x1476f6f]
#08: mozilla::layers::PaintedLayerComposite::RenderLayer(mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&)[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0x1470886]
#09: void mozilla::layers::RenderLayers<mozilla::layers::ContainerLayerComposite>(mozilla::layers::ContainerLayerComposite*, mozilla::layers::LayerManagerComposite*, mozilla::gfx::IntRectTyped<mozilla::RenderTargetPixel> const&)[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0x1462472]
#10: void mozilla::layers::ContainerRender<mozilla::layers::ContainerLayerComposite>(mozilla::layers::ContainerLayerComposite*, mozilla::layers::LayerManagerComposite*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&)[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0x1454ed5]
#11: mozilla::layers::LayerManagerComposite::Render(mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&)[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0x146d61f]
#12: mozilla::layers::LayerManagerComposite::UpdateAndRender()[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0x146ca52]
#13: mozilla::layers::LayerManagerComposite::EndTransaction(mozilla::TimeStamp const&, mozilla::layers::LayerManager::EndTransactionFlags)[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0x146c585]
#14: mozilla::layers::CompositorBridgeParent::CompositeToTarget(mozilla::gfx::DrawTarget*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const*)[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0x147f247]
#15: mozilla::layers::CompositorVsyncScheduler::Composite(mozilla::TimeStamp)[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0x147e5cd]
#16: mozilla::detail::RunnableMethodImpl<void (mozilla::layers::CompositorVsyncScheduler::*)(mozilla::TimeStamp), true, true, mozilla::TimeStamp>::Run()[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0x148c29b]
#17: MessageLoop::RunTask(already_AddRefed<mozilla::Runnable>)[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0xad83a7]
#18: MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask&&)[/obj-mail-debug/dist/EarlybirdDebug.app/Contents/MacOS/XUL +0xad8d44]

Comment 24

[Jorg K (CEST = GMT+2)]

Thanks, Aleth. Looks like a different code path to me ;-)

Comment 25

[Mason Chang [Inactive] [:mchang]]

Are you only seeing this on aurora and not nightly? I downloaded the debug build from comment 18, but am not seeing any crash or assertions. From the thunderbird build instructions, it doesn't look like there's an easy way to build thunderbird from mozilla-central. Where could I get the thunderbird aurora branch?

Otherwise, you could try tracing where [1] is coming from and see where the surface is allocated. All the assertion is saying is "we have a surface that is RGBX format and it wasn't allocated to clear the texture to all white".

[1] http://searchfox.org/mozilla-central/source/gfx/2d/DrawTargetSkia.cpp#630

Comment 26

[Jorg K (CEST = GMT+2)]

(In reply to Mason Chang [:mchang] from comment #25)
> Are you only seeing this on aurora and not nightly?
Correct. As per comment #18, it's the red Z in OS X 10.10 debug. We are not seeing this is C-C/Daily:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=c679c89e86ba8617f840c9defa89a304a17f86ce&selectedJob=45133

> I downloaded the debug build from comment 18, but am not seeing any crash or assertions.
So you have Mac hardware, right? You would have to run the Mozmill test. It is 100% reproducible. I actually don't know how you would do that without having compiled it yourself, perhaps Aleth can explain this. Usually you would type:
mozmake SOLO_TEST=message-header/test-message-header.js mozmill-one
in the object directory. But if you haven't compiled it yourself, I don't know how to start the test.

> From the
> thunderbird build instructions, it doesn't look like there's an easy way to
> build thunderbird from mozilla-central. Where could I get the thunderbird
> aurora branch?
Building TB from C-C/M-C doesn't help since the crash is only on Aurora. So your best bet is to use the download and trigger it there. Otherwise, you'd have to download M-A and C-A and compile TB which I can't recommend.

> Otherwise, you could try tracing where [1] is coming from and see where the
> surface is allocated. All the assertion is saying is "we have a surface that
> is RGBX format and it wasn't allocated to clear the texture to all white".
> [1] http://searchfox.org/mozilla-central/source/gfx/2d/DrawTargetSkia.cpp#630
I can't trace anything since I don't Mac hardware. You're saying that in pat.mSurface (pat being passed in) you want to know where the surface is created?

Comment 27

[Mason Chang [Inactive] [:mchang]]

> > thunderbird build instructions, it doesn't look like there's an easy way to
> > build thunderbird from mozilla-central. Where could I get the thunderbird
> > aurora branch?
> Building TB from C-C/M-C doesn't help since the crash is only on Aurora. So
> your best bet is to use the download and trigger it there. Otherwise, you'd
> have to download M-A and C-A and compile TB which I can't recommend.

Well if I have to run Mozmill, and I need to compile TB to get it to work, looks like I don't have much of a choice :/. Where's the C-A repository?
 
> > Otherwise, you could try tracing where [1] is coming from and see where the
> > surface is allocated. All the assertion is saying is "we have a surface that
> > is RGBX format and it wasn't allocated to clear the texture to all white".
> > [1] http://searchfox.org/mozilla-central/source/gfx/2d/DrawTargetSkia.cpp#630

> I can't trace anything since I don't Mac hardware. You're saying that in
> pat.mSurface (pat being passed in) you want to know where the surface is
> created?

Correct. If we can find out where the surface is being created, that's really where all the work is. Once we know, we can just memset it to the correct value, which is what the original patch did. It's odd that it's fixed in nightly but not aurora though, even after uplift. Does C-A automatically update M-A?

Comment 28

[Jorg K (CEST = GMT+2)]

(In reply to Mason Chang [:mchang] from comment #27)
> Well if I have to run Mozmill, and I need to compile TB to get it to work,
> looks like I don't have much of a choice :/. Where's the C-A repository?
I'm pretty sure you don't need to compile to run Mozmill. Aleth managed to do it. Please wait for the instructions from Aleth. BTW, C-A is here: https://hg.mozilla.org/releases/comm-aurora/

> Correct. If we can find out where the surface is being created, that's
> really where all the work is. Once we know, we can just memset it to the
> correct value, which is what the original patch did. It's odd that it's
> fixed in nightly but not aurora though, even after uplift.
This is odd, but I assume that it's a different code path.

> Does C-A automatically update M-A?
Thunderbird is built from two repositories, we use C-* and include M-* in a /mozilla subdirectory. Yes, TB Aurora is built using C-A and M-A in the /mozilla directory.

Comment 29

[aleth [:aleth]]

(In reply to Jorg K (GMT+2, PTO during summer) from comment #28)
> (In reply to Mason Chang [:mchang] from comment #27)
> > Well if I have to run Mozmill, and I need to compile TB to get it to work,
> > looks like I don't have much of a choice :/. Where's the C-A repository?
> I'm pretty sure you don't need to compile to run Mozmill. Aleth managed to
> do it. Please wait for the instructions from Aleth. BTW, C-A is here:
> https://hg.mozilla.org/releases/comm-aurora/

Actually I compiled TB, it seemed easier than figuring out how to set thing up from the various downloads.

TB build instructions are here: https://developer.mozilla.org/en-US/docs/Simple_Thunderbird_build
After the compile, the test can be run from the objdir using "make SOLO_TEST=message-header/test-message-header.js mozmill-one".

Comment 30

[Jorg K (CEST = GMT+2)]

Well, if you still have that, you could zip it up and upload it to, say, WeTransfer.com.

Comment 31

[Jorg K (CEST = GMT+2)]

Filed bug 1300498 to track down the bad surface.