Closed Bug 1322741 Opened 8 years ago Closed 7 years ago

Crash in mozilla::layers::SyncObjectD3D11::Init

Categories

(Core :: Graphics: Layers, defect)

Product:
Core
Component:
Graphics: Layers
Platform:
x86
Windows 10
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: mccr8, Assigned: vliu)

References

Details

(Keywords: crash)

Crash Data

Attachments

(2 files, 8 obsolete files)

0001-Bug-1322741-Make-sure-ID3D11Device-is-up-to-date-in-.patch
4.28 KB, patch
Details | Diff | Splinter Review
0001-Bug-1322741-Uplift-Make-sure-ID3D11Device-is-up-to-d.patch
3.56 KB, patch
Details | Diff | Splinter Review 
This bug was filed from the Socorro interface and is 
report bp-e15a77f0-2507-4ae2-b9ca-55f5b2161205.
=============================================================

It looks like this is a new crash starting around 12-1, so maybe it is a regression from bug 1319557, but maybe it is just a signature change.
David, maybe you could look at this?
Flags: needinfo?(dvander)
From looked into the crash report, the crash happens in [1]

[1]: https://hg.mozilla.org/mozilla-central/annotate/d8f63b2935af/gfx/layers/d3d11/TextureD3D11.cpp#l1212

Since Crash Address is 0, the device might get nullptr. Adding a check to prevent further operations.
Attachment #8817793 - Flags: review?(dvander)
Assignee: nobody → vliu
Comment on attachment 8817793 [details] [diff] [review]
Add null pointer check for ID3D11Device.

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

Hrm, this crash is very confusing. Getting a null device should be impossible, but this crash started appearing very recently.

All of these crash reports have device resets. FinalizeFrame is only called if the device *hasn't* reset [1]. So somehow, we are painting immediately after *acknowledging* a device reset, but *before* updating a TextureFactoryIdentifier (which contains the SyncObject).

In content processes - which is where these crashes are - device resets are handled here [2]. This message is sent by the parent process here [3].

My guess is, when the parent process is sending messages to all tabs, it's not sending them atomically for a given process. The content process can end up painting in between handling the device reset for two different tabs. The result is that it can accidentally paint without a device, which is bad. I think we should fix this more directly.

[1] http://searchfox.org/mozilla-central/rev/594937fec2e2fc45fa9308ba2fb964816631f017/gfx/layers/client/ClientLayerManager.cpp#655
[2] http://searchfox.org/mozilla-central/rev/594937fec2e2fc45fa9308ba2fb964816631f017/gfx/layers/ipc/CompositorBridgeChild.cpp#370
[3] http://searchfox.org/mozilla-central/rev/594937fec2e2fc45fa9308ba2fb964816631f017/gfx/layers/ipc/CompositorBridgeParent.cpp#1819
Attachment #8817793 - Flags: review?(dvander)
The easiest fix would be to change CompositorBridgeParent::ResetCompositorTask so it sends batches messages per process instead of per-tab. It's a little annoying, since it'd need 2-3 steps: one to collect a list of layer ids per CPCBP, and a second to send message. As a bonus we'd be able to back out bug 1316788 after.

Another idea would be to ditch the message entirely, and have each ContentChild drive the reset process since it knows all of its tabs. We already do that for GPU process crashes. But we'd need to redo all of the "update acknowledgement" code, which makes me think the first solution is easier.

Thoughts?
Flags: needinfo?(dvander) → needinfo?(vliu)
This the #9 topcrash in Nightly 2016121603020.

Vincent, any comments on dvander's proposal?
Sorry to late reply.

(In reply to David Anderson [:dvander] from comment #4)
> The easiest fix would be to change
> CompositorBridgeParent::ResetCompositorTask so it sends batches messages per
> process instead of per-tab. It's a little annoying, since it'd need 2-3
> steps: one to collect a list of layer ids per CPCBP, and a second to send
> message. As a bonus we'd be able to back out bug 1316788 after.
> Thoughts?

For each tab, it has its own Compositor Bridge to communicate with parent. Once ResetCompositorTask sends batches messages for process instead of per-tab, should we use one of these CB to send? Or is there any other message channel suitable for this? 


After some discussions internally, here came out another simpler idea. When message was sent from 
[1], we add an extra flag to record SyncObject per tab. In content side, when SyncObject was updated in [2], we modified this flag to like *updated* per tab. If so, DidRenderingDeviceReset() in [3] could check this flag to judge whether FinalizeFrame() should be called or not. If SyncObject didn't updated in this frame, next frame will keep tracking till SyncObject has updated.  

It seems that it is more simpler. How do you think for this proposal?

[1]: http://searchfox.org/mozilla-central/rev/594937fec2e2fc45fa9308ba2fb964816631f017/gfx/layers/ipc/CompositorBridgeParent.cpp#1819
[2]: http://searchfox.org/mozilla-central/rev/594937fec2e2fc45fa9308ba2fb964816631f017/gfx/layers
/ipc/CompositorBridgeChild.cpp#375
[3]: http://searchfox.org/mozilla-central/rev/594937fec2e2fc45fa9308ba2fb964816631f017/gfx/layers/client/ClientLayerManager.cpp#655
Flags: needinfo?(vliu) → needinfo?(dvander)
(In reply to Vincent Liu[:vliu] from comment #6)
> Sorry to late reply.
> 
> (In reply to David Anderson [:dvander] from comment #4)

> 
> After some discussions internally, here came out another simpler idea. When
> message was sent from 
> [1], we add an extra flag to record SyncObject per tab. In content side,
> when SyncObject was updated in [2], we modified this flag to like *updated*
> per tab. If so, DidRenderingDeviceReset() in [3] could check this flag to
> judge whether FinalizeFrame() should be called or not. If SyncObject didn't
> updated in this frame, next frame will keep tracking till SyncObject has
> updated.  
> 

For this extra flag I mentioned, aSeqNo might helpful for both checking conten device and SyncObject.
When CompositorBridgeChild::RecvCompositorUpdated() was called in content side, SeqNo can stored in
data structure of content device and SyncObject separately. When tick was raised, we can check both SeqNo 
in DidRenderingDeviceReset(). If they were the same in specific tab, it means both content device and SyncObject were updated. If so, we can call FinalizeFrame(). But if they were not the same, skipping in this tick and check again in next frame. How do you think?

[4]: https://dxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/CompositorBridgeChild.cpp#369
Flags: needinfo?(dvander)
Flags: needinfo?(dvander)
Attached patch WIP-1.patch (obsolete) — — Splinter Review 
Hi Jerry,
The attached WIP intends to implement code based on Comment 7. Could you please have feedback for me? Thanks
Attachment #8817793 - Attachment is obsolete: true
Attachment #8821000 - Flags: feedback?(hshih)
(In reply to Vincent Liu[:vliu] from comment #6)
>
> For each tab, it has its own Compositor Bridge to communicate with parent.
> Once ResetCompositorTask sends batches messages for process instead of
> per-tab, should we use one of these CB to send? Or is there any other
> message channel suitable for this? 

There is only one PCompositorBridge per content process. We differentiate tabs via layer tree ids.

> After some discussions internally, here came out another simpler idea. When
> message was sent from 
> [1], we add an extra flag to record SyncObject per tab. In content side,
> when SyncObject was updated in [2], we modified this flag to like *updated*
> per tab. If so, DidRenderingDeviceReset() in [3] could check this flag to
> judge whether FinalizeFrame() should be called or not. If SyncObject didn't
> updated in this frame, next frame will keep tracking till SyncObject has
> updated. 

This is still not fixing the root of the problem, which is that we run the event loop in between resetting devices and layer invalidation logic. They should run in sequence.

I think the best solution is #2, it'd look something like:
 (1) compositor puts each LayerTransactionParent into a "not accepting updates" state
 (2) compositor notifies UI process of device reset, with a generation #
 (3) UI forwards this to each ContentParent, then to each ContentChild
 (4) each ContentChild does a miniature version of TabChild::ReinitRendering. at the end we send that generation # back to the compositor (for each tab child), and that flips each LayerTransactionParent back into "accepting updates" mode
Flags: needinfo?(dvander)
That said if we just want to fix this quickly: I'd rather take the original patch. When we end up doing the full fix, the sequence numbers will go away, so better to not add more uses of them.

The original patch can be hardened by storing the ID3D11Device in the SyncObjectD3D11, and then skipping FinalizeFrame if they're null or different.
Comment on attachment 8821000 [details] [diff] [review]
WIP-1.patch

wait for new version of patch
Attachment #8821000 - Flags: feedback?(hshih)
(In reply to David Anderson [:dvander] from comment #10)
> That said if we just want to fix this quickly: I'd rather take the original
> patch. When we end up doing the full fix, the sequence numbers will go away,
> so better to not add more uses of them.
> 
> The original patch can be hardened by storing the ID3D11Device in the
> SyncObjectD3D11, and then skipping FinalizeFrame if they're null or
> different.

Hi David,
The proposed patch keep a reference of ID3D11Device in [1] and check it before FinalizeFrame.
Could you please have review. Thanks


[1]: https://dxr.mozilla.org/mozilla-central/source/gfx/layers/client/ClientLayerManager.cpp#611
Attachment #8821841 - Flags: review?(dvander)
Attachment #8821000 - Attachment is obsolete: true
Comment on attachment 8821841 [details] [diff] [review]
Make sure ID3D11Device is up-to-date in SyncObjectD3D11.

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

::: gfx/layers/d3d11/TextureD3D11.cpp
@@ +1239,5 @@
>    mD3D11SyncedTextures.push_back(aTexture);
>  }
>  
> +bool
> +SyncObjectD3D11::IsSyncObjectUpdated()

"IsSyncObjectValid" would be a better name.

@@ +1242,5 @@
> +bool
> +SyncObjectD3D11::IsSyncObjectUpdated()
> +{
> +  RefPtr<ID3D11Device> dev = DeviceManagerDx::Get()->GetContentDevice();
> +  if (!dev || (dev == mD3D11Device)) {

Should this be != mD3D11Device?
Attachment #8821841 - Flags: review?(dvander) → review+
(In reply to David Anderson [:dvander] from comment #13)
> Comment on attachment 8821841 [details] [diff] [review]
> Make sure ID3D11Device is up-to-date in SyncObjectD3D11.
> 
> Review of attachment 8821841 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/d3d11/TextureD3D11.cpp
> @@ +1239,5 @@
> >    mD3D11SyncedTextures.push_back(aTexture);
> >  }
> >  
> > +bool
> > +SyncObjectD3D11::IsSyncObjectUpdated()
> 
> "IsSyncObjectValid" would be a better name.
> 

agree

> @@ +1242,5 @@
> > +bool
> > +SyncObjectD3D11::IsSyncObjectUpdated()
> > +{
> > +  RefPtr<ID3D11Device> dev = DeviceManagerDx::Get()->GetContentDevice();
> > +  if (!dev || (dev == mD3D11Device)) {
> 
> Should this be != mD3D11Device?

Yes, you are right. it should return false once dev is null or different. Thanks.
Modified patch based on Comment 13.
Attachment #8821841 - Attachment is obsolete: true
Pushed by vliu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef2dd5f6e702
Make sure ID3D11Device is up-to-date in SyncObjectD3D11. r=dvander
Status: NEW → ASSIGNED
Keywords: leave-open
Hello Vincent, can you also upload this patch to aurora and beta ?
The crashes in this bug are originally from Bug 1160157 and your patch can also fix them.
Flags: needinfo?(vliu)
Backed out for mass failing reftests on Windows 8 x64:

https://hg.mozilla.org/integration/mozilla-inbound/rev/df0b580da7bda974c6bff33f8503289f01828dd4

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=ef2dd5f6e702292c74bbabc1606bc6d54609ce04
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=41094313&repo=mozilla-inbound

E.g.
05:27:51     INFO - REFTEST TEST-UNEXPECTED-FAIL | file:///C:/slave/test/build/tests/reftest/tests/layout/reftests/reftest-sanity/html-vs-xhtml-by-extension.html != file:///C:/slave/test/build/tests/reftest/tests/layout/reftests/reftest-sanity/html-vs-xhtml-by-extension.xhtml | image comparison
(In reply to Kevin Chen[:kechen] (UTC + 8) from comment #18)
> Hello Vincent, can you also upload this patch to aurora and beta ?
> The crashes in this bug are originally from Bug 1160157 and your patch can
> also fix them.
It seems that my patch fails on reftests and still need to figure it out. Yes, I will upload to aurora and beta once the patch is ready.
Flags: needinfo?(vliu)
Attached patch WIP-2.patch (obsolete) — — Splinter Review 
Hi David,
The landed patch was Backed out for mass failing reftests on Windows 8 x64. Please see Comment 19 for detail. 
In this testing environment, GPU was enabled and each tab lived in its own content process. From [1], UI process didn't
create ContentDevice. Instead, each ContentDevice lived in each content process. 

[1]: https://dxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxWindowsPlatform.cpp#1469

With my patch, UpdateContentDevice() was called only for device-reset consideration. But not considering in initialization phase.  

 void
 ClientLayerManager::UpdateTextureFactoryIdentifier(const TextureFactoryIdentifier& aNewIdentifier)
 {
   mForwarder->IdentifyTextureHost(aNewIdentifier);
+  if (mForwarder->GetSyncObject()) {
+    mForwarder->GetSyncObject()->UpdateContentDevice();
+  }
 }

To make sure mD3D11Device was updated in initialization phase for each tab, I put it in SyncObjectD3D11::Init() and also called in SyncObjectD3D11
construction. 

--- a/gfx/layers/d3d11/TextureD3D11.cpp
+++ b/gfx/layers/d3d11/TextureD3D11.cpp
@@ -1193,26 +1193,31 @@ IntSize
 CompositingRenderTargetD3D11::GetSize() const
 {
   return TextureSourceD3D11::GetSize();
 }
 
 SyncObjectD3D11::SyncObjectD3D11(SyncHandle aSyncHandle)
  : mSyncHandle(aSyncHandle)
 {
+  Init();
 }
 
 bool
 SyncObjectD3D11::Init()
 {
   if (mKeyedMutex) {
     return true;
   }
 
   RefPtr<ID3D11Device> device = DeviceManagerDx::Get()->GetContentDevice();
+  if (!device) {
+    return;
+  }
+  mD3D11Device = device;
 
   HRESULT hr = device->OpenSharedResource(
     mSyncHandle,
     __uuidof(ID3D11Texture2D),
     (void**)(ID3D11Texture2D**)getter_AddRefs(mD3D11Texture));
   if (FAILED(hr) || !mD3D11Texture) {
     gfxCriticalNote << "Failed to OpenSharedResource for SyncObjectD3D11: " << hexa(hr);
     if (!CompositorBridgeChild::CompositorIsInGPUProcess() &&
@@ -1234,16 +1239,35 @@ SyncObjectD3D11::Init()

Some questions I am not quite sure.

1. Is it posible ContentDevice was not ready at the moment when Init() was called in SyncObjectD3D11 construction in initialization phase?
2. Is it possible chaging the behavior for device-reset case?
3. Or do you have other concern I didn't consider?
Attachment #8823280 - Flags: feedback?(dvander)
(In reply to Vincent Liu[:vliu] from comment #21)
>
> 1. Is it posible ContentDevice was not ready at the moment when Init() was
> called in SyncObjectD3D11 construction in initialization phase?

No, the device should exist by then.
Comment on attachment 8823280 [details] [diff] [review]
WIP-2.patch

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

::: gfx/layers/client/ClientLayerManager.cpp
@@ +609,5 @@
>  ClientLayerManager::UpdateTextureFactoryIdentifier(const TextureFactoryIdentifier& aNewIdentifier)
>  {
>    mForwarder->IdentifyTextureHost(aNewIdentifier);
> +  if (mForwarder->GetSyncObject()) {
> +    mForwarder->GetSyncObject()->UpdateContentDevice();

(In reply to Vincent Liu[:vliu] from comment #21)
>
> 2. Is it possible chaging the behavior for device-reset case?

For the device reset case, the old handle is invalid, and ShadowLayerForwarder will create a new SyncObject. Now that you've added Init() to the constructor, I don't think you need UpdateContentDevice anymore.

> 3. Or do you have other concern I didn't consider?

You're making it non-lazy, which means now we'll acquire the texture before the first paint, instead of during the first paint. I don't think that matters though :)
Attachment #8823280 - Flags: review+
Attachment #8823280 - Flags: feedback?(dvander)
Attachment #8823280 - Flags: feedback+
See Also: → 1160157
Reattach the patch based on review on Comment 23.
Attachment #8822126 - Attachment is obsolete: true
Attachment #8823280 - Attachment is obsolete: true
Pushed by vliu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4d2c65de6f51
Make sure ID3D11Device is up-to-date in SyncObjectD3D11. r=dvander
sorry had to back this out for reftest failures like https://treeherder.mozilla.org/logviewer.html#?job_id=66395401&repo=mozilla-inbound
Flags: needinfo?(vliu)
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc0722b8389f
Backed out changeset 4d2c65de6f51 for causing e10s reftest failures on windows
Hi David,
The previous patch still fails in webgl-color-test.html. I think the reason should relative to non-lazy in constructor.
This proposed patch only put ID3D11Device into mD3D11Device. For Init(), the behavior keeps to original. This patch pass
the failed test case. Could you please have a review? Thanks
Attachment #8825358 - Flags: review?(dvander)
Attachment #8823963 - Attachment is obsolete: true
Comment on attachment 8825358 [details] [diff] [review]
Make sure ID3D11Device is up-to-date in SyncObjectD3D11.

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

::: gfx/layers/d3d11/TextureD3D11.cpp
@@ +1198,5 @@
>  SyncObjectD3D11::SyncObjectD3D11(SyncHandle aSyncHandle)
>   : mSyncHandle(aSyncHandle)
>  {
> +  RefPtr<ID3D11Device> device = DeviceManagerDx::Get()->GetContentDevice();
> +  mD3D11Device = device;

nit: mD3D11Device = DeviceManagerDx::...?
Attachment #8825358 - Flags: review?(dvander) → review+
Pushed by vliu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/871dc9ef3423
Make sure ID3D11Device is up-to-date in SyncObjectD3D11. r=dvander
Hello Vincent, could you help me to uplift the fix to aurora and beta ?
This fix can fix some crashes in bug 1160157.
Flags: needinfo?(vliu)
Comment on attachment 8825358 [details] [diff] [review]
Make sure ID3D11Device is up-to-date in SyncObjectD3D11.

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: may not fix some crashes in bug 1160157
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]: No 
[List of other uplifts needed for the feature/fix]: No
[Is the change risky?]: Low.
[Why is the change risky/not risky?]:
[String changes made/needed]:
Flags: needinfo?(vliu)
Attachment #8825358 - Flags: approval-mozilla-beta?
Attachment #8825358 - Flags: approval-mozilla-aurora?
Attachment #8825358 - Flags: approval-mozilla-beta?
Attachment #8825358 - Flags: approval-mozilla-aurora?
Re-attached the landed code to uplift.
Attachment #8825358 - Attachment is obsolete: true
Comment on attachment 8828718 [details] [diff] [review]
0001-Bug-1322741-Make-sure-ID3D11Device-is-up-to-date-in-.patch

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: may not fix some crashes in bug 1160157
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]: No 
[List of other uplifts needed for the feature/fix]: No
[Is the change risky?]: Low.
[Why is the change risky/not risky?]:
[String changes made/needed]:
Attachment #8828718 - Flags: approval-mozilla-beta?
Attachment #8828718 - Flags: approval-mozilla-aurora?
(In reply to Vincent Liu[:vliu] from comment #37)
> Comment on attachment 8828718 [details] [diff] [review]
> 0001-Bug-1322741-Make-sure-ID3D11Device-is-up-to-date-in-.patch
> 
> Approval Request Comment
> [Feature/Bug causing the regression]:
> [User impact if declined]: may not fix some crashes in bug 1160157
> [Is this code covered by automated tests?]: No
> [Has the fix been verified in Nightly?]:
> [Needs manual test from QE? If yes, steps to reproduce]: No 
> [List of other uplifts needed for the feature/fix]: No
> [Is the change risky?]: Low.
> [Why is the change risky/not risky?]:
> [String changes made/needed]:

Could you please answer the questions in the template?  Thanks.

Bug 1160157 says "nightly and aurora only", and all the reports for the signature associated with this bug are on 53.  Why is this needed in beta?
Flags: needinfo?(vliu)
Comment on attachment 8828718 [details] [diff] [review]
0001-Bug-1322741-Make-sure-ID3D11Device-is-up-to-date-in-.patch

Clearing aurora approval flag, this landed in 53 which is now aurora (or will be later today).
Attachment #8828718 - Flags: approval-mozilla-aurora?
Hello Julien,

The thing is that when was Bug 1160157 filed, it only crashed in aurora and night channel; however, in fact, versions after 39 are all affected now.

Please help me to uplift this patch to beta and I will change the Bug 1160157 's title so that won't confuse others.

Thank you.
Flags: needinfo?(vliu)
Comment on attachment 8828718 [details] [diff] [review]
0001-Bug-1322741-Make-sure-ID3D11Device-is-up-to-date-in-.patch

try to fix d3d11 crash, beta52+
Attachment #8828718 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
needs rebasing for beta 

merging gfx/layers/d3d11/TextureD3D11.cpp
merging gfx/layers/d3d11/TextureD3D11.h
warning: conflicts while merging gfx/layers/d3d11/TextureD3D11.cpp! (edit, then use 'hg resolve --mark')
warning: conflicts while merging gfx/layers/d3d11/TextureD3D11.h! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(vliu)
(In reply to Carsten Book [:Tomcat] from comment #42)
> needs rebasing for beta 
> 
> merging gfx/layers/d3d11/TextureD3D11.cpp
> merging gfx/layers/d3d11/TextureD3D11.h
> warning: conflicts while merging gfx/layers/d3d11/TextureD3D11.cpp! (edit,
> then use 'hg resolve --mark')
> warning: conflicts while merging gfx/layers/d3d11/TextureD3D11.h! (edit,
> then use 'hg resolve --mark')
> abort: unresolved conflicts, can't continue
> (use 'hg resolve' and 'hg graft --continue')

Ok. I will rebase the patch and request uplift again.
Flags: needinfo?(vliu)
Carry r+ from Comment 30.
Attachment #8828718 - Attachment is obsolete: true
(In reply to Kevin Chen[:kechen] (UTC + 8) from comment #40)
> Hello Julien,
> 
> The thing is that when was Bug 1160157 filed, it only crashed in aurora and
> night channel; however, in fact, versions after 39 are all affected now.
> 
> Please help me to uplift this patch to beta and I will change the Bug
> 1160157 's title so that won't confuse others.
> 
> Thank you.

Rebased the patch and proposed to uplift to beta.
Comment on attachment 8830122 [details] [diff] [review]
0001-Bug-1322741-Uplift-Make-sure-ID3D11Device-is-up-to-d.patch

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: may not fix some crashes in bug 1160157
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]: No 
[List of other uplifts needed for the feature/fix]: No
[Is the change risky?]: Low.
[Why is the change risky/not risky?]:
[String changes made/needed]:
Attachment #8830122 - Flags: approval-mozilla-beta?
Comment on attachment 8830122 [details] [diff] [review]
0001-Bug-1322741-Uplift-Make-sure-ID3D11Device-is-up-to-d.patch

You can stick with the previous approval.
Attachment #8830122 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/releases/mozilla-beta/rev/1cc12032b6f7

I have no idea what the status for 53/54 are supposed to be here, though.
status-firefox52: --- → fixed
Setting qe-verify- per Comment 46.
Flags: qe-verify-
(In reply to Ryan VanderMeulen [:RyanVM] from comment #48)
> https://hg.mozilla.org/releases/mozilla-beta/rev/1cc12032b6f7
> 
> I have no idea what the status for 53/54 are supposed to be here, though.

Looks like it landed on central on January 18th, so it would be in both.
changeset:   329769:871dc9ef3423
user:        vincentliu <vliu@mozilla.com>
date:        Wed Jan 18 10:12:42 2017 +0800
summary:     Bug 1322741 - Make sure ID3D11Device is up-to-date in SyncObjectD3D11. r=dvander
Flags: needinfo?(ryanvm)
I guess the reason it was unclear was because the bug was left open after that commit landed. Are we intending to do more here still?
Flags: needinfo?(ryanvm) → needinfo?(milan)
Thanks, makes sense.

Vincent, do we expect more work in this bug, or should we close it?
Flags: needinfo?(milan) → needinfo?(vliu)
(In reply to Milan Sreckovic [:milan] from comment #52)
> Thanks, makes sense.
> 
> Vincent, do we expect more work in this bug, or should we close it?

At this point I didn't see any similar bug after this patch landed. Based on this, I will close this bug.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: needinfo?(vliu)
Resolution: --- → FIXED
status-firefox53: --- → fixed
Keywords: leave-open
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: