views: add Harmony dialog width support

ui/views/window/dialog_delegate.h

Index: ui/views/window/dialog_delegate.h
diff --git a/ui/views/window/dialog_delegate.h b/ui/views/window/dialog_delegate.h
index c74f5461ab9f131376a6261deb563a448be75c49..b17d1845ce83faa475f8dc959925f7b769887c7c 100644
--- a/ui/views/window/dialog_delegate.h
+++ b/ui/views/window/dialog_delegate.h
@@ -31,7 +31,15 @@ class LabelButton;
 ///////////////////////////////////////////////////////////////////////////////
 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;
 
@@ -117,10 +125,16 @@ class VIEWS_EXPORT DialogDelegate : public ui::DialogModel,
   // 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()
@@ -143,6 +157,7 @@ class VIEWS_EXPORT DialogDelegateView : public DialogDelegate,
   void GetAccessibleState(ui::AXViewState* state) override;
   void ViewHierarchyChanged(
       const ViewHierarchyChangedDetails& details) override;
+  gfx::Size GetPreferredSize() const override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DialogDelegateView);