Skip to content

Commit

Permalink
Merge to M71: Follow up to fix 1 px gap of app list
Browse files Browse the repository at this point in the history
The original change fixed the bounds for app list window, but not for
its parent, so the issue is not fixed entirely. Fix it for the parent
window in this CL.

Bug: 884889
Change-Id: I6a48bae85fd3d89957d58a4aabf270c7df288d5d
Reviewed-on: https://chromium-review.googlesource.com/c/1282386
Commit-Queue: Weidong Guo <weidongg@chromium.org>
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/master@{#600054}(cherry picked from commit 3cabcd8)
Reviewed-on: https://chromium-review.googlesource.com/c/1287046
Reviewed-by: Weidong Guo <weidongg@chromium.org>
Cr-Original-Commit-Position: refs/branch-heads/3575@{#6}
Cr-Original-Branched-From: 16ca0c3-refs/heads/master@{#597882}(cherry picked from commit efe4bb9b77874ce4d94c53da7d40d5a06fd5691c)
Reviewed-on: https://chromium-review.googlesource.com/c/1294872
Cr-Commit-Position: refs/branch-heads/3578@{#247}
Cr-Branched-From: 4226ddf-refs/heads/master@{#599034}
  • Loading branch information
Weidong Guo committed Oct 22, 2018
1 parent 01c64ad commit 907c56a
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 18 deletions.
30 changes: 28 additions & 2 deletions ash/app_list/app_list_presenter_delegate_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "base/command_line.h"
#include "chromeos/chromeos_switches.h"
#include "ui/aura/window.h"
#include "ui/display/manager/display_manager.h"
#include "ui/events/event.h"
#include "ui/keyboard/keyboard_controller.h"
#include "ui/views/widget/widget.h"
Expand Down Expand Up @@ -54,7 +55,9 @@ bool IsSideShelf(aura::Window* root_window) {

AppListPresenterDelegateImpl::AppListPresenterDelegateImpl(
AppListControllerImpl* controller)
: controller_(controller) {}
: controller_(controller), display_observer_(this) {
display_observer_.Add(display::Screen::GetScreen());
}

AppListPresenterDelegateImpl::~AppListPresenterDelegateImpl() {
Shell::Get()->RemovePreTargetHandler(this);
Expand All @@ -77,11 +80,19 @@ void AppListPresenterDelegateImpl::Init(app_list::AppListView* view,
aura::Window* root_window = Shell::GetRootWindowForDisplayId(display_id);

app_list::AppListView::InitParams params;
params.parent =
aura::Window* parent_window =
RootWindowController::ForWindow(root_window)
->GetContainer(IsHomeLauncherEnabledInTabletMode()
? kShellWindowId_AppListTabletModeContainer
: kShellWindowId_AppListContainer);

// Snap the window bounds to fit the screen size (See
// https://crbug.com/884889).
const gfx::Rect bounds = ash::screen_util::SnapBoundsToDisplayEdge(
parent_window->GetBoundsInScreen(), parent_window);
parent_window->SetBoundsInScreen(
bounds, Shell::Get()->display_manager()->GetDisplayForId(display_id));
params.parent = parent_window;
params.initial_apps_page = current_apps_page;
params.is_tablet_mode = Shell::Get()
->tablet_mode_controller()
Expand Down Expand Up @@ -171,6 +182,21 @@ void AppListPresenterDelegateImpl::OnTargetVisibilityChanged(bool visible) {
controller_->OnTargetVisibilityChanged(visible);
}

void AppListPresenterDelegateImpl::OnDisplayMetricsChanged(
const display::Display& display,
uint32_t changed_metrics) {
if (!presenter_->GetWindow())
return;

// Snap the window bounds to fit the screen size (See
// https://crbug.com/884889).
aura::Window* parent_window = presenter_->GetWindow()->parent();
const gfx::Rect bounds = ash::screen_util::SnapBoundsToDisplayEdge(
parent_window->GetBoundsInScreen(), parent_window);
parent_window->SetBoundsInScreen(bounds, display);
view_->OnParentWindowBoundsChanged();
}

////////////////////////////////////////////////////////////////////////////////
// AppListPresenterDelegateImpl, private:

Expand Down
16 changes: 15 additions & 1 deletion ash/app_list/app_list_presenter_delegate_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#include "ash/app_list/presenter/app_list_presenter_delegate.h"
#include "ash/ash_export.h"
#include "base/macros.h"
#include "base/scoped_observer.h"
#include "ui/display/display_observer.h"
#include "ui/events/event_handler.h"
#include "ui/keyboard/keyboard_controller_observer.h"

Expand All @@ -19,6 +21,10 @@ class AppListView;
class AppListViewDelegate;
} // namespace app_list

namespace display {
class Screen;
} // namespace display

namespace ui {
class LocatedEvent;
} // namespace ui
Expand All @@ -33,7 +39,8 @@ class AppListControllerImpl;
// update its layout as necessary.
class ASH_EXPORT AppListPresenterDelegateImpl
: public app_list::AppListPresenterDelegate,
public ui::EventHandler {
public ui::EventHandler,
public display::DisplayObserver {
public:
explicit AppListPresenterDelegateImpl(AppListControllerImpl* controller);
~AppListPresenterDelegateImpl() override;
Expand All @@ -56,6 +63,10 @@ class ASH_EXPORT AppListPresenterDelegateImpl
void OnVisibilityChanged(bool visible, aura::Window* root_window) override;
void OnTargetVisibilityChanged(bool visible) override;

// DisplayObserver overrides:
void OnDisplayMetricsChanged(const display::Display& display,
uint32_t changed_metrics) override;

private:
void ProcessLocatedEvent(ui::LocatedEvent* event);

Expand All @@ -75,6 +86,9 @@ class ASH_EXPORT AppListPresenterDelegateImpl
// Not owned, owns this class.
AppListControllerImpl* const controller_ = nullptr;

// An observer that notifies AppListView when the display has changed.
ScopedObserver<display::Screen, display::DisplayObserver> display_observer_;

DISALLOW_COPY_AND_ASSIGN(AppListPresenterDelegateImpl);
};

Expand Down
5 changes: 1 addition & 4 deletions ash/app_list/views/app_list_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,6 @@ AppListView::AppListView(AppListViewDelegate* delegate)
model_(delegate->GetModel()),
search_model_(delegate->GetSearchModel()),
is_background_blur_enabled_(app_list_features::IsBackgroundBlurEnabled()),
display_observer_(this),
hide_view_animation_observer_(
std::make_unique<HideViewAnimationObserver>()),
transition_animation_observer_(
Expand All @@ -321,7 +320,6 @@ AppListView::AppListView(AppListViewDelegate* delegate)
weak_ptr_factory_(this) {
CHECK(delegate);

display_observer_.Add(display::Screen::GetScreen());
// Enable arrow key in FocusManager. Arrow left/right and up/down triggers
// the same focus movement as tab/shift+tab.
views::FocusManager::set_arrow_key_traversal_enabled(true);
Expand Down Expand Up @@ -1602,8 +1600,7 @@ bool AppListView::CloseKeyboardIfVisible() {
return false;
}

void AppListView::OnDisplayMetricsChanged(const display::Display& display,
uint32_t changed_metrics) {
void AppListView::OnParentWindowBoundsChanged() {
// Set the |fullscreen_widget_| size to fit the new display metrics.
fullscreen_widget_->GetNativeView()->SetBounds(
GetPreferredWidgetBoundsForState(app_list_state_));
Expand Down
16 changes: 5 additions & 11 deletions ash/app_list/views/app_list_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
#include "ash/public/cpp/app_list/app_list_constants.h"
#include "base/callback.h"
#include "base/macros.h"
#include "base/scoped_observer.h"
#include "build/build_config.h"
#include "ui/display/display_observer.h"
#include "ui/views/widget/widget.h"
#include "ui/views/widget/widget_delegate.h"

Expand All @@ -25,7 +23,7 @@ class Window;
}

namespace display {
class Screen;
class Display;
}

namespace ui {
Expand All @@ -49,8 +47,7 @@ class TransitionAnimationObserver;
// and hosts a AppsGridView and passes AppListModel to it for display.
// TODO(newcomer|weidongg): Organize the cc file to match the order of
// definitions in this header.
class APP_LIST_EXPORT AppListView : public views::WidgetDelegateView,
public display::DisplayObserver {
class APP_LIST_EXPORT AppListView : public views::WidgetDelegateView {
public:
class TestApi {
public:
Expand Down Expand Up @@ -187,6 +184,9 @@ class APP_LIST_EXPORT AppListView : public views::WidgetDelegateView,
// Called when on-screen keyboard's visibility is changed.
void OnScreenKeyboardShown(bool shown);

// Called when parent window's bounds is changed.
void OnParentWindowBoundsChanged();

// If the on-screen keyboard is shown, hide it. Return whether keyboard was
// hidden
bool CloseKeyboardIfVisible();
Expand Down Expand Up @@ -321,10 +321,6 @@ class APP_LIST_EXPORT AppListView : public views::WidgetDelegateView,
// Overridden from views::WidgetDelegateView:
views::View* GetInitiallyFocusedView() override;

// Overridden from DisplayObserver:
void OnDisplayMetricsChanged(const display::Display& display,
uint32_t changed_metrics) override;

// Gets app list background opacity during dragging.
float GetAppListBackgroundOpacityDuringDragging();

Expand Down Expand Up @@ -392,8 +388,6 @@ class APP_LIST_EXPORT AppListView : public views::WidgetDelegateView,
const bool is_background_blur_enabled_;
// The state of the app list, controlled via SetState().
AppListViewState app_list_state_ = AppListViewState::PEEKING;
// An observer that notifies AppListView when the display has changed.
ScopedObserver<display::Screen, display::DisplayObserver> display_observer_;

// A widget observer that sets the AppListView state when the widget is
// closed.
Expand Down

0 comments on commit 907c56a

Please sign in to comment.