Closed
Bug 1322741
Opened 8 years ago
Closed 7 years ago
Crash in mozilla::layers::SyncObjectD3D11::Init
Categories
(Core :: Graphics: Layers, defect)
Tracking
()
RESOLVED
FIXED
mozilla53
People
(Reporter: mccr8, Assigned: vliu)
References
Details
(Keywords: crash)
Crash Data
Attachments
(2 files, 8 obsolete files)
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.
Assignee | ||
Comment 2•8 years ago
|
||
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 | ||
Updated•7 years ago
|
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)
Comment 5•7 years ago
|
||
This the #9 topcrash in Nightly 2016121603020. Vincent, any comments on dvander's proposal?
Assignee | ||
Comment 6•7 years ago
|
||
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)
Assignee | ||
Comment 7•7 years ago
|
||
(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)
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(dvander)
Assignee | ||
Comment 8•7 years ago
|
||
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 11•7 years ago
|
||
Comment on attachment 8821000 [details] [diff] [review] WIP-1.patch wait for new version of patch
Attachment #8821000 -
Flags: feedback?(hshih)
Assignee | ||
Comment 12•7 years ago
|
||
(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)
Assignee | ||
Updated•7 years ago
|
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+
Assignee | ||
Comment 14•7 years ago
|
||
(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.
Assignee | ||
Comment 15•7 years ago
|
||
Modified patch based on Comment 13.
Assignee | ||
Updated•7 years ago
|
Attachment #8821841 -
Attachment is obsolete: true
Assignee | ||
Comment 16•7 years ago
|
||
try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=808b15a897099be7e059f043c93f7a167bbee5de
Comment 17•7 years ago
|
||
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
Assignee | ||
Updated•7 years ago
|
Status: NEW → ASSIGNED
Keywords: leave-open
Comment 18•7 years ago
|
||
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)
Comment 19•7 years ago
|
||
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
Assignee | ||
Comment 20•7 years ago
|
||
(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)
Assignee | ||
Comment 21•7 years ago
|
||
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+
Assignee | ||
Comment 24•7 years ago
|
||
Reattach the patch based on review on Comment 23.
Assignee | ||
Updated•7 years ago
|
Attachment #8822126 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8823280 -
Attachment is obsolete: true
Assignee | ||
Comment 25•7 years ago
|
||
try result: https://treeherder.mozilla.org/#/jobs?repo=try&tochange=b309049b0b8c247387d4537e5c1d1ffa84b71346&fromchange=c0e5dde3f9a33538b20279a476f6679404ee5dce&selectedJob=66076034
Comment 26•7 years ago
|
||
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
Comment 27•7 years ago
|
||
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)
Comment 28•7 years ago
|
||
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
Assignee | ||
Comment 29•7 years ago
|
||
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)
Assignee | ||
Updated•7 years ago
|
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+
Assignee | ||
Comment 31•7 years ago
|
||
try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a059a7abdb160b09ddfb1ebaf87bb23c8c1b4f4
Flags: needinfo?(vliu)
Comment 32•7 years ago
|
||
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
Comment 33•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/871dc9ef3423
Comment 34•7 years ago
|
||
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)
Assignee | ||
Comment 35•7 years ago
|
||
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?
Assignee | ||
Updated•7 years ago
|
Attachment #8825358 -
Flags: approval-mozilla-beta?
Attachment #8825358 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 36•7 years ago
|
||
Re-attached the landed code to uplift.
Attachment #8825358 -
Attachment is obsolete: true
Assignee | ||
Comment 37•7 years ago
|
||
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?
Comment 38•7 years ago
|
||
(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 39•7 years ago
|
||
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?
Comment 40•7 years ago
|
||
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 41•7 years ago
|
||
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+
Comment 42•7 years ago
|
||
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)
Assignee | ||
Comment 43•7 years ago
|
||
(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)
Assignee | ||
Comment 44•7 years ago
|
||
Carry r+ from Comment 30.
Attachment #8828718 -
Attachment is obsolete: true
Assignee | ||
Comment 45•7 years ago
|
||
(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.
Assignee | ||
Comment 46•7 years ago
|
||
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 47•7 years ago
|
||
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?
Comment 48•7 years ago
|
||
bugherder uplift |
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
(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)
Comment 51•7 years ago
|
||
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)
Assignee | ||
Comment 53•7 years ago
|
||
(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
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•