Cleanup BookmarkBubbleView, remove LocationBarBubbleDelegateView::GetDialogButtons()

chrome/browser/ui/views/location_bar/zoom_bubble_view.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/location_bar/zoom_bubble_view.h"
 
 #include <cmath>
 
 #include "base/auto_reset.h"
 #include "base/i18n/number_formatting.h"
@@ skipping 221 matching lines @@
     immersive_mode_controller_->AddObserver(this);
   UseCompactMargins();
   chrome::RecordDialogCreation(chrome::DialogIdentifier::ZOOM);
 }
 
 ZoomBubbleView::~ZoomBubbleView() {
   if (immersive_mode_controller_)
     immersive_mode_controller_->RemoveObserver(this);
 }
 
+int ZoomBubbleView::GetDialogButtons() const {
+  // TODO(tapted): The zoom bubble should expose its cancel (reset) button to
+  // DialogDelegate, but it does not want DialogClientView to create it.

[Peter Kasting, 2017/05/26 16:29:18]

I'm not really sure what this TODO is saying we should do.  The Zoom bubble is
basically a toast.  The reset button is right in the middle of a row of other
controls.  It seems reasonable to return DIALOG_BUTTON_NONE here.

[tapted, 2017/05/27 00:18:49]

Hm. This would be a consistency thing - exposing reset to the ui::DialogModel
allows it to interact with framework code. E.g. automatically mapping
accelerators or a11y roles, automated testing codepaths (e.g.
DialowFrameView->CancelWindow() flows), putting buttons on the TouchBar. But
'Reset' may be special in other ways that makes this tricky. I've removed the
comment and I'll investigate more.

[Peter Kasting, 2017/05/27 00:28:45]

If we want this button to work like a cancel button, then it does seem like
maybe a good idea to do this.

Another argument is that this button is really a "set to 100%" button rather
than "reset"/"cancel", so it's closer to + and - ("set to some other value"),
and the answer to the cases above is "we want all the buttons to be handled the
same way regarding these issues" -- which would lead to the question of e.g.
"can other buttons easily be placed on the TouchBar".

Anyway, I trust you to figure it out :)

[tapted, 2017/05/27 01:09:39]

Yeah.. I'll likely want to play with DialogModel some more - cancel/ok buttons
map neatly to DialogClientView::Cancel/AcceptWindow, but there isn't a neat
route to activate other buttons yet (short of walking the view hierarchy looking
for buttons.. which is probably not robust)

+  return ui::DIALOG_BUTTON_NONE;
+}
+
 void ZoomBubbleView::OnGestureEvent(ui::GestureEvent* event) {
   if (!zoom_bubble_ || !zoom_bubble_->auto_close_ ||
       event->type() != ui::ET_GESTURE_TAP) {
     return;
   }
 
   auto_close_ = false;
   StopTimer();
   event->SetHandled();
 }
@@ skipping 186 matching lines @@
                &ZoomBubbleView::CloseBubble);
 }
 
 void ZoomBubbleView::StopTimer() {
   timer_.Stop();
 }
 
 ZoomBubbleView::ZoomBubbleExtensionInfo::ZoomBubbleExtensionInfo() {}
 
 ZoomBubbleView::ZoomBubbleExtensionInfo::~ZoomBubbleExtensionInfo() {}