Closed Bug 1293759 Opened 3 years ago Closed 3 years ago

RGBX Assertion with basic layers

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: mchang, Assigned: mchang)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #1279063 +++
Attached patch basic.patchSplinter Review
From https://bugzilla.mozilla.org/show_bug.cgi?id=1279063#c18

Does this patch work for you if you build locally?
Flags: needinfo?(surkov.alexander)
If push comes to shove, we can run a Thunderbird try with an M-C change included.
(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
Flags: needinfo?(surkov.alexander)
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.
Attachment #8779476 - Flags: review?(lsalzman)
Attachment #8779476 - Flags: review?(lsalzman) → review+
Thanks for solving this so quickly!
Can you please get this landed. Thanking you in advance.
Flags: needinfo?(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.
Flags: needinfo?(mchang)
Pushed by mchang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fbf4eba5db4
Memset RGBX surfaces with basic layers and a skia backend. r=lsalzman
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.
Flags: needinfo?(mchang)
Attachment #8779476 - Flags: approval-mozilla-aurora?
Duplicate of this bug: 1294789
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?
https://hg.mozilla.org/mozilla-central/rev/7fbf4eba5db4
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
(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.
(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.
Flags: needinfo?(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 on attachment 8779476 [details] [diff] [review]
basic.patch

Fixes a test failure on Thunderbird, Aurora50+
Attachment #8779476 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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
Flags: needinfo?(aleth)
Flags: needinfo?(aleth) → needinfo?(mchang)
I think it's premature to ask Mason, since we need a stack dump to see where the call is coming from.
I thought this was exactly the same issue that was fixed on central? In which case you'd guess some additional patch needs uplift.
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.
It'd be nice to get a full stack trace since the one in comment 18 doesn't have line numbers :/.
Flags: needinfo?(mchang) → needinfo?(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]
Flags: needinfo?(aleth)
Thanks, Aleth. Looks like a different code path to me ;-)
Flags: needinfo?(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
Flags: needinfo?(mchang) → needinfo?(jorgk)
(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?
Flags: needinfo?(jorgk) → needinfo?(aleth)
> > 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?
(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.
(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".
Flags: needinfo?(aleth)
Well, if you still have that, you could zip it up and upload it to, say, WeTransfer.com.
Blocks: 1300498
Filed bug 1300498 to track down the bad surface.
You need to log in before you can comment on or make changes to this bug.