add oauth2 zippies to views extension install dialog

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

Index: chrome/browser/ui/views/extensions/extension_install_dialog_view.cc
diff --git a/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc b/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc
index 15216d8b9ed83b665ce6a2d90602b4b40644747b..7a1018f73aac420a4327e934f5eafd2015c6e6b0 100644
--- a/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc
+++ b/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc
@@ -15,8 +15,13 @@
 #include "chrome/common/extensions/extension.h"
 #include "content/public/browser/page_navigator.h"
 #include "grit/generated_resources.h"
+#include "grit/theme_resources.h"
+#include "ui/base/animation/animation_delegate.h"
+#include "ui/base/animation/slide_animation.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/point3.h"
+#include "ui/gfx/transform.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
@@ -67,7 +72,13 @@ void AddResourceIcon(const gfx::ImageSkia* skia_image, void* data) {
   parent->AddChildView(image_view);
 }
 
-}  // namespace
+// Creates a string for displaying |message| to the user. If it has to look
+// like a entry in a bullet point list, one is added.
+string16 PrepareForDisplay(const string16& message, bool bullet_point) {
+  return bullet_point ? l10n_util::GetStringFUTF16(
+      IDS_EXTENSION_PERMISSION_LINE,
+      message) : message;
+}
 
 // Implements the extension installation dialog for TOOLKIT_VIEWS.
 class ExtensionInstallDialogView : public views::DialogDelegateView,
@@ -78,6 +89,10 @@ class ExtensionInstallDialogView : public views::DialogDelegateView,
                              const ExtensionInstallPrompt::Prompt& prompt);
   virtual ~ExtensionInstallDialogView();
 
+  // Changes the size of the containing widget to match the preferred size
+  // of this dialog.
+  void SizeToContents();
+
  private:
   // views::DialogDelegateView:
   virtual string16 GetDialogButtonLabel(ui::DialogButton button) const OVERRIDE;
@@ -108,6 +123,62 @@ class ExtensionInstallDialogView : public views::DialogDelegateView,
   DISALLOW_COPY_AND_ASSIGN(ExtensionInstallDialogView);
 };
 
+// A view to display a single IssueAdviceInfoEntry.
+class IssueAdviceView : public views::View,
+                        public ui::AnimationDelegate {
+ public:
+  IssueAdviceView(ExtensionInstallDialogView* owner,
+                  const IssueAdviceInfoEntry& issue_advice,
+                  int horizontal_space);
+  virtual ~IssueAdviceView() {}
+
+  // Implementation of views::View:
+  virtual bool OnMousePressed(const views::MouseEvent& event) OVERRIDE;
+  virtual void OnMouseReleased(const views::MouseEvent& event) OVERRIDE;
+  virtual void ChildPreferredSizeChanged(views::View* child) OVERRIDE;
+
+  // Implementation of ui::AnimationDelegate:
+  virtual void AnimationProgressed(const ui::Animation* animation) OVERRIDE;
+
+ private:
+  // A view which displays all the details of an IssueAdviceInfoEntry.
+  class DetailsView : public views::View {
+   public:
+    DetailsView();
+    virtual ~DetailsView() {}
+
+    // Implementation of views::View:
+    virtual gfx::Size GetPreferredSize() OVERRIDE;
+
+    // Animates this to be a height proportional to |state|.
+    void AnimateToState(double state);
+
+   private:
+    double state_;
+
+    DISALLOW_COPY_AND_ASSIGN(DetailsView);
+  };
+
+  // The dialog that owns |this|. It's also an ancestor in the View hierarchy.
+  ExtensionInstallDialogView* owner_;
+
+  // The data that backs |this|.
+  IssueAdviceInfoEntry issue_advice_;
+
+  // A view for showing |issue_advice_.details|.
+  DetailsView* details_view_;
+
+  // The '>' zippy control.
+  views::ImageView* arrow_view_;
+
+  views::GridLayout* layout_;
+  ui::SlideAnimation slide_animation_;
+
+  DISALLOW_COPY_AND_ASSIGN(IssueAdviceView);
+};
+
+}  // namespace
+
 ExtensionInstallDialogView::ExtensionInstallDialogView(
     content::PageNavigator* navigator,
     ExtensionInstallPrompt::Delegate* delegate,
@@ -137,7 +208,7 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
   // +--------------------+------+
   //
   // Regular install
-  //      w/ permissions                 no permissions
+  // w/ permissions XOR oauth issues    no permissions
   // +--------------------+------+  +--------------+------+
   // | heading            | icon |  | heading      | icon |
   // +--------------------|      |  +--------------+------+
@@ -147,6 +218,23 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
   // +--------------------|      |
   // | permission2        |      |
   // +--------------------+------+
+  //
+  // w/ permissions AND oauth issues
+  // +--------------------+------+
+  // | heading            | icon |
+  // +--------------------|      |
+  // | permissions_header |      |
+  // +--------------------|      |
+  // | permission1        |      |
+  // +--------------------|      |
+  // | permission2        |      |
+  // +--------------------+------+
+  // | oauth header              |
+  // +---------------------------+
+  // | oauth issue 1             |
+  // +---------------------------+
+  // | oauth issue 2             |
+  // +---------------------------+
 
   views::GridLayout* layout = views::GridLayout::CreatePanel(this);
   SetLayoutManager(layout);
@@ -162,7 +250,7 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
                         views::GridLayout::FILL,
                         0,  // no resizing
                         views::GridLayout::USE_PREF,
-                        0,  // no fixed with
+                        0,  // no fixed width
                         left_column_width);
   if (!is_bundle_install()) {
     column_set->AddPaddingColumn(0, views::kPanelHorizMargin);
@@ -203,6 +291,10 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
       // Also span the permission header and each of the permission rows (all
       // have a padding row above it).
       icon_row_span = 3 + prompt.GetPermissionCount() * 2;
+    } else if (prompt.GetOAuthIssueCount()) {
+      // Also span the permission header and each of the permission rows (all
+      // have a padding row above it).
+      icon_row_span = 3 + prompt.GetOAuthIssueCount() * 2;
     }
     layout->AddView(icon, 1, icon_row_span);
   }
@@ -246,8 +338,7 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
       layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
       layout->StartRow(0, column_set_id);
       views::Label* extension_label = new views::Label(
-          l10n_util::GetStringFUTF16(
-              IDS_EXTENSION_PERMISSION_LINE, extension_name));
+          PrepareForDisplay(extension_name, true));
       extension_label->SetMultiLine(true);
       extension_label->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
       extension_label->SizeToFit(left_column_width);
@@ -294,11 +385,50 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
       layout->AddView(permission_label);
     }
   }
+
+  if (prompt.GetOAuthIssueCount()) {
+    // Slide in under the permissions; stretch all the way to the right of the
+    // dialog.
+    int space_for_oauth = left_column_width;
+    if (prompt.GetPermissionCount()) {
+      space_for_oauth += kIconSize;
+      column_set = layout->AddColumnSet(++column_set_id);
+      column_set->AddColumn(views::GridLayout::FILL,
+                            views::GridLayout::FILL,
+                            1,
+                            views::GridLayout::USE_PREF,
+                            0,  // no fixed width
+                            space_for_oauth);
+    }
+
+    layout->StartRowWithPadding(0, column_set_id,
+                                0, views::kRelatedControlVerticalSpacing);
+    views::Label* oauth_header = new views::Label(prompt.GetOAuthHeading());
+    oauth_header->SetMultiLine(true);
+    oauth_header->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
+    oauth_header->SizeToFit(left_column_width);
+    layout->AddView(oauth_header);
+
+    layout->AddPaddingRow(0, views::kRelatedControlSmallVerticalSpacing);

[Peter Kasting, 2012/07/27 00:45:09]

This results in two "small vertical spacing" padding rows in a row; seems
incorrect

[Evan Stade, 2012/07/27 03:06:54]

I was doing this intentionally for reasons I won't attempt to explain, but I
found a problem with my approach and I've fixed this as well.

+    for (size_t i = 0; i < prompt.GetOAuthIssueCount(); ++i) {
+      layout->StartRowWithPadding(
+          0, column_set_id,
+          0, views::kRelatedControlSmallVerticalSpacing);
+
+      IssueAdviceView* issue_advice_view =
+          new IssueAdviceView(this, prompt.GetOAuthIssue(i), space_for_oauth);
+      layout->AddView(issue_advice_view);
+    }
+  }
 }
 
 ExtensionInstallDialogView::~ExtensionInstallDialogView() {
 }
 
+void ExtensionInstallDialogView::SizeToContents() {
+  GetWidget()->SetSize(GetWidget()->non_client_view()->GetPreferredSize());
+}
+
 string16 ExtensionInstallDialogView::GetDialogButtonLabel(
     ui::DialogButton button) const {
   switch (button) {
@@ -360,3 +490,128 @@ void ShowExtensionInstallDialogImpl(
       new ExtensionInstallDialogView(navigator, delegate, prompt),
       parent)->Show();
 }
+
+// IssueAdviceView::DetailsView ------------------------------------------------
+
+IssueAdviceView::DetailsView::DetailsView() : state_(0) {
+  SetLayoutManager(new views::BoxLayout(
+      views::BoxLayout::kHorizontal,
+      0, 0, views::kRelatedControlSmallVerticalSpacing));
+}
+
+gfx::Size IssueAdviceView::DetailsView::GetPreferredSize() {
+  gfx::Size size = views::View::GetPreferredSize();
+  return gfx::Size(size.width(), size.height() * state_);
+}
+
+void IssueAdviceView::DetailsView::AnimateToState(double state) {
+  state_ = state;
+  PreferredSizeChanged();
+  SchedulePaint();
+}
+
+// IssueAdviceView -------------------------------------------------------------
+
+IssueAdviceView::IssueAdviceView(ExtensionInstallDialogView* owner,
+                                 const IssueAdviceInfoEntry& issue_advice,
+                                 int horizontal_space)
+    : owner_(owner),
+      issue_advice_(issue_advice),
+      details_view_(issue_advice.details.empty() ? NULL : new DetailsView()),
+      arrow_view_(NULL),
+      layout_(new views::GridLayout(this)),
+      slide_animation_(this) {
+  // TODO(estade): replace this with a more appropriate image.
+  const gfx::ImageSkia& image = *ui::ResourceBundle::GetSharedInstance().
+      GetImageSkiaNamed(IDR_OMNIBOX_TTS);
+
+  SetLayoutManager(layout_);

[Peter Kasting, 2012/07/27 00:45:09]

Nit: By convention we'd normally do most of this in ViewHierarchyChanged() when
this gets added to a parent view.  This makes sure that all the views we create
get correct memory management (since they won't be deleted if there's no parent
to own them).

Technically it's not terribly important in your case since you create this view
and then immediately add it to a hierarchy, so only an exception could interrupt
the process (and in that case Chrome would terminate), but for future-proofing
and consistency, it might be nice to do that here as well.

[Evan Stade, 2012/07/27 03:06:54]

well as long as |this| owns them, and |this| gets deleted, we're ok, right?
Meaning the child views would only leak if |this| leaks.

I've gone ahead and made the change though.

[Peter Kasting, 2012/07/27 03:38:03]

Oh, sorry, I didn't realize that with a layout manager, AddView() basically
means |this| now owns the view.  My fault for being murky on layout managers.

You're right that this was safe as it was, and it was probably simpler.  You may
want to get an authoritative opinion from sky on what the best practice here is,
but I'm now kind of leaning toward doing things as you had them here. 
(Sorry...)

[tfarina, 2012/07/27 03:50:34]

View owns the LayoutManager, when you do SetLayoutManager(layout_manager);
|this| will own |layout_manager|. That is why we don't need to store any pointer
in the class, and just have it as a local variable here.

[Peter Kasting, 2012/07/27 03:53:55]

The issue was the ownership of the views inside the layout manager (which it
turns out are owned by |this| once added to the layout manager), not the
ownership of the layout manager itself.

But yes, we don't need to keep the layout manager as a member if we don't need
to access it outside the constructor.

+  int column_set_id = 0;
+  views::ColumnSet* column_set = layout_->AddColumnSet(column_set_id);
+  if (details_view_) {
+    column_set->AddColumn(views::GridLayout::LEADING,
+                          views::GridLayout::LEADING,
+                          0,
+                          views::GridLayout::FIXED,
+                          image.width(),
+                          0);
+    horizontal_space -= image.width();
+  }
+  column_set->AddColumn(views::GridLayout::LEADING,
+                        views::GridLayout::FILL,
+                        0,
+                        views::GridLayout::FIXED,
+                        horizontal_space,
+                        0);
+  layout_->StartRow(0, column_set_id);
+
+  if (details_view_) {
+    arrow_view_ = new views::ImageView();
+    arrow_view_->SetImage(image);
+    arrow_view_->SetVerticalAlignment(views::ImageView::CENTER);
+    layout_->AddView(arrow_view_);
+  }
+
+  views::Label* description_label =
+      new views::Label(PrepareForDisplay(issue_advice.description,
+                                         !details_view_));
+  description_label->SetMultiLine(true);
+  description_label->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
+  description_label->SizeToFit(horizontal_space);
+  layout_->AddView(description_label);
+
+  if (!details_view_)
+    return;
+
+  layout_->StartRowWithPadding(0, column_set_id,
+                               0, views::kRelatedControlSmallVerticalSpacing);
+  layout_->SkipColumns(1);
+  layout_->AddView(details_view_);
+
+  details_view_->SetLayoutManager(
+      new views::BoxLayout(views::BoxLayout::kVertical,
+                           0, 0, views::kRelatedControlSmallVerticalSpacing));
+
+  for (size_t i = 0; i < issue_advice.details.size(); ++i) {

[Peter Kasting, 2012/07/27 00:45:09]

Just making sure -- it looks like as the details view expands, the contents
inside will appear to be pinned at the top, and we'll reveal more and more at
the bottom until all the text is visible, without the text actually shifting. 
Is this right?  That's how things should work, I just want to make sure the text
doesn't shift vertically while the slide animation is running.

[Evan Stade, 2012/07/27 03:06:54]

yes, that happens. I can create a video of it if you want.

[Peter Kasting, 2012/07/27 03:38:03]

Don't worry, I believe you :)

+    views::Label* detail_label =
+        new views::Label(PrepareForDisplay(issue_advice.details[i], true));
+    detail_label->SetMultiLine(true);
+    detail_label->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
+    detail_label->SizeToFit(horizontal_space);
+    details_view_->AddChildView(detail_label);
+  }
+}
+
+bool IssueAdviceView::OnMousePressed(const views::MouseEvent& event) {
+  return details_view_ && event.IsLeftMouseButton();
+}
+
+void IssueAdviceView::OnMouseReleased(const views::MouseEvent& event) {
+  if (slide_animation_.IsShowing())

[Peter Kasting, 2012/07/27 00:45:09]

Nit: I'm not sure this is quite right -- seems like if the animation is fully
open, we'll click once to begin hiding, and then if we click again while the
animation is running it will continue to hide.  Seems like instead each click
should reverse the direction of any running animation.

[Evan Stade, 2012/07/27 03:06:54]

IsShowing() refers to the goal state actually, so this is correct (and no matter
how madly I click, it always ends up in the right state).

[Peter Kasting, 2012/07/27 03:38:03]

Awesome!

+    slide_animation_.Hide();
+  else
+    slide_animation_.Show();
+}
+
+void IssueAdviceView::AnimationProgressed(const ui::Animation* animation) {
+  DCHECK_EQ(animation, &slide_animation_);
+
+  if (details_view_)
+    details_view_->AnimateToState(animation->GetCurrentValue());
+
+  if (arrow_view_) {
+    ui::Transform rotate;
+    if (animation->GetCurrentValue() != 0.0) {
+      rotate.SetTranslate(-arrow_view_->width() / 2.0,
+                          -arrow_view_->height() / 2.0);
+      rotate.ConcatRotate(animation->GetCurrentValue() * 90);
+      rotate.ConcatTranslate(arrow_view_->width() / 2.0,
+                             arrow_view_->height() / 2.0);
+    }
+    arrow_view_->SetTransform(rotate);
+  }
+}
+
+void IssueAdviceView::ChildPreferredSizeChanged(views::View* child) {
+  owner_->SizeToContents();
+}