Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2410613003:
Moving logic for System UI state changes into DesktopCanvas class (Closed)
DescriptionMoving logic for System UI state changes into DesktopCanvas class
Most of the logic for reacting to System UI changes was already in the
DesktopCanvaqs class however there was one boolean check outside of
that. Instead of adding yet another parameter to the DesktopCanvas
handler method, I now pass the event parameters themselves. This will
allow future changes to the logic to be made solely in the DesktopCanvas
class.
BUG=654565
Committed: https://crrev.com/77a6c664de52f6a28910ab585a347ed0f4c2d899
Cr-Commit-Position: refs/heads/master@{#424284}
Patch Set 1 #Patch Set 2 : Fixing some System UI size checks #
Total comments: 2
Patch Set 3 : Addressing CR feedback #
Dependent Patchsets: Messages
Total messages: 24 (18 generated)
The CQ bit was checked by joedow@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Moving logic for System UI state changes into DesktopCanvas class Most of the logic for reacting to System UI changes was already in the DesktopCanvaqs class however there was one boolean check outside of that. Instead of adding yet another parameter to the DesktopCanvas handler method, I now pass the event parameters themselves. This will allow future changes to the logic to be made solely in the DesktopCanvas class. BUG= ========== to ========== Moving logic for System UI state changes into DesktopCanvas class Most of the logic for reacting to System UI changes was already in the DesktopCanvaqs class however there was one boolean check outside of that. Instead of adding yet another parameter to the DesktopCanvas handler method, I now pass the event parameters themselves. This will allow future changes to the logic to be made solely in the DesktopCanvas class. BUG=654565 ==========
The CQ bit was checked by joedow@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by joedow@chromium.org to run a CQ dry run
Patchset #2 (id:20001) has been deleted
Dry run: 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
Dry run: This issue passed the CQ dry run.
joedow@chromium.org changed reviewers: + lambroslambrou@chromium.org, yuweih@chromium.org
PTAL! Note that there should not be any behavioral changes in this one, this CL just moves the boolean check into the DesktopCanvas class. Thanks, Joe
lgtm https://codereview.chromium.org/2410613003/diff/40001/remoting/android/java/s... File remoting/android/java/src/org/chromium/chromoting/TouchInputHandler.java (right): https://codereview.chromium.org/2410613003/diff/40001/remoting/android/java/s... remoting/android/java/src/org/chromium/chromoting/TouchInputHandler.java:353: mDesktopCanvas.onSystemUiVisibilityChanged(parameter); Maybe get rid of this private method and set mDesktopCanvas directly in attachEvent(desktop.onSystemUiVisibilityChanged(), ...) ?
https://codereview.chromium.org/2410613003/diff/40001/remoting/android/java/s... File remoting/android/java/src/org/chromium/chromoting/TouchInputHandler.java (right): https://codereview.chromium.org/2410613003/diff/40001/remoting/android/java/s... remoting/android/java/src/org/chromium/chromoting/TouchInputHandler.java:353: mDesktopCanvas.onSystemUiVisibilityChanged(parameter); On 2016/10/10 23:06:30, Yuwei wrote: > Maybe get rid of this private method and set mDesktopCanvas directly in > attachEvent(desktop.onSystemUiVisibilityChanged(), ...) ? Done
The CQ bit was checked by joedow@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from yuweih@chromium.org Link to the patchset: https://codereview.chromium.org/2410613003/#ps60001 (title: "Addressing CR feedback")
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.
Committed patchset #3 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Moving logic for System UI state changes into DesktopCanvas class Most of the logic for reacting to System UI changes was already in the DesktopCanvaqs class however there was one boolean check outside of that. Instead of adding yet another parameter to the DesktopCanvas handler method, I now pass the event parameters themselves. This will allow future changes to the logic to be made solely in the DesktopCanvas class. BUG=654565 ========== to ========== Moving logic for System UI state changes into DesktopCanvas class Most of the logic for reacting to System UI changes was already in the DesktopCanvaqs class however there was one boolean check outside of that. Instead of adding yet another parameter to the DesktopCanvas handler method, I now pass the event parameters themselves. This will allow future changes to the logic to be made solely in the DesktopCanvas class. BUG=654565 Committed: https://crrev.com/77a6c664de52f6a28910ab585a347ed0f4c2d899 Cr-Commit-Position: refs/heads/master@{#424284} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/77a6c664de52f6a28910ab585a347ed0f4c2d899 Cr-Commit-Position: refs/heads/master@{#424284} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
