views: add Harmony dialog width support

ui/views/window/dialog_delegate.h

 // 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.
 
 #ifndef UI_VIEWS_WINDOW_DIALOG_DELEGATE_H_
 #define UI_VIEWS_WINDOW_DIALOG_DELEGATE_H_
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/strings/string16.h"
@@ skipping 13 matching lines @@
 // DialogDelegate
 //
 //  DialogDelegate is an interface implemented by objects that wish to show a
 //  dialog box Window. The window that is displayed uses this interface to
 //  determine how it should be displayed and notify the delegate object of
 //  certain events.
 //
 ///////////////////////////////////////////////////////////////////////////////
 class VIEWS_EXPORT DialogDelegate : public ui::DialogModel,
                                     public WidgetDelegate {
+  // These values are in the 32px units the Harmony spec uses.

[Evan Stade, 2016/10/03 19:04:19]

nit: I don't really think we need to mention "harmony" or "spec" in comments
that are meant to persist. (When "harmony" is default, it will be implicit.) I'd
just as soon leave this comment off completely.

[Elly Fong-Jones, 2016/10/05 13:20:31]

But even after we're done, I think people may want to figure out what these
values are / where they came from, and the spec will still be named Harmony
then, so it seems better to reference it by name here.

  public:
+  enum DialogWidth {

[Evan Stade, 2016/10/03 19:04:19]

protected?

[sky, 2016/10/03 21:24:02]

enum class and remove DIALOG_WIDTH from each name? Also, it seems unusual to
have these constants defined here, but used only by DialogDelegateView. Should
the constants move to DialogDelegateView?

[Evan Stade, 2016/10/04 00:13:46]

ahhhh, I forgot about this distinction. We do have some dialogs which are
DialogDelegates but *not* DialogDelegateViews, and we probably want them to also
be sized accordingly. This makes me think we need to move this logic to
DialogClientView.

DialogClientView sizes the dialog[1] but how it does so should change.
DialogDelegate probably should get a new function DialogWidth GetWidthFactor()
and then DCV can use that to size the contents. This will cover both
DialogDelegateView and any other DialogDelegate implementor.

[1]
https://cs.chromium.org/chromium/src/ui/views/window/dialog_client_view.cc?rc...

[Elly Fong-Jones, 2016/10/05 13:20:31]

I don't understand this suggestion - aren't many dialogs not DialogClientViews?
like the collected cookies dialog, for example.

I did enum class and removed the DIALOG_WIDTH prefixes, though.

[Evan Stade, 2016/10/05 19:52:28]

I assume you meant to delete this comment

+    DIALOG_WIDTH_UNSPECIFIED,
+    DIALOG_WIDTH_SMALL = 10,
+    DIALOG_WIDTH_MEDIUM = 14,
+    DIALOG_WIDTH_LARGE = 16,
+  };
+
   DialogDelegate();
   ~DialogDelegate() override;
 
   // Creates a widget at a default location.
   static Widget* CreateDialogWidget(WidgetDelegate* delegate,
                                     gfx::NativeWindow context,
                                     gfx::NativeView parent);
 
   // Returns the dialog widget InitParams for a given |context| or |parent|.
   // If |bounds| is not empty, used to initially place the dialog, otherwise
@@ skipping 65 matching lines @@
 
   // A helper for accessing the DialogClientView object contained by this
   // delegate's Window.
   const DialogClientView* GetDialogClientView() const;
   DialogClientView* GetDialogClientView();
 
  protected:
   // Overridden from WidgetDelegate:
   ui::AXRole GetAccessibleWindowRole() const override;
 
+  DialogWidth dialog_width() const { return dialog_width_; }

[Evan Stade, 2016/10/03 19:04:19]

this getter doesn't seem necessary

[Elly Fong-Jones, 2016/10/05 13:20:31]

It was only necessary because the member was private, not protected, and I
needed it in DialogDelegateView. It can go now :)

+  void set_dialog_width(DialogWidth width) { dialog_width_ = width; }
+
  private:
   // A flag indicating whether this dialog is able to use the custom frame
   // style for dialogs.
   bool supports_custom_frame_;
+
+  // The Harmony width to use for this dialog.

[Evan Stade, 2016/10/03 19:04:19]

s/Harmony //

+  DialogWidth dialog_width_ = DIALOG_WIDTH_UNSPECIFIED;
 };
 
 // A DialogDelegate implementation that is-a View. Used to override GetWidget()
 // to call View's GetWidget() for the common case where a DialogDelegate
 // implementation is-a View. Note that DialogDelegateView is not owned by
 // view's hierarchy and is expected to be deleted on DeleteDelegate call.
 class VIEWS_EXPORT DialogDelegateView : public DialogDelegate,
                                         public View {
  public:
   DialogDelegateView();
   ~DialogDelegateView() override;
 
   // Overridden from DialogDelegate:
   void DeleteDelegate() override;
   Widget* GetWidget() override;
   const Widget* GetWidget() const override;
   View* GetContentsView() override;
 
   // Overridden from View:
   void GetAccessibleState(ui::AXViewState* state) override;
   void ViewHierarchyChanged(
       const ViewHierarchyChangedDetails& details) override;
+  gfx::Size GetPreferredSize() const override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DialogDelegateView);
 };
 
 }  // namespace views
 
 #endif  // UI_VIEWS_WINDOW_DIALOG_DELEGATE_H_