Skip to content

Commit

Permalink
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-Commit-Position: refs/heads/master@{#600054}
  • Loading branch information
Weidong Guo authored and Commit Bot committed Oct 16, 2018
1 parent 5c4bf4a commit 3cabcd8
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 @@ -1607,8 +1605,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 3cabcd8

Please sign in to comment.