Closed
Bug 1187464
Opened 10 years ago
Closed 9 years ago
crash in mozilla::layers::BasicCompositor::DrawQuad(mozilla::gfx::RectTyped<T> const&, mozilla::gfx::RectTyped<T> const&, mozilla::layers::EffectChain const&, float, mozilla::gfx::Matrix4x4 const&)
Categories
(Core :: Graphics: Layers, defect)
Tracking
()
RESOLVED
FIXED
mozilla47
People
(Reporter: u279076, Assigned: milan)
References
Details
(Keywords: crash, regression)
Crash Data
Attachments
(3 files, 7 obsolete files)
2.82 KB,
patch
|
milan
:
review+
|
Details | Diff | Splinter Review |
15.72 KB,
patch
|
milan
:
review+
|
Details | Diff | Splinter Review |
4.18 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is
report bp-bdbfeb54-a138-4687-b7c5-c5eb82150724.
=============================================================
0 XUL mozilla::layers::BasicCompositor::DrawQuad(mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::EffectChain const&, float, mozilla::gfx::Matrix4x4 const&) gfx/layers/basic/BasicCompositor.cpp
1 XUL mozilla::layers::ImageHost::Composite(mozilla::layers::EffectChain&, float, mozilla::gfx::Matrix4x4 const&, mozilla::gfx::Filter const&, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits> const&, nsIntRegion const*) gfx/layers/composite/ImageHost.cpp
2 XUL mozilla::layers::CanvasLayerComposite::RenderLayer(nsIntRect const&) gfx/layers/composite/CanvasLayerComposite.cpp
3 XUL void mozilla::layers::RenderLayers<mozilla::layers::ContainerLayerComposite>(mozilla::layers::ContainerLayerComposite*, mozilla::layers::LayerManagerComposite*, mozilla::gfx::IntRectTyped<mozilla::RenderTargetPixel> const&) gfx/layers/composite/ContainerLayerComposite.cpp
4 XUL void mozilla::layers::ContainerRender<mozilla::layers::ContainerLayerComposite>(mozilla::layers::ContainerLayerComposite*, mozilla::layers::LayerManagerComposite*, nsIntRect const&) gfx/layers/composite/ContainerLayerComposite.cpp
5 XUL void mozilla::layers::RenderLayers<mozilla::layers::ContainerLayerComposite>(mozilla::layers::ContainerLayerComposite*, mozilla::layers::LayerManagerComposite*, mozilla::gfx::IntRectTyped<mozilla::RenderTargetPixel> const&) gfx/layers/composite/ContainerLayerComposite.cpp
6 XUL void mozilla::layers::ContainerRender<mozilla::layers::ContainerLayerComposite>(mozilla::layers::ContainerLayerComposite*, mozilla::layers::LayerManagerComposite*, nsIntRect const&) gfx/layers/composite/ContainerLayerComposite.cpp
7 XUL mozilla::layers::LayerManagerComposite::Render() gfx/layers/composite/LayerManagerComposite.cpp
8 XUL mozilla::layers::LayerManagerComposite::EndTransaction(void (*)(mozilla::layers::PaintedLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) gfx/layers/composite/LayerManagerComposite.cpp
9 XUL mozilla::layers::CompositorParent::CompositeToTarget(mozilla::gfx::DrawTarget*, nsIntRect const*) gfx/layers/composite/LayerManagerComposite.cpp
10 XUL mozilla::layers::CompositorVsyncObserver::Composite(mozilla::TimeStamp) gfx/layers/ipc/CompositorParent.cpp
11 XUL MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) ipc/chromium/src/base/message_loop.cc
12 XUL MessageLoop::DoWork() ipc/chromium/src/base/message_loop.cc
13 XUL base::MessagePumpDefault::Run(base::MessagePump::Delegate*) ipc/chromium/src/base/message_pump_default.cc
14 XUL MessageLoop::Run() ipc/chromium/src/base/message_loop.cc
15 XUL base::Thread::ThreadMain() ipc/chromium/src/base/thread.cc
16 XUL ThreadFunc ipc/chromium/src/base/platform_thread_posix.cc
17 libsystem_pthread.dylib _pthread_body
18 libsystem_pthread.dylib _pthread_start
19 libsystem_pthread.dylib thread_start
20 XUL base::strlcpy(char*, char const*, unsigned long) ipc/chromium/src/base/string_util.cc
21 @0x10e1cbc6f
=============================================================
More reports: https://crash-stats.mozilla.com/report/list?product=Firefox&signature=mozilla%3A%3Alayers%3A%3ABasicCompositor%3A%3ADrawQuad%28mozilla%3A%3Agfx%3A%3ARectTyped%3CT%3E+const%26%2C+mozilla%3A%3Agfx%3A%3ARectTyped%3CT%3E+const%26%2C+mozilla%3A%3Alayers%3A%3AEffectChain+const%26%2C+float%2C+mozilla%3A%3Agfx%3A%3AMatrix4x4+const%26%29
Note, I've filed this as a Mac OS X bug but there are reports of this on Windows. I suspect the Windows crashes deserve a different bug for the following reasons:
1. The crash reasons are different.
* Windows shows EXCEPTION_ACCESS_VIOLATION_READ
* Mac shows EXC_BAD_ACCESS/KERN_INVALID_ADDRESS
2. The crashes start with different Firefox versions
* Windows crashes started with Firefox 30
* Mac crashes started with Firefox 39
3. Volume is higher on Mac OS X than it is on Windows, especially when you consider user population.
* Windows has 43% of the crashes with 87% of the users
* Mac OS X has 57% of the crashes with 7% of the users
4. The URLs reported are quite different on Windows compared to Mac
* Top URL on Windows is Facebook.com with 83% whereas only 2% of the crashes on Mac OS X are with Facebook.com
* Top URL on Mac is Google (Maps and Chrome) with 70% whereas these URLs don't show up at all for Windows users
5. The stacks are slightly different.
* Frame 2 on Windows is in mozilla::layers::ImageLayerComposite::RenderLayer(nsIntRect const&)
* Frame 2 on Mac is in mozilla::layers::CanvasLayerComposite::RenderLayer(nsIntRect const&)
I'll follow-up here with the bug number for the Windows crashes once filed.
[Tracking Requested - why for this release]: regression in Firefox 39 based on crash-stats.
Note, the Windows version of this crash is bug 1187466 but those crashes started in Firefox 30 (again based on crash-stats).
tracking-firefox39:
--- → ?
Keywords: regression
Note that this is the #6 topcrash for Mac-specific crashes accounting for 0.78%.
Comment 3•10 years ago
|
||
From the crash log, it might be possible that the following return nullptr.
> TextureSourceBasic* source = texturedEffect->mTexture->AsSourceBasic();
When BasicCompositor::DrawQuad() is called, TexturedEffect and TextureSource need to be valid. One possibility is that TextureSource is not compatible to BasicCompositor.
Comment 4•10 years ago
|
||
BasicCompositor compatible TextureSource is created by BasicCompositor::CreateDataTextureSource(). The CreateDataTextureSource() is called in BufferTextureHost::Upload().
MacIOSurface case, MacIOSurfaceTextureHostBasic creates MacIOSurfaceTextureSourceBasic as BasicCompositor compatible TextureSource.
Comment 5•10 years ago
|
||
nical, do you know if there is a possibility that incompatible TextureSource is created when BasicCompositor is used?
Flags: needinfo?(nical.bugzilla)
fwiw, Firefox 39 is an automatic crash for me with Google Maps. about:crashes led me to this bug, as well as #1187466
Relevant crash reports that were all caused by simply visiting maps.google.com:
bp-c3e54866-ea09-4957-a7ed-67e9b2150809
8/9/15 04:16
bp-c1226648-3fb9-4744-9ccf-ecfe42150805
8/5/15 10:52
bp-0c374fdd-8529-45fc-b638-024522150731
7/31/15 12:41
9DC11FE5-0B67-4FFE-89FD-5FC8BF66C8AF
7/30/15 00:02
bp-a2f8421a-b154-4727-a349-c74ff2150726
7/26/15 07:09
bp-bb4bd41c-8f0f-49f9-83a5-dca182150724
7/24/15 16:15
bp-c6899c74-2b7e-4092-bb9e-d0bc52150721
7/21/15 07:57
bp-f418002b-0184-4b45-8ced-a6ad22150716
7/16/15 13:18
bp-1ca98412-5f50-49ab-ab2f-01c5d2150716
7/16/15 13:18
bp-2705edea-2ac1-401e-9f3b-8917d2150716
7/16/15 12:38
There would be far more crashes, but I've mostly remembered to not open Google Maps.
While I doubt I'm doing anything unique, let me know if you need me to try anything.
(In reply to dylan from comment #6)
> fwiw, Firefox 39 is an automatic crash for me with Google Maps.
What specifically are you doing on Google Maps leading up to this crash?
Do you still see this bug if you downgrade to Firefox 38?
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #7)
> (In reply to dylan from comment #6)
> > fwiw, Firefox 39 is an automatic crash for me with Google Maps.
>
> What specifically are you doing on Google Maps leading up to this crash?
> Do you still see this bug if you downgrade to Firefox 38?
Simply visiting maps.google.com. I've not tried a downgrade. I know it's not a universal problem as my other nearly identically configured Mac with Firefox 39.0.5 does not crash in this manner.
(In reply to dylan from comment #8)
> Simply visiting maps.google.com. I've not tried a downgrade. I know it's not
> a universal problem as my other nearly identically configured Mac with
> Firefox 39.0.5 does not crash in this manner.
Does this reproduce on the same system if you run with a new Firefox profile?
https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles
Comment 10•10 years ago
|
||
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #9)
> (In reply to dylan from comment #8)
> > Simply visiting maps.google.com. I've not tried a downgrade. I know it's not
> > a universal problem as my other nearly identically configured Mac with
> > Firefox 39.0.5 does not crash in this manner.
>
> Does this reproduce on the same system if you run with a new Firefox profile?
> https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-
> firefox-profiles
I have a user experiencing similar symptoms with this type of crash. I'll have them try a clean reinstall.. https://support.mozilla.org/en-US/questions/1077887
Updated•10 years ago
|
Comment 11•10 years ago
|
||
Summary: crash in mozilla::layers::BasicCompositor::DrawQuad(mozilla::gfx::RectTyped<T> const&, mozilla::gfx::RectTyped<T> const&, mozilla::layers::EffectChain const&, float, mozilla::gfx::Matrix4x4 const&) → [Mac OS X] crash in mozilla::layers::BasicCompositor::DrawQuad(mozilla::gfx::RectTyped<T> const&, mozilla::gfx::RectTyped<T> const&, mozilla::layers::EffectChain const&, float, mozilla::gfx::Matrix4x4 const&)
Comment 12•10 years ago
|
||
Issue reported on the French support board: https://forums.mozfr.org/viewtopic.php?p=801324#p801324
CR: https://crash-stats.mozilla.com/report/index/bp-afdcf1d4-e12f-49b3-bf22-3ccda2150914
Accélération graphique
Fenêtres avec accélération graphique 0/1 Basic (OMTC)
ID du périphérique 0x 126
ID du vendeur 0x8086
Prise en charge matérielle pour le décodage H264 false
Rendu WebGL ATI Technologies Inc. -- AMD Radeon HD 6750M OpenGL Engine
windowLayerManagerRemote true
Zoom/Panoramique asynchrones aucun
AzureCanvasBackend skia
AzureContentBackend quartz
AzureFallbackCanvasBackend none
AzureSkiaAccelerated 0
Updated•10 years ago
|
Crash Signature: [@ mozilla::layers::BasicCompositor::DrawQuad(mozilla::gfx::RectTyped<T> const&, mozilla::gfx::RectTyped<T> const&, mozilla::layers::EffectChain const&, float, mozilla::gfx::Matrix4x4 const&)] → [@ mozilla::layers::BasicCompositor::DrawQuad(mozilla::gfx::RectTyped<T> const&, mozilla::gfx::RectTyped<T> const&, mozilla::layers::EffectChain const&, float, mozilla::gfx::Matrix4x4 const&)]
[@ mozilla::layers::BasicCompositor::DrawQuad]
Updated•9 years ago
|
Flags: needinfo?(nical.bugzilla)
Assignee | ||
Comment 14•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/31981/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31981/
Attachment #8711112 -
Flags: review?(mstange)
Assignee | ||
Comment 15•9 years ago
|
||
This is just to stop the crash, and skip the effect. I'm assuming we somehow get accelerated filter, and can't deal with it? Markus to review mostly because he'd know what to do to properly fix the problem, rather than just crash avoidance. This will still crash in nightly+aurora, but at least we'll confirm why. Won't crash in beta/release.
Comment 16•9 years ago
|
||
Oh no, so many people on BasicCompositor on OS X before it was ready... :(
It looks like Mac people started getting BasicCompositor with 39 or a bit earlier, and then bug 1180688 put a stop to that in Firefox 42, but the patch that was uplifted to 40 and 41 only disabled BasicCompositor in the cases where people would have got an empty window.
As for the crash... source being null is one option, source->GetAsSurface(dest) being null is another option that could be happening. I doubt bug 1241665 will have fixed this - in the broken state that was fixed by bug 1241665, both source and the surface would have been non-null, but the surface would have had bad data.
Comment 17•9 years ago
|
||
Comment on attachment 8711112 [details]
MozReview Request: Bug 1187464: Part 1. We can get the wrong texture source type, so deal with it. r?mstange
https://reviewboard.mozilla.org/r/31981/#review28897
Attachment #8711112 -
Flags: review?(mstange) → review+
Comment 18•9 years ago
|
||
Hmm, it looks like we should null-check the result of source->GetAsSurface(dest) anyway. For example X11DataTextureSourceBasic::GetSurface(DrawTarget* aTarget) can explicitly return null.
Assignee | ||
Comment 19•9 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #18)
> Hmm, it looks like we should null-check the result of
> source->GetAsSurface(dest) anyway. For example
> X11DataTextureSourceBasic::GetSurface(DrawTarget* aTarget) can explicitly
> return null.
Good idea.
Assignee: nobody → milan
Assignee | ||
Comment 20•9 years ago
|
||
Attachment #8711781 -
Flags: review?(botond)
Assignee | ||
Comment 21•9 years ago
|
||
Attachment #8711783 -
Flags: review?(mstange)
Comment 22•9 years ago
|
||
Comment on attachment 8711783 [details] [diff] [review]
Part 3. More nullptr checks, including GetSurface. r=mstange
Review of attachment 8711783 [details] [diff] [review]:
-----------------------------------------------------------------
The rest looks good.
::: gfx/layers/basic/BasicCompositor.cpp
@@ +417,5 @@
> }
>
> switch (aEffectChain.mPrimaryEffect->mType) {
> case EffectTypes::SOLID_COLOR: {
> + if (sourceMask) {
This check is wrong. We want to be able to draw color layers even if they don't have a mask layer. FillRectWithMask handles the case when sourceMask is null.
Attachment #8711783 -
Flags: review?(mstange) → review-
Assignee | ||
Comment 23•9 years ago
|
||
Thanks for the quick review!
Attachment #8711783 -
Attachment is obsolete: true
Attachment #8711796 -
Flags: review?(mstange)
Updated•9 years ago
|
Attachment #8711796 -
Flags: review?(mstange) → review+
Updated•9 years ago
|
Attachment #8711781 -
Flags: review?(botond) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 24•9 years ago
|
||
Comment 25•9 years ago
|
||
this failed to apply:
renamed 1187464 -> 1187464.3.p2
applying 1187464.3.p2
patching file gfx/layers/basic/BasicCompositor.cpp
Hunk #3 FAILED at 441
1 out of 3 hunks FAILED -- saving rejects to file gfx/layers/basic/BasicCompositor.cpp.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh 1187464.3.p2
Flags: needinfo?(milan)
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 26•9 years ago
|
||
Must be something in inbound right now, I'll rebase as it gets through central.
Assignee | ||
Comment 27•9 years ago
|
||
Attachment #8711112 -
Attachment is obsolete: true
Flags: needinfo?(milan)
Assignee | ||
Comment 28•9 years ago
|
||
Attachment #8711781 -
Attachment is obsolete: true
Attachment #8714933 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Attachment #8714932 -
Attachment description: Bug 1187464: Part 1. We can get the wrong texture source type, so deal with it. Carry r=mstange → Part 1. We can get the wrong texture source type, so deal with it. Carry r=mstange
Attachment #8714932 -
Flags: review+
Assignee | ||
Comment 29•9 years ago
|
||
Attachment #8711796 -
Attachment is obsolete: true
Attachment #8714934 -
Flags: review+
Assignee | ||
Comment 30•9 years ago
|
||
Not sure why it didn't apply the first time, but here are the patches freshly rebased.
Keywords: checkin-needed
Comment 31•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/63e2a056311a
https://hg.mozilla.org/integration/mozilla-inbound/rev/0cee3cdcce99
https://hg.mozilla.org/integration/mozilla-inbound/rev/b64710d45b28
Keywords: checkin-needed
Comment 32•9 years ago
|
||
sorry had to back this out for reftest failures and problems like https://treeherder.mozilla.org/logviewer.html#?job_id=20974329&repo=mozilla-inbound
Flags: needinfo?(milan)
Comment 33•9 years ago
|
||
Assignee | ||
Comment 34•9 years ago
|
||
Comment on attachment 8714934 [details] [diff] [review]
Part 3. More nullptr checks, including GetSurface. Carry r=mstange
Review of attachment 8714934 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/layers/basic/BasicCompositor.cpp
@@ +176,5 @@
> SourceSurface *aMask,
> const Matrix* aMaskTransform)
> {
> + if (!aSource || !aMask) {
> + gfxWarning() << "DrawSurfaceWithTextureCoords problem " << gfx::hexa(aSource) << " and " << gfx::hexa(aMask);
Looks like this check is wrong as well - we should allow null aMask.
Attachment #8714934 -
Flags: review+ → review-
Comment 35•9 years ago
|
||
this has some nice perf wins (mostly on winxp):
http://alertmanager.allizom.org:8080/alerts.html?rev=b64710d45b2852b6f082411e6a066497aad50182&showAll=1&testIndex=0&platIndex=0
one regression about linxu64 tp5 xres, not sure why it jumps up 80%, but it still shows a win when looking at a longer history- we used to have much noisier xres data and higher data, this 80% regression only puts it halfway back to what it used to be.
Assignee | ||
Comment 36•9 years ago
|
||
Attachment #8714932 -
Attachment is obsolete: true
Flags: needinfo?(milan)
Attachment #8717101 -
Flags: review+
Assignee | ||
Comment 37•9 years ago
|
||
Attachment #8714933 -
Attachment is obsolete: true
Attachment #8717103 -
Flags: review+
Assignee | ||
Comment 38•9 years ago
|
||
Attachment #8714934 -
Attachment is obsolete: true
Attachment #8717106 -
Flags: review+
Assignee | ||
Comment 39•9 years ago
|
||
Assignee | ||
Comment 40•9 years ago
|
||
Comment on attachment 8717106 [details] [diff] [review]
Part 3. More nullptr checks, including GetSurface. Carry r=mstange
Better to review this change, since it is different than the previous version.
Attachment #8717106 -
Flags: review+ → review?(mstange)
Updated•9 years ago
|
Attachment #8717106 -
Flags: review?(mstange) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 41•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/acb9cdf6473e
https://hg.mozilla.org/integration/mozilla-inbound/rev/688a04c23fef
https://hg.mozilla.org/integration/mozilla-inbound/rev/213814f2dccc
Keywords: checkin-needed
Comment 42•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/acb9cdf6473e
https://hg.mozilla.org/mozilla-central/rev/688a04c23fef
https://hg.mozilla.org/mozilla-central/rev/213814f2dccc
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Assignee | ||
Updated•9 years ago
|
OS: Mac OS X → All
Summary: [Mac OS X] crash in mozilla::layers::BasicCompositor::DrawQuad(mozilla::gfx::RectTyped<T> const&, mozilla::gfx::RectTyped<T> const&, mozilla::layers::EffectChain const&, float, mozilla::gfx::Matrix4x4 const&) → crash in mozilla::layers::BasicCompositor::DrawQuad(mozilla::gfx::RectTyped<T> const&, mozilla::gfx::RectTyped<T> const&, mozilla::layers::EffectChain const&, float, mozilla::gfx::Matrix4x4 const&)
You need to log in
before you can comment on or make changes to this bug.
Description
•