Fix the Web Bluetooth chooser when it is used on Chrome apps on non-Mac

chrome/browser/ui/views/extensions/chooser_dialog_view.cc

 // Copyright 2016 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/extensions/chooser_dialog_view.h"
 
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chooser_controller/chooser_controller.h"
 #include "chrome/browser/extensions/chrome_extension_chooser_dialog.h"
 #include "chrome/browser/ui/views/chooser_content_view.h"
 #include "components/constrained_window/constrained_window_views.h"
 #include "components/web_modal/web_contents_modal_dialog_manager.h"
 #include "content/public/browser/browser_thread.h"
+#include "ui/gfx/color_palette.h"
+#include "ui/views/bubble/bubble_border.h"
+#include "ui/views/bubble/bubble_frame_view.h"
 #include "ui/views/controls/link.h"
 #include "ui/views/controls/styled_label.h"
+#include "ui/views/layout/layout_constants.h"
 #include "ui/views/window/dialog_client_view.h"
 
 ChooserDialogView::ChooserDialogView(
     std::unique_ptr<ChooserController> chooser_controller) {
   // ------------------------------------
   // | Chooser dialog title             |
   // | -------------------------------- |
   // | | option 0                     | |
   // | | option 1                     | |
   // | | option 2                     | |
@@ skipping 31 matching lines @@
 }
 
 bool ChooserDialogView::IsDialogButtonEnabled(ui::DialogButton button) const {
   return chooser_content_view_->IsDialogButtonEnabled(button);
 }
 
 views::View* ChooserDialogView::CreateFootnoteView() {
   return chooser_content_view_->CreateFootnoteView();
 }
 
+views::ClientView* ChooserDialogView::CreateClientView(views::Widget* widget) {
+  views::DialogClientView* client =
+      new views::DialogClientView(widget, GetContentsView());
+  client->set_button_row_insets(gfx::Insets());

[sky, 2016/11/04 21:35:35]

It would be nice not to have explicitly set this. What happens if you don't? Is
there another way that it can be automatically detected?

[juncai, 2016/11/05 00:34:23]

The DialogDelegate::CreateClientView() doesn't call
client->set_button_row_insets(gfx::Insets()):
https://cs.chromium.org/chromium/src/ui/views/window/dialog_delegate.cc?dr=C&...
And it causes the right side of the "Cancel" button not vertically aligned with
the right side of the table view.

The chooser on the Chrome browser works fine and I found the
BubbleDialogDelegateView::CreateClientView() calls
client->set_button_row_insets(gfx::Insets()):
https://cs.chromium.org/chromium/src/ui/views/bubble/bubble_dialog_delegate.c...

That's why I added the call to client->set_button_row_insets(gfx::Insets())
here.

+  return client;
+}
+
+views::NonClientFrameView* ChooserDialogView::CreateNonClientFrameView(
+    views::Widget* widget) {
+  views::BubbleFrameView* frame = new views::BubbleFrameView(

[sky, 2016/11/04 21:35:35]

Isn't this the same as DialogDelegate::CreateDialogFrameView but with
content_margins? If I'm write, can you make
DialogDelegate::CreateDialogFrameView() take an insets and call it?

Also, this style is not always used, do you not care about that? By that I mean
this code in DialogDelegate is only used if ShouldUseCustomFrame() returns true,
which isn't always the case.

[juncai, 2016/11/05 00:34:23]

Yes, it is the same as DialogDelegate::CreateDialogFrameView except the
content_margins.

Done.

+      gfx::Insets(views::kPanelVertMargin, views::kButtonHEdgeMarginNew, 0,
+                  views::kButtonHEdgeMarginNew),
+      gfx::Insets(views::kPanelVertMargin, views::kPanelHorizMargin,
+                  views::kPanelVertMargin, views::kPanelHorizMargin));
+  const views::BubbleBorder::Shadow kShadow = views::BubbleBorder::SMALL_SHADOW;
+  std::unique_ptr<views::BubbleBorder> border(new views::BubbleBorder(
+      views::BubbleBorder::FLOAT, kShadow, gfx::kPlaceholderColor));
+  border->set_use_theme_background_color(true);
+  frame->SetBubbleBorder(std::move(border));
+  views::DialogDelegate* delegate =
+      widget->widget_delegate()->AsDialogDelegate();
+  if (delegate)
+    frame->SetFootnoteView(delegate->CreateFootnoteView());
+  return frame;
+}
+
 bool ChooserDialogView::Accept() {
   chooser_content_view_->Accept();
   return true;
 }
 
 bool ChooserDialogView::Cancel() {
   chooser_content_view_->Cancel();
   return true;
 }
 
@@ skipping 27 matching lines @@
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(chooser_controller);
 
   web_modal::WebContentsModalDialogManager* manager =
       web_modal::WebContentsModalDialogManager::FromWebContents(web_contents_);
   if (manager) {
     constrained_window::ShowWebModalDialogViews(
         new ChooserDialogView(std::move(chooser_controller)), web_contents_);
   }
 }