Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2250963006:
Register the window in MaximizeModeWindowManager when the window is shown. (Closed)
DescriptionRegister the window in MaximizeModeWindowManager when the window is shown.
This is necessary for arc app to set the "don't manage me" flag.
BUG=635899
Committed: https://crrev.com/5ef3cfef16fad285e84d8655e592d522401b6ff8
Cr-Commit-Position: refs/heads/master@{#412882}
Patch Set 1 #Patch Set 2 #
Total comments: 4
Patch Set 3 : . #Patch Set 4 : cleanup added_windos_ in dtor #
Messages
Total messages: 24 (16 generated)
The CQ bit was checked by oshima@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
oshima@chromium.org changed reviewers: + skuhne@google.com
Description was changed from ========== Register the window in MaximizeModeWindowManager when the window is shown. BUG=635899 ========== to ========== Register the window in MaximizeModeWindowManager when the window is shown. This is necessary for arc app to set the "don't manage me" flag. BUG=635899 ==========
skuhne@chromium.org changed reviewers: + skuhne@chromium.org
Please see comments, then lgtm https://codereview.chromium.org/2250963006/diff/20001/ash/common/wm/maximize_... File ash/common/wm/maximize_mode/maximize_mode_window_manager.cc (right): https://codereview.chromium.org/2250963006/diff/20001/ash/common/wm/maximize_... ash/common/wm/maximize_mode/maximize_mode_window_manager.cc:121: if (!params.target->IsVisible()) { This is confusing. You mention "wait until the window becomes visible" but then if (!...isVisible()). Maybe you should write: If the window is invisible we do .. since the client may update ..? https://codereview.chromium.org/2250963006/diff/20001/ash/common/wm/maximize_... ash/common/wm/maximize_mode/maximize_mode_window_manager.cc:173: } Could you put line 167-173 into a function and call that from here and from line 128?
https://codereview.chromium.org/2250963006/diff/20001/ash/common/wm/maximize_... File ash/common/wm/maximize_mode/maximize_mode_window_manager.cc (right): https://codereview.chromium.org/2250963006/diff/20001/ash/common/wm/maximize_... ash/common/wm/maximize_mode/maximize_mode_window_manager.cc:121: if (!params.target->IsVisible()) { On 2016/08/18 01:45:45, Mr4D wrote: > This is confusing. You mention "wait until the window becomes visible" but then > if (!...isVisible()). > > Maybe you should write: > If the window is invisible we do .. since the client may update ..? Updated to: // Don't register the window if the window is invisible. Instead, // wait until it becomes visible because the client may update the // flag to control if the window should be added. https://codereview.chromium.org/2250963006/diff/20001/ash/common/wm/maximize_... ash/common/wm/maximize_mode/maximize_mode_window_manager.cc:173: } On 2016/08/18 01:45:45, Mr4D wrote: > Could you put line 167-173 into a function and call that from here and from line > 128? I believe we can move the added to workspace to MaximizeAndTrackWindow, but let me do it in a separate CL because this has to be merged to m53.
The CQ bit was checked by oshima@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from skuhne@chromium.org Link to the patchset: https://codereview.chromium.org/2250963006/#ps40001 (title: ".")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by oshima@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from skuhne@chromium.org Link to the patchset: https://codereview.chromium.org/2250963006/#ps60001 (title: "cleanup added_windows_ in dtor")
The CQ bit was unchecked by oshima@chromium.org
Patchset #4 (id:60001) has been deleted
The CQ bit was checked by oshima@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from skuhne@chromium.org Link to the patchset: https://codereview.chromium.org/2250963006/#ps80001 (title: "cleanup added_windos_ in dtor")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Register the window in MaximizeModeWindowManager when the window is shown. This is necessary for arc app to set the "don't manage me" flag. BUG=635899 ========== to ========== Register the window in MaximizeModeWindowManager when the window is shown. This is necessary for arc app to set the "don't manage me" flag. BUG=635899 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Register the window in MaximizeModeWindowManager when the window is shown. This is necessary for arc app to set the "don't manage me" flag. BUG=635899 ========== to ========== Register the window in MaximizeModeWindowManager when the window is shown. This is necessary for arc app to set the "don't manage me" flag. BUG=635899 Committed: https://crrev.com/5ef3cfef16fad285e84d8655e592d522401b6ff8 Cr-Commit-Position: refs/heads/master@{#412882} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/5ef3cfef16fad285e84d8655e592d522401b6ff8 Cr-Commit-Position: refs/heads/master@{#412882} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
