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)

Unspecified
All
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox39 - wontfix
firefox40 --- affected
firefox41 --- affected
firefox42 --- affected
firefox47 --- fixed

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).
Keywords: regression
Note that this is the #6 topcrash for Mac-specific crashes accounting for 0.78%.
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.
BasicCompositor compatible TextureSource is created by BasicCompositor::CreateDataTextureSource(). The CreateDataTextureSource() is called in BufferTextureHost::Upload(). MacIOSurface case, MacIOSurfaceTextureHostBasic creates MacIOSurfaceTextureSourceBasic as BasicCompositor compatible TextureSource.
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
(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
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&)
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
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]
Flags: needinfo?(nical.bugzilla)
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.
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 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+
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.
(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
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-
Thanks for the quick review!
Attachment #8711783 - Attachment is obsolete: true
Attachment #8711796 - Flags: review?(mstange)
Attachment #8711796 - Flags: review?(mstange) → review+
Attachment #8711781 - Flags: review?(botond) → review+
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)
Must be something in inbound right now, I'll rebase as it gets through central.
See Also: → 1187466
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+
Not sure why it didn't apply the first time, but here are the patches freshly rebased.
Keywords: checkin-needed
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 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-
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.
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)
Attachment #8717106 - Flags: review?(mstange) → review+
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.

Attachment

General

Created:
Updated:
Size: