|
|
Chromium Code Reviews
DescriptionUpdate WindowTreeClientImpl OnWindowDestroying
Update capture handling, forcing the queue to revert to null capture is incorrect. The in flight queue is listening for window destruction, and updates the revert target appropriately.
Capture does not need to be explicitly released here. mus::Window removes focus upon destruction. The in flight queue listens for this to update revert target appropriately.
TEST=views_mus_unittests, mus_public_unittests
BUG=611983
Committed: https://crrev.com/14f17517b43e58c9017bc911f36b60f4d6de44fd
Cr-Commit-Position: refs/heads/master@{#397602}
Patch Set 1 : #
Total comments: 2
Patch Set 2 : Rebase #Messages
Total messages: 23 (10 generated)
Patchset #1 (id:1) has been deleted
jonross@chromium.org changed reviewers: + sadrul@chromium.org
Hey, Could you take a look at this change? An update to the OnWindowDestroying that you needed to fix a test
https://codereview.chromium.org/2021383002/diff/20001/components/mus/public/c... File components/mus/public/cpp/lib/window_tree_client_impl.cc (left): https://codereview.chromium.org/2021383002/diff/20001/components/mus/public/c... components/mus/public/cpp/lib/window_tree_client_impl.cc:441: // Normally just updating the queued changes is sufficient. However since Would it be possible to write a test for this change?
https://codereview.chromium.org/2021383002/diff/20001/components/mus/public/c... File components/mus/public/cpp/lib/window_tree_client_impl.cc (left): https://codereview.chromium.org/2021383002/diff/20001/components/mus/public/c... components/mus/public/cpp/lib/window_tree_client_impl.cc:441: // Normally just updating the queued changes is sufficient. However since On 2016/05/31 18:29:03, sadrul wrote: > Would it be possible to write a test for this change? Pre-existing tests have coverage: MenuRunnerTest.WidgetDoesntTakeCapture WidgetTest.MousePressCausesCapture WidgetTest.CaptureDuringMousePressNotOverridden WindowTreeClientImplTest.LostCaptureDifferentInFlightChange
lgtm
The CQ bit was checked by jonross@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2021383002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2021383002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-device-gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...) ios-simulator on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
jonross@chromium.org changed reviewers: + sky@chromium.org
Hey sky@ Could you provide an owners review for this change to window_tree_client?
LGTM
The CQ bit was checked by jonross@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sadrul@chromium.org Link to the patchset: https://codereview.chromium.org/2021383002/#ps40001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2021383002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2021383002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by jonross@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2021383002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2021383002/40001
Message was sent while issue was closed.
Committed patchset #2 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Update WindowTreeClientImpl OnWindowDestroying Update capture handling, forcing the queue to revert to null capture is incorrect. The in flight queue is listening for window destruction, and updates the revert target appropriately. Capture does not need to be explicitly released here. mus::Window removes focus upon destruction. The in flight queue listens for this to update revert target appropriately. TEST=views_mus_unittests, mus_public_unittests BUG=611983 ========== to ========== Update WindowTreeClientImpl OnWindowDestroying Update capture handling, forcing the queue to revert to null capture is incorrect. The in flight queue is listening for window destruction, and updates the revert target appropriately. Capture does not need to be explicitly released here. mus::Window removes focus upon destruction. The in flight queue listens for this to update revert target appropriately. TEST=views_mus_unittests, mus_public_unittests BUG=611983 Committed: https://crrev.com/14f17517b43e58c9017bc911f36b60f4d6de44fd Cr-Commit-Position: refs/heads/master@{#397602} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/14f17517b43e58c9017bc911f36b60f4d6de44fd Cr-Commit-Position: refs/heads/master@{#397602} |
