Commit 3e4e3b3e authored by Sarah Chan's avatar Sarah Chan Committed by Commit Bot

Revert "[MacViews] Wire up AutofillPopupView"

This reverts commit 01b39191.

Reason for revert: Causes crash

Original change's description:
> [MacViews] Wire up AutofillPopupView
> 
> Show toolkit-views AutofillPopupView with --secondary-ui-md.
> 
> Removed AutofillPopupBaseView FocusManager accelerator code.
> The code is no longer necessary since it was added for a now
> obsolete rAc dialog. The autofill popup will get dismissed by
> the web contents, which listens for the accelerators.
> 
> Bug: 728182
> Change-Id: I9f7816fe41279a397f5066d28d88099d50ef033e
> Reviewed-on: https://chromium-review.googlesource.com/889983
> Commit-Queue: Sarah Chan <spqchan@chromium.org>
> Reviewed-by: Trent Apted <tapted@chromium.org>
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#534527}

TBR=ellyjones@chromium.org,tapted@chromium.org,estade@chromium.org,spqchan@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 728182
Change-Id: Ibda0e0f4a15e54e2f2d4c19951719f7d604473ff
Reviewed-on: https://chromium-review.googlesource.com/919972Reviewed-by: default avatarSarah Chan <spqchan@chromium.org>
Reviewed-by: default avatarTrent Apted <tapted@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536935}
parent 62c55a88
......@@ -66,8 +66,8 @@ split_static_library("ui") {
"cocoa/autofill/autofill_dialog_constants.h",
"cocoa/autofill/autofill_popup_base_view_cocoa.h",
"cocoa/autofill/autofill_popup_base_view_cocoa.mm",
"cocoa/autofill/autofill_popup_view_bridge_views.h",
"cocoa/autofill/autofill_popup_view_bridge_views.mm",
"cocoa/autofill/autofill_popup_view_bridge.h",
"cocoa/autofill/autofill_popup_view_bridge.mm",
"cocoa/autofill/autofill_popup_view_cocoa.h",
"cocoa/autofill/autofill_popup_view_cocoa.mm",
"cocoa/autofill/autofill_tooltip_controller.h",
......@@ -2740,12 +2740,6 @@ split_static_library("ui") {
"views/apps/app_info_dialog/app_info_summary_panel.h",
"views/apps/chrome_native_app_window_views.cc",
"views/apps/chrome_native_app_window_views.h",
"views/autofill/autofill_popup_base_view.cc",
"views/autofill/autofill_popup_base_view.h",
"views/autofill/autofill_popup_view_native_views.cc",
"views/autofill/autofill_popup_view_native_views.h",
"views/autofill/autofill_popup_view_views.cc",
"views/autofill/autofill_popup_view_views.h",
"views/autofill/card_unmask_prompt_views.cc",
"views/autofill/card_unmask_prompt_views.h",
"views/autofill/save_card_bubble_views.cc",
......@@ -2985,6 +2979,12 @@ split_static_library("ui") {
"javascript_dialogs/javascript_dialog.cc",
"views/accessibility/invert_bubble_view.cc",
"views/accessibility/invert_bubble_view.h",
"views/autofill/autofill_popup_base_view.cc",
"views/autofill/autofill_popup_base_view.h",
"views/autofill/autofill_popup_view_native_views.cc",
"views/autofill/autofill_popup_view_native_views.h",
"views/autofill/autofill_popup_view_views.cc",
"views/autofill/autofill_popup_view_views.h",
"views/autofill/password_generation_popup_view_views.cc",
"views/autofill/password_generation_popup_view_views.h",
"views/autofill/save_card_icon_view.cc",
......
......@@ -4,15 +4,13 @@
#import <Cocoa/Cocoa.h>
#include "chrome/browser/ui/cocoa/autofill/autofill_popup_view_bridge_views.h"
#include "chrome/browser/ui/cocoa/autofill/autofill_popup_view_bridge.h"
#include "base/logging.h"
#include "chrome/browser/ui/autofill/autofill_popup_controller.h"
#include "chrome/browser/ui/autofill/autofill_popup_layout_model.h"
#include "chrome/browser/ui/autofill/autofill_popup_view_delegate.h"
#import "chrome/browser/ui/cocoa/autofill/autofill_popup_view_cocoa.h"
#include "chrome/browser/ui/cocoa/browser_dialogs_views_mac.h"
#include "chrome/browser/ui/views/autofill/autofill_popup_view_views.h"
#include "ui/gfx/geometry/rect.h"
namespace autofill {
......@@ -65,9 +63,6 @@ void AutofillPopupViewBridge::OnSuggestionsChanged() {
AutofillPopupView* AutofillPopupView::Create(
AutofillPopupController* controller) {
if (chrome::ShowAllDialogsWithViewsToolkit())
return AutofillPopupViewViews::Create(controller);
return new AutofillPopupViewBridge(controller);
}
......
......@@ -11,7 +11,7 @@
#include "chrome/app/vector_icons/vector_icons.h"
#include "chrome/browser/ui/autofill/autofill_popup_controller.h"
#include "chrome/browser/ui/autofill/autofill_popup_layout_model.h"
#include "chrome/browser/ui/cocoa/autofill/autofill_popup_view_bridge_views.h"
#include "chrome/browser/ui/cocoa/autofill/autofill_popup_view_bridge.h"
#import "chrome/browser/ui/cocoa/browser_window_controller.h"
#import "chrome/browser/ui/cocoa/tab_contents/tab_contents_controller.h"
#import "chrome/browser/ui/cocoa/tabs/tab_strip_controller.h"
......
......@@ -9,10 +9,6 @@
#include "base/single_thread_task_runner.h"
#include "base/threading/thread_task_runner_handle.h"
#include "build/build_config.h"
#include "chrome/browser/platform_util.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_window.h"
#include "ui/base/ui_features.h"
#include "ui/native_theme/native_theme.h"
#include "ui/views/border.h"
#include "ui/views/controls/scroll_view.h"
......@@ -52,20 +48,24 @@ AutofillPopupBaseView::~AutofillPopupBaseView() {
void AutofillPopupBaseView::DoShow() {
const bool initialize_widget = !GetWidget();
if (initialize_widget) {
#if !defined(OS_MACOSX) || BUILDFLAG(MAC_VIEWS_BROWSER)
// Without this, the popup will not dismiss if the window is moved.
// TODO(spqchan): Find a way to dismiss the popup when the window is moved.
// See https://crbug.com/808270.
parent_widget_->AddObserver(this);
#endif
views::FocusManager* focus_manager = parent_widget_->GetFocusManager();
focus_manager->RegisterAccelerator(
ui::Accelerator(ui::VKEY_RETURN, ui::EF_NONE),
ui::AcceleratorManager::kNormalPriority,
this);
focus_manager->RegisterAccelerator(
ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE),
ui::AcceleratorManager::kNormalPriority,
this);
// The widget is destroyed by the corresponding NativeWidget, so we use
// a weak pointer to hold the reference and don't have to worry about
// deletion.
views::Widget* widget = new views::Widget;
views::Widget::InitParams params(views::Widget::InitParams::TYPE_POPUP);
params.delegate = this;
params.parent = parent_widget_ ? parent_widget_->GetNativeView()
: delegate_->container_view();
params.parent = parent_widget_->GetNativeView();
widget->Init(params);
scroll_view_ = new views::ScrollView;
......@@ -118,9 +118,8 @@ void AutofillPopupBaseView::OnWidgetBoundsChanged(views::Widget* widget,
}
void AutofillPopupBaseView::RemoveObserver() {
#if !defined(OS_MACOSX) || BUILDFLAG(MAC_VIEWS_BROWSER)
parent_widget_->GetFocusManager()->UnregisterAccelerators(this);
parent_widget_->RemoveObserver(this);
#endif
views::WidgetFocusManager::GetInstance()->RemoveFocusChangeListener(this);
}
......@@ -137,23 +136,10 @@ void AutofillPopupBaseView::DoUpdateBoundsAndRedrawPopup() {
// Compute the space available for the popup. It's the space between its top
// and the bottom of its parent view, minus some margin space.
gfx::Rect clipping_bounds;
#if !defined(OS_MACOSX) || BUILDFLAG(MAC_VIEWS_BROWSER)
clipping_bounds = parent_widget_->GetClientAreaBoundsInScreen();
#else
gfx::NativeWindow window =
platform_util::GetTopLevel(delegate_->container_view());
Browser* browser = chrome::FindBrowserWithWindow(window);
DCHECK(browser);
// This is not the same as "GetClientAreaBoundsInScreen()", but it gives us
// the lower bounds the popup will have to clip to on the screen.
clipping_bounds = browser->window()->GetBounds();
#endif
int available_vertical_space = clipping_bounds.height() -
(bounds.y() - clipping_bounds.y()) -
kPopupBottomMargin;
int available_vertical_space =
parent_widget_->GetClientAreaBoundsInScreen().height() -
(bounds.y() - parent_widget_->GetClientAreaBoundsInScreen().y()) -
kPopupBottomMargin;
if (available_vertical_space < bounds.height()) {
// The available space is not enough for the full popup so clamp the widget
......@@ -258,6 +244,22 @@ void AutofillPopupBaseView::OnGestureEvent(ui::GestureEvent* event) {
event->SetHandled();
}
bool AutofillPopupBaseView::AcceleratorPressed(
const ui::Accelerator& accelerator) {
DCHECK_EQ(accelerator.modifiers(), ui::EF_NONE);
if (accelerator.key_code() == ui::VKEY_ESCAPE) {
HideController();
return true;
}
if (accelerator.key_code() == ui::VKEY_RETURN)
return delegate_->AcceptSelectedLine();
NOTREACHED();
return false;
}
void AutofillPopupBaseView::SetSelection(const gfx::Point& point) {
if (delegate_)
delegate_->SetSelectionAtPoint(point);
......
......@@ -54,6 +54,7 @@ class AutofillPopupBaseView : public views::WidgetDelegateView,
bool OnMousePressed(const ui::MouseEvent& event) override;
void OnMouseReleased(const ui::MouseEvent& event) override;
void OnGestureEvent(ui::GestureEvent* event) override;
bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
// views::WidgetFocusChangeListener implementation.
void OnNativeFocusChanged(gfx::NativeView focused_now) override;
......
......@@ -6,7 +6,6 @@
#include "base/feature_list.h"
#include "base/optional.h"
#include "build/build_config.h"
#include "chrome/browser/ui/autofill/autofill_popup_controller.h"
#include "chrome/browser/ui/autofill/autofill_popup_layout_model.h"
#include "chrome/browser/ui/views/autofill/autofill_popup_view_native_views.h"
......@@ -16,7 +15,6 @@
#include "components/autofill/core/browser/suggestion.h"
#include "ui/accessibility/ax_node_data.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/ui_features.h"
#include "ui/events/keycodes/keyboard_codes.h"
#include "ui/gfx/canvas.h"
#include "ui/gfx/geometry/point.h"
......@@ -258,18 +256,16 @@ void AutofillPopupViewViews::CreateChildViews() {
}
}
AutofillPopupView* AutofillPopupViewViews::Create(
AutofillPopupView* AutofillPopupView::Create(
AutofillPopupController* controller) {
views::Widget* observing_widget =
views::Widget::GetTopLevelWidgetForNativeView(
controller->container_view());
#if !defined(OS_MACOSX) || BUILDFLAG(MAC_VIEWS_BROWSER)
// If the top level widget can't be found, cancel the popup since we can't
// fully set it up.
if (!observing_widget)
return NULL;
#endif
if (base::FeatureList::IsEnabled(autofill::kAutofillExpandedPopupViews))
return new AutofillPopupViewNativeViews(controller, observing_widget);
......@@ -277,11 +273,4 @@ AutofillPopupView* AutofillPopupViewViews::Create(
return new AutofillPopupViewViews(controller, observing_widget);
}
#if !defined(OS_MACOSX) || BUILDFLAG(MAC_VIEWS_BROWSER)
AutofillPopupView* AutofillPopupView::Create(
AutofillPopupController* controller) {
return AutofillPopupViewViews::Create(controller);
}
#endif
} // namespace autofill
......@@ -25,8 +25,6 @@ class AutofillPopupViewViews : public AutofillPopupBaseView,
views::Widget* parent_widget);
~AutofillPopupViewViews() override;
static AutofillPopupView* Create(AutofillPopupController* controller);
private:
FRIEND_TEST_ALL_PREFIXES(AutofillPopupViewViewsTest, OnSelectedRowChanged);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment