Closed
Bug 1291296
Opened 8 years ago
Closed 8 years ago
Crash in mozilla::layers::ShadowLayerForwarder::UseTextures
Categories
(Core :: Graphics: Canvas2D, defect)
Tracking
()
RESOLVED
FIXED
mozilla51
People
(Reporter: marcia, Assigned: nical)
References
Details
(Keywords: crash, Whiteboard: [gfx-noted])
Crash Data
Attachments
(1 file, 1 obsolete file)
4.77 KB,
patch
|
ethlin
:
review+
ritu
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is
report bp-432a6aee-5fd3-4619-9097-8a6802160802.
=============================================================
New crash showing up on the Nightly radar: http://bit.ly/2ay3Aft. Appears to have started using 20160731030203. All Windows crashes with highest concentration of Windows 10.
Some of the URLs are videos.
ni on nsilva for possible ideas since he appears to have been working in that code.
Flags: needinfo?(nical.bugzilla)
Assignee | ||
Comment 1•8 years ago
|
||
Excellent. This is basically bug 1286437 with a more interesting stack trace.
Assignee: nobody → nical.bugzilla
Flags: needinfo?(nical.bugzilla)
Assignee | ||
Comment 2•8 years ago
|
||
Not asking review yet because I haven't verified that the patch fixes the issue. I have only been able to reproduce the crash on one laptop and only with an optimized build, I'll test the patch on that configuration tomorrow.
Updated•8 years ago
|
Whiteboard: [gfx-noted]
Assignee | ||
Comment 3•8 years ago
|
||
Comment on attachment 8777397 [details] [diff] [review]
Support switching a shared buffer provider to a new and/or incompatible PCompositorBridge
Review of attachment 8777397 [details] [diff] [review]:
-----------------------------------------------------------------
Unfortunately I am only able to reproduce the bug with a download nightly build, not with my own builds. That said, this patch makes it impossible to run into the assertion of the crash (incompatible channels when updating a canvas), because it checks that the channels are compatible before sending and tries to do something sensible (copy the data into something compatible with the new channel) in case it they are not.
Attachment #8777397 -
Flags: review?(ethlin)
Comment 4•8 years ago
|
||
(In reply to Nicolas Silva [:nical] from comment #3)
> Comment on attachment 8777397 [details] [diff] [review]
> Support switching a shared buffer provider to a new and/or incompatible
> PCompositorBridge
>
> Review of attachment 8777397 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Unfortunately I am only able to reproduce the bug with a download nightly
> build, not with my own builds. That said, this patch makes it impossible to
> run into the assertion of the crash (incompatible channels when updating a
> canvas), because it checks that the channels are compatible before sending
> and tries to do something sensible (copy the data into something compatible
> with the new channel) in case it they are not.
I can reproduce this bug with my own build by dragging a canvas tab to another window with e10s off. The behavior is similar to bug 1250954.
Comment 5•8 years ago
|
||
Comment on attachment 8777397 [details] [diff] [review]
Support switching a shared buffer provider to a new and/or incompatible PCompositorBridge
Review of attachment 8777397 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/layers/PersistentBufferProvider.cpp
@@ +147,5 @@
> + // The forwarder should not change most of the time.
> + return true;
> + }
> +
> + if (mFwd->AsTextureForwarder() == aFwd->AsTextureForwarder() ||
Should this be '!='?
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to Ethan Lin[:ethlin] from comment #5)
> Should this be '!='?
oh my. Yes it should!
Assignee | ||
Comment 7•8 years ago
|
||
Comment on attachment 8777397 [details] [diff] [review]
Support switching a shared buffer provider to a new and/or incompatible PCompositorBridge
Cancelling the review, there is another adjustement I need t make to this patch regarding the activity tracker. I am also able to reproduce the bug with e10s disabled with my own builds now (thanks for the tip!).
Attachment #8777397 -
Flags: review?(ethlin)
Assignee | ||
Comment 8•8 years ago
|
||
I have been able to verify that this patch fixes the problem.
Attachment #8777397 -
Attachment is obsolete: true
Attachment #8777779 -
Flags: review?(ethlin)
Comment 9•8 years ago
|
||
Comment on attachment 8777779 [details] [diff] [review]
Support switching a shared buffer provider to a new and/or incompatible PCompositorBridge
Review of attachment 8777779 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/layers/PersistentBufferProvider.cpp
@@ +170,5 @@
> + BackendSelector::Canvas,
> + TextureFlags::DEFAULT,
> + TextureAllocationFlags::ALLOC_DEFAULT
> + );
> +
I think we should have 'MOZ_ASSERT(newTexture)' here.
Attachment #8777779 -
Flags: review?(ethlin) → review+
Comment 10•8 years ago
|
||
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d59fd1ff4bec
Make it possible to change a PersistentBufferProvider's Forwarder without crashing. r=ethlin
Assignee | ||
Updated•8 years ago
|
Blocks: 1285271
status-firefox50:
--- → affected
Comment 11•8 years ago
|
||
[Tracking Requested - why for this release]: #6 in the top crasher list for Firefox 50.
tracking-firefox50:
--- → ?
Comment 12•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Hello Nicolas, could you please request uplift to Aurora50? Thanks!
Flags: needinfo?(nical.bugzilla)
Assignee | ||
Comment 14•8 years ago
|
||
Comment on attachment 8777779 [details] [diff] [review]
Support switching a shared buffer provider to a new and/or incompatible PCompositorBridge
Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: Crashes
[Describe test coverage new/current, TreeHerder]: None.
[Risks and why]: Low risk, has been baking on central for a while.
[String/UUID change made/needed]: None.
Flags: needinfo?(nical.bugzilla)
Attachment #8777779 -
Flags: approval-mozilla-aurora?
Comment on attachment 8777779 [details] [diff] [review]
Support switching a shared buffer provider to a new and/or incompatible PCompositorBridge
Crash fix, the data from Nightly showed this still happened on 08-16 (even with the fix) but I still think this is worth uplifting to Aurora50.
Attachment #8777779 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 16•8 years ago
|
||
bugherder uplift |
Comment 17•8 years ago
|
||
Crash volume for signature 'mozilla::layers::ShadowLayerForwarder::UseTextures':
- nightly (version 51): 46 crashes from 2016-08-01.
- aurora (version 50): 414 crashes from 2016-08-01.
- beta (version 49): 3 crashes from 2016-08-02.
- release (version 48): 2 crashes from 2016-07-25.
- esr (version 45): 1 crash from 2016-05-02.
Crash volume on the last weeks (Week N is from 08-22 to 08-28):
W. N-1 W. N-2 W. N-3
- nightly 10 3 26
- aurora 115 241 48
- beta 2 0 0
- release 0 0 2
- esr 0 0 0
Affected platforms: Windows, Mac OS X
Crash rank on the last 7 days:
Browser Content Plugin
- nightly #72 #207
- aurora #32
- beta #13586
- release
- esr
status-firefox48:
--- → affected
status-firefox49:
--- → affected
status-firefox-esr45:
--- → affected
Updated•8 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•