New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 786290 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jan 2018
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Migrate usages of KeyboardControllerObserver::OnKeyboardBoundsChanging

Project Member Reported by blakeo@chromium.org, Nov 17 2017

Issue description

I want to remove this method and change its various implementers to use a more specific observer method, which I added in https://chromium-review.googlesource.com/c/chromium/src/+/768310

With the Full-Width keyboard, there were various assumptions people could make when the keyboard covered a certain region of the screen. However, with the addition of a floating keyboard, these assumptions will sometimes be broken. Sometimes the observer wants to know where the literal bounds of the keyboard are for visual purposes. Other times, they are checking availability of the keyboard (by seeing if the bounds are empty). Other times, they want to know if there's a certain region of the screen that is inaccessible. These all now depend on the type of keyboard that is currently loaded (except for the availability one which is sort of a hack, so a separate API should be added that is cleaner).
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d98f5865dddd710df0ff4ec4840607d10d5193dc

commit d98f5865dddd710df0ff4ec4840607d10d5193dc
Author: Blake O'Hare <blakeo@chromium.org>
Date: Fri Nov 17 10:43:53 2017

Define a default body to OnKeyboardBoundsChanging

WHY?

Because of http://crrev/c/768310

I'll be migrating consumers of the OnKeyboardBoundsChanging method to
use a more specific method and don't want empty definitions of this
method for those consumers.

Bug:  786290 
Change-Id: I5eb12104de100c18b96dfd350d252ca721a57b9f
Reviewed-on: https://chromium-review.googlesource.com/776254
Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517361}
[modify] https://crrev.com/d98f5865dddd710df0ff4ec4840607d10d5193dc/ui/keyboard/keyboard_controller_observer.h

Comment 2 by blakeo@chromium.org, Nov 21 2017

List of affected observers that I found via a quick search on codesearch:

- TouchExplorationManager
- LockLayoutManager
- ArcImeService
- ChromeKeyboardUI
- ShelfLayoutManager
- AppListPresenterDelegate
- PanelLayoutManager
- SystemModalContainerLayoutManager
- WebUiLoginView
- WorkspaceLayoutManager
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/505db23457e3c0efb7fc08aba8320301e819e01e

commit 505db23457e3c0efb7fc08aba8320301e819e01e
Author: Blake O'Hare <blakeo@chromium.org>
Date: Wed Nov 22 05:53:38 2017

Replace usage of OnKeyboardBoundsChanging with a more specific method

OnKeyboardBoundsChanging is being split into several more specific
methods that have a stronger contract of how they are being used by the
implementer. More information about why this is being done is in the
associated bug.

OnKeyboardVisibleBoundsChanging is called when the keyboard bounds
changes (exactly like before) but is only used for purely visual
purposes when the observer wants to know where the keyboard is visually
on the screen, and, for example, makes no implication that the area
covered cannot be used by the workspace layout.

Bug:  786290 
Change-Id: Ie204988c128620d692d62590c13b403f99c78859
Reviewed-on: https://chromium-review.googlesource.com/784651
Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518548}
[modify] https://crrev.com/505db23457e3c0efb7fc08aba8320301e819e01e/chrome/browser/ui/ash/chrome_keyboard_ui.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a7588a30d59fea194ad7a1afa5695405dcc41e80

commit a7588a30d59fea194ad7a1afa5695405dcc41e80
Author: Blake O'Hare <blakeo@chromium.org>
Date: Fri Nov 24 02:15:19 2017

Rename usage of OnKeyboardBoundsChanging with a more specific method

OnKeyboardBoundsChanging is being split into several more specific
methods that have a stronger contract of how they are being used by the
implementer. More information about why this is being done is in the
associated bug.

OnKeyboardVisibleBoundsChanging is called when the keyboard bounds
changes (exactly like before) but is only used for purely visual
purposes when the observer wants to know where the keyboard is visually
on the screen, and, for example, makes no implication that the area
covered cannot be used by the workspace layout.

Bug:  786290 
Change-Id: I2f80b4a66f65d39655a0fd8790d48c07662d22de
Reviewed-on: https://chromium-review.googlesource.com/778701
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Reviewed-by: OOO(back on 11/27)Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519045}
[modify] https://crrev.com/a7588a30d59fea194ad7a1afa5695405dcc41e80/ash/ash_touch_exploration_manager_chromeos.cc
[modify] https://crrev.com/a7588a30d59fea194ad7a1afa5695405dcc41e80/ash/ash_touch_exploration_manager_chromeos.h

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e2fcf00ae2d04e1659c63eb262a03f527a8115a6

commit e2fcf00ae2d04e1659c63eb262a03f527a8115a6
Author: Blake O'Hare <blakeo@chromium.org>
Date: Mon Nov 27 06:07:01 2017

Split out keyboard controller observer methods even further

As far as keyboard controller observer methods that report bounds,
currently there is OnKeyboardWorkspaceOccludedBoundsChanging which
reports what part of the workspace is covered and
OnKeyboardVisibleBoundsChanging which reports the bounds for purely
visual purposes.

However, there is a difference between workspace bounds that are simply
occluded and incovenient to use and the region that is completely
unusable due to that space being occupied by the keyboard. I am adding
OnKeyboardWorkspaceDisplacingBoundsChanging for this. The layout of the
window manager will change for the displacing bounds. If the bounds of
the workspace stay the same but it is occluded, then the UI may react
to that differently. For example, moving windows out of the way for UX
convenience.

Currently this works, however various consumers of the keyboard
controller observer API's are also querying the locked status, and this
is very brittle and will produce unexpected results when the floating
keyboard is added.

Bug:  786290 
Change-Id: Ib61cde1c6558e3a001c51523bef141ca49b14861
Reviewed-on: https://chromium-review.googlesource.com/790111
Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519226}
[modify] https://crrev.com/e2fcf00ae2d04e1659c63eb262a03f527a8115a6/ui/keyboard/container_behavior.h
[modify] https://crrev.com/e2fcf00ae2d04e1659c63eb262a03f527a8115a6/ui/keyboard/container_floating_behavior.cc
[modify] https://crrev.com/e2fcf00ae2d04e1659c63eb262a03f527a8115a6/ui/keyboard/container_floating_behavior.h
[modify] https://crrev.com/e2fcf00ae2d04e1659c63eb262a03f527a8115a6/ui/keyboard/container_full_width_behavior.cc
[modify] https://crrev.com/e2fcf00ae2d04e1659c63eb262a03f527a8115a6/ui/keyboard/container_full_width_behavior.h
[modify] https://crrev.com/e2fcf00ae2d04e1659c63eb262a03f527a8115a6/ui/keyboard/keyboard_controller.cc
[modify] https://crrev.com/e2fcf00ae2d04e1659c63eb262a03f527a8115a6/ui/keyboard/keyboard_controller_observer.h

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0467289fe14105270ad7004c62c6fe796fbf5d2f

commit 0467289fe14105270ad7004c62c6fe796fbf5d2f
Author: Blake O'Hare <blakeo@chromium.org>
Date: Mon Nov 27 07:33:59 2017

Update KeyboardController/SystemModalContainerLayoutManager interaction

Two key changes:

1) Change OnKeyboardBoundsChanging to
OnKeyboardWorkspaceOccludedBoundsChanging. This will only get
notifications when the bounds change in a way that affect the available
workspace area.

2) Change current_keyboard_bounds() to GetWorkspaceObscuringBounds()
This will only return the bounds of the keyboard if it is blocking the
available workspace area.

The full-width anchored keyboard prevents the bottom portion of the
screen from being used when visible, but the floating keyboard does not
have this restriction. The associated bugs are an effort to
disambiguate the various usages of the keyboard bounds so that the
floating keyboard will not cause unintended side effects to the
workspace.

Bug:  787713 ,  786290 
Change-Id: I31f5250d937d57c30437222c2c2165ac43cd215b
Reviewed-on: https://chromium-review.googlesource.com/784756
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519234}
[modify] https://crrev.com/0467289fe14105270ad7004c62c6fe796fbf5d2f/ash/wm/system_modal_container_layout_manager.cc
[modify] https://crrev.com/0467289fe14105270ad7004c62c6fe796fbf5d2f/ash/wm/system_modal_container_layout_manager.h

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/58326bfa9b9c81b994e3bb97d89d9808c44be676

commit 58326bfa9b9c81b994e3bb97d89d9808c44be676
Author: Blake O'Hare <blakeo@chromium.org>
Date: Mon Nov 27 08:35:28 2017

Replace LockLayoutManager's usage of OnKeyboardBoundsChanging

OnKeyboardBoundsChanging is being split into several more specific
methods that have a stronger contract of how they are being used by the
implementer. More information about why this is being done is in the
associated bug.

OnKeyboardWorkspaceOccludedBoundChanging is called when the keyboard's
bounds change in a way that makes certain regions of the workspace
unusable, as far as workspace layout is concerned. When the keyboard's
bounds change in a way that is purely visual or the keyboard is in a
mode where it shouldn't affect workspace layout (such as a floating
keyboard or accessibility keyboard), this method will not be called.

Bug:  786290 
Change-Id: Ia463129590204665bb461f13c3cde70388e10d1d
Reviewed-on: https://chromium-review.googlesource.com/778580
Reviewed-by: OOO(back on 11/27)Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519246}
[modify] https://crrev.com/58326bfa9b9c81b994e3bb97d89d9808c44be676/ash/wm/lock_layout_manager.cc
[modify] https://crrev.com/58326bfa9b9c81b994e3bb97d89d9808c44be676/ash/wm/lock_layout_manager.h

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f53e38f39880031626a488f4c8dee4846a074660

commit f53e38f39880031626a488f4c8dee4846a074660
Author: Blake O'Hare <blakeo@chromium.org>
Date: Wed Nov 29 03:33:40 2017

Replace usage of OnKeyboardBoundsChanging with a more specific method

OnKeyboardBoundsChanging is being split into several more specific
methods that have a stronger contract of how they are being used by the
implementer. More information about why this is being done is in the
associated bug.

Since these new bounds change notification methods are only going to be
called in specific circumstances depending on how they're being used, a
dedicated OnKeyboardAvailabilityChanging method has been added for
implementers that were simply checking availability of the keyboard
with bounds.IsEmpty() which will always fire at the expected times.

I've also removed OnKeyboardClosed(), which has a default body now.

Bug:  786290 
Change-Id: I2bf230f81144445707238db76cd9050a0d4224a6
Reviewed-on: https://chromium-review.googlesource.com/784690
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519989}
[modify] https://crrev.com/f53e38f39880031626a488f4c8dee4846a074660/chrome/browser/chromeos/login/ui/webui_login_view.cc
[modify] https://crrev.com/f53e38f39880031626a488f4c8dee4846a074660/chrome/browser/chromeos/login/ui/webui_login_view.h

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a9249a3ab02174ce71383e54fedb216ed578ba51

commit a9249a3ab02174ce71383e54fedb216ed578ba51
Author: Blake O'Hare <blakeo@chromium.org>
Date: Thu Nov 30 02:13:10 2017

AppListPresenterDelegate should use keyboard obscuring bounds notifier

Changing the AppListPresenterDelegates implementation of the
KeyboardControllerObserver to use the obscuring bounds (bounds of the
workspace that are obscured by the keyboard) rather than the real
bounds.

More information on why is in the attached bug, but essentially, there
will be more nuance to how these bounds notifications are sent out once
the floating keyboard is added (which will behave as though it does
not cover anything on the screen). Since the AppListPresenterDelegate
essentially just uses this notification as a tickle which calls
UpdateBounds(), which ultimately calls into the getter to
ObscuringBounds, then only the OnObscuringBoundsChanging notification
is needed.

Bug:  786290 
Change-Id: I4e9f0b8478c3866c3326ce324611ed19911e2a0c
Reviewed-on: https://chromium-review.googlesource.com/795273
Reviewed-by: Jenny Zhang <jennyz@chromium.org>
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520391}
[modify] https://crrev.com/a9249a3ab02174ce71383e54fedb216ed578ba51/ash/app_list/app_list_presenter_delegate.cc
[modify] https://crrev.com/a9249a3ab02174ce71383e54fedb216ed578ba51/ash/app_list/app_list_presenter_delegate.h

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8ad071a794874d6aeba85eba5c3c91e059b16ffe

commit 8ad071a794874d6aeba85eba5c3c91e059b16ffe
Author: Blake O'Hare <blakeo@chromium.org>
Date: Fri Dec 01 09:04:07 2017

Consolidate notifications sent to KeyboardControllerObservers

If multiple consecutive notifications are sent with identical values,
there is no need to send the notification.

For example, the visual bounds of the keyboard may change, but if it is
in floating keyboard mode, it's possible that neither the availability
nor the occluded bounds have changed. In these cases, simply don't send
a notification to observers.

The reason this is important is that a couple of the observers
assume that notifications will generally alternate between non-empty
and empty bounds and assume that an empty notification is a state
transition. One example of this is restoring window positions to their
saved locations upon the keyboard disappearing as signaled by an empty
bounds notification.

Bug:  786290 
Change-Id: Ibee2f25f34679b0c6516ba141be6667d0636fc95
Reviewed-on: https://chromium-review.googlesource.com/802842
Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520890}
[modify] https://crrev.com/8ad071a794874d6aeba85eba5c3c91e059b16ffe/ui/keyboard/BUILD.gn
[modify] https://crrev.com/8ad071a794874d6aeba85eba5c3c91e059b16ffe/ui/keyboard/keyboard_controller.cc
[modify] https://crrev.com/8ad071a794874d6aeba85eba5c3c91e059b16ffe/ui/keyboard/keyboard_controller.h
[modify] https://crrev.com/8ad071a794874d6aeba85eba5c3c91e059b16ffe/ui/keyboard/keyboard_controller_unittest.cc
[add] https://crrev.com/8ad071a794874d6aeba85eba5c3c91e059b16ffe/ui/keyboard/notification_manager.cc
[add] https://crrev.com/8ad071a794874d6aeba85eba5c3c91e059b16ffe/ui/keyboard/notification_manager.h
[add] https://crrev.com/8ad071a794874d6aeba85eba5c3c91e059b16ffe/ui/keyboard/notification_manager_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Dec 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b550fcc14228b5fcdc0b5303991779dd2b37d690

commit b550fcc14228b5fcdc0b5303991779dd2b37d690
Author: Blake O'Hare <blakeo@chromium.org>
Date: Tue Dec 05 01:36:52 2017

Update PanelLayoutManager to use Keyboard occluded bounds

OnKeyboardBoundsChanging is being split into several more specific
methods that have a stronger contract of how they are being used by the
implementer. More information about why this is being done is in the
associated bug.

OnKeyboardWorkspaceOccludedBoundChanging is called when the keyboard's
bounds change in a way that makes certain regions of the workspace
unusable, as far as workspace layout is concerned. When the keyboard's
bounds change in a way that is purely visual or the keyboard is in a
mode where it shouldn't affect workspace layout (such as a floating
keyboard), this method will not be called.

Bug:  786290 
Change-Id: I4237be051b1a4bb3cf6eeff5598079205b4e1292
Reviewed-on: https://chromium-review.googlesource.com/804626
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521580}
[modify] https://crrev.com/b550fcc14228b5fcdc0b5303991779dd2b37d690/ash/wm/panels/panel_layout_manager.cc
[modify] https://crrev.com/b550fcc14228b5fcdc0b5303991779dd2b37d690/ash/wm/panels/panel_layout_manager.h

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4f8542d8cd6cb93bf665d36fa3a2102609631055

commit 4f8542d8cd6cb93bf665d36fa3a2102609631055
Author: Blake O'Hare <blakeo@chromium.org>
Date: Thu Dec 07 07:28:11 2017

Add another KeyboardControllerObserver method that aggregates state

This observer method is redundant with the other methods. However it
seems that some of the implementations are querying multiple aspects of
the keyboard and performing an action based on that. For example, the
Arc IME needs to know both changes in availability and the occluded
bounds at the same time. Using the methods currently provided is
unviable because it would require tracking state between the two calls
and then invoking the system call to ARC++ once both calls have
completed. This is cumbersome.

In addition to this, I'd also like to eliminate direct calls to
KeyboardController::GetInstance()->keyboard_locked() and so I am
putting this property into the struct of this notification as well.

There are only a couple of usages where this pattern is necessary and
so most consumers should simply use the more specific existing methods.

Bug:  786290 
Change-Id: I3e2db92273f0a144c87caef29b860cb6b7f83995
Reviewed-on: https://chromium-review.googlesource.com/813147
Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522375}
[modify] https://crrev.com/4f8542d8cd6cb93bf665d36fa3a2102609631055/ui/keyboard/keyboard_controller.cc
[modify] https://crrev.com/4f8542d8cd6cb93bf665d36fa3a2102609631055/ui/keyboard/keyboard_controller_observer.h
[modify] https://crrev.com/4f8542d8cd6cb93bf665d36fa3a2102609631055/ui/keyboard/notification_manager.cc
[modify] https://crrev.com/4f8542d8cd6cb93bf665d36fa3a2102609631055/ui/keyboard/notification_manager.h

Project Member

Comment 13 by bugdroid1@chromium.org, Dec 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3ef446508b5a059e28d450007307c5cffe483fd1

commit 3ef446508b5a059e28d450007307c5cffe483fd1
Author: Blake O'Hare <blakeo@chromium.org>
Date: Mon Dec 11 06:42:43 2017

Replace ArcImeService's usage of OnKeyboardBoundsChanging

OnKeyboardBoundsChanging is being split into several more specific
methods that have a stronger contract of how they are being used by the
implementer. More information about why this is being done is in the
associated bug.

OnKeyboardAppearanceChanging is called when any time the keyboard's
bounds or availability changes and has a struct as its parameter that
includes several properties of the keyboard appearance.

Currently the IME Bridge does not support any additional params other
than the bounds and then uses the bounds to check if the keyboard is
available by checking for empty bounds. I've opened b/70251261 to
change this, since empty bounds can still mean the keyboard is
available. The KeyboardStateDescriptor struct contains a boolean that
indicates if the keyboard is truly available for use. This value needs
to be plumbed into the ime bridge.

Bug:  786290 
Change-Id: I256c348418ec792879e2687064b1a16468432908
Reviewed-on: https://chromium-review.googlesource.com/778740
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523048}
[modify] https://crrev.com/3ef446508b5a059e28d450007307c5cffe483fd1/components/arc/ime/arc_ime_service.cc
[modify] https://crrev.com/3ef446508b5a059e28d450007307c5cffe483fd1/components/arc/ime/arc_ime_service.h

Project Member

Comment 14 by bugdroid1@chromium.org, Dec 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f91a759b1f61b963edb7ad69fb6df144c38557ad

commit f91a759b1f61b963edb7ad69fb6df144c38557ad
Author: Blake O'Hare <blakeo@chromium.org>
Date: Thu Dec 14 03:42:16 2017

Update ShelfLayoutManager KeyboardControllerObserver methods

The floating keyboard should not cause the shelf to disappear. Although
the regular full-width keyboard should.

This CL does 3 things:
- Uses the keyboard's occluding bounds rather than visual bounds. This
  describes the area that is not accessible due to the keyboard
  covering it. This will be empty bounds in the case of a floating
  keyboard.
- Uses keyboard availability event rather than checking if bounds are
  empty to determine if the keyboard is available.
- Uses the is_locked attribute on the state descriptor object rather
  than checking the GetInstance()'s value directly. There are too many
  places where code is querying the static instance directly for the
  keyboard's current lock state and so we'd like to...lock those down.

Bug:  786290 
Change-Id: I8aba795b7228a14d00a8aaf218d4f56fb1006091
Reviewed-on: https://chromium-review.googlesource.com/816483
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523998}
[modify] https://crrev.com/f91a759b1f61b963edb7ad69fb6df144c38557ad/ash/shelf/shelf.cc
[modify] https://crrev.com/f91a759b1f61b963edb7ad69fb6df144c38557ad/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/f91a759b1f61b963edb7ad69fb6df144c38557ad/ash/shelf/shelf_layout_manager.h
[modify] https://crrev.com/f91a759b1f61b963edb7ad69fb6df144c38557ad/ash/shelf/shelf_layout_manager_unittest.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Dec 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f5a8f4faf818b4df1b5e104b29920dd6c36679c

commit 9f5a8f4faf818b4df1b5e104b29920dd6c36679c
Author: Blake O'Hare <blakeo@chromium.org>
Date: Mon Dec 18 01:59:47 2017

Update WorkspaceLayoutManager to only listen to locked keyboard changes

OnKeyboardWorkspaceDisplacingBoundsChanging gets fired only if the
bounds change in a way that affect workspace layout (i.e. when the
locked full width keyboard does something). This simplifies the logic
here so that the check to end early if the keyboard is not locked can
be removed entirely.

Bug:  786290 
Change-Id: I9710049271bcbf0e5f662f0d4959cbc817ca569a
Reviewed-on: https://chromium-review.googlesource.com/828480
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524637}
[modify] https://crrev.com/9f5a8f4faf818b4df1b5e104b29920dd6c36679c/ash/wm/workspace/workspace_layout_manager.cc
[modify] https://crrev.com/9f5a8f4faf818b4df1b5e104b29920dd6c36679c/ash/wm/workspace/workspace_layout_manager.h
[modify] https://crrev.com/9f5a8f4faf818b4df1b5e104b29920dd6c36679c/ash/wm/workspace/workspace_layout_manager_unittest.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Jan 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/16af148639b2477f50430e01368698a5ab40e38c

commit 16af148639b2477f50430e01368698a5ab40e38c
Author: Blake O'Hare <blakeo@chromium.org>
Date: Tue Jan 09 02:43:08 2018

Remove the OnKeyboardBoundsChanging definition

This also includes removing one usage in a unit test in ash/wm/ and
concludes my KeyboardControllerObserver refactoring effort described in
the bug.

Bug:  786290 
Change-Id: I2a043dfc799a1c6e6ee502d80e9b9b075fe56ff1
Reviewed-on: https://chromium-review.googlesource.com/842342
Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527875}
[modify] https://crrev.com/16af148639b2477f50430e01368698a5ab40e38c/ash/wm/always_on_top_controller_unittest.cc
[modify] https://crrev.com/16af148639b2477f50430e01368698a5ab40e38c/ui/keyboard/keyboard_controller_observer.h
[modify] https://crrev.com/16af148639b2477f50430e01368698a5ab40e38c/ui/keyboard/keyboard_controller_unittest.cc
[modify] https://crrev.com/16af148639b2477f50430e01368698a5ab40e38c/ui/keyboard/notification_manager.cc

Status: Fixed (was: Assigned)
Yay

Sign in to add a comment