Unify TickImpl called during Device shutdown.

Bug: dawn:400
Change-Id: I26ad1a739d123ebc81a2e7d309c8a42f517be06c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/22662
Commit-Queue: Natasha Lee <natlee@microsoft.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/Device.cpp

@@ -165,6 +165,9 @@ namespace dawn_native {
         mDynamicUploader = nullptr;
         mMapRequestTracker = nullptr;
 
+        // call TickImpl once last time to clean up resources
+        // assert the errors are device loss so we can continue with destruction
+        AssertAndIgnoreDeviceLossError(TickImpl());
         AssumeCommandsComplete();
         // Tell the backend that it can free all the objects now that the GPU timeline is empty.
         ShutDownImpl();
@@ -186,6 +189,7 @@ namespace dawn_native {
             // Assert that errors are device losses so that we can continue with destruction.
             // Assume all commands are complete after WaitForIdleForDestruction (because they were)
             AssertAndIgnoreDeviceLossError(WaitForIdleForDestruction());
+            AssertAndIgnoreDeviceLossError(TickImpl());
             AssumeCommandsComplete();
             ASSERT(mFutureCallbackSerial <= mCompletedSerial);
             mState = State::Disconnected;

src/dawn_native/d3d12/DeviceD3D12.cpp

@@ -459,8 +459,6 @@ namespace dawn_native { namespace d3d12 {
         // Wait for all in-flight commands to finish executing
         DAWN_TRY(WaitForSerial(GetLastSubmittedCommandSerial()));
 
-        // Call tick one last time so resources are cleaned up.
-        DAWN_TRY(TickImpl());
         return {};
     }
 
@@ -512,13 +510,15 @@ namespace dawn_native { namespace d3d12 {
     void Device::ShutDownImpl() {
         ASSERT(GetState() == State::Disconnected);
 
-        // Immediately forget about all pending commands
+        // Immediately forget about all pending commands for the case where device is lost on its
+        // own and WaitForIdleForDestruction isn't called.
         mPendingCommands.Release();
 
         if (mFenceEvent != nullptr) {
             ::CloseHandle(mFenceEvent);
         }
 
+        // We need to handle clearing up com object refs that were enqeued after TickImpl
         mUsedComObjectRefs.ClearUpTo(GetCompletedCommandSerial());
 
         ASSERT(mUsedComObjectRefs.Empty());

src/dawn_native/metal/DeviceMTL.mm

@@ -292,8 +292,6 @@ namespace dawn_native { namespace metal {
             CheckPassedSerials();
         }
 
-        DAWN_TRY(TickImpl());
-
         return {};
     }
 

src/dawn_native/null/DeviceNull.cpp

@@ -185,6 +185,7 @@ namespace dawn_native { namespace null {
     }
 
     MaybeError Device::WaitForIdleForDestruction() {
+        mPendingOperations.clear();
         return {};
     }
 

src/dawn_native/opengl/DeviceGL.cpp

@@ -200,7 +200,6 @@ namespace dawn_native { namespace opengl {
         gl.Finish();
         CheckPassedSerials();
         ASSERT(mFencesInFlight.empty());
-        Tick();
 
         return {};
     }

src/dawn_native/vulkan/DeviceVk.cpp

@@ -718,6 +718,16 @@ namespace dawn_native { namespace vulkan {
     }
 
     MaybeError Device::WaitForIdleForDestruction() {
+        // Immediately tag the recording context as unused so we don't try to submit it in Tick.
+        // Move the mRecordingContext.used to mUnusedCommands so it can be cleaned up in
+        // ShutDownImpl
+        if (mRecordingContext.used) {
+            CommandPoolAndBuffer commands = {mRecordingContext.commandPool,
+                                             mRecordingContext.commandBuffer};
+            mUnusedCommands.push_back(commands);
+            mRecordingContext = CommandRecordingContext();
+        }
+
         VkResult waitIdleResult = VkResult::WrapUnsafe(fn.QueueWaitIdle(mQueue));
         // Ignore the result of QueueWaitIdle: it can return OOM which we can't really do anything
         // about, Device lost, which means workloads running on the GPU are no longer accessible
@@ -782,9 +792,6 @@ namespace dawn_native { namespace vulkan {
         }
         mRecordingContext.signalSemaphores.clear();
 
-        // Assert that errors are device loss so that we can continue with destruction
-        AssertAndIgnoreDeviceLossError(TickImpl());
-
         ASSERT(mCommandsInFlight.Empty());
         for (const CommandPoolAndBuffer& commands : mUnusedCommands) {
             fn.DestroyCommandPool(mVkDevice, commands.pool, nullptr);