summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorpkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-06-22 20:02:05 +0000
committerpkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-06-22 20:02:05 +0000
commit1a53f54afb599e5cbadd865b8ae6d8806b439d3e (patch)
treed7c8fb23fff13c6b4e753df9447891f89780aed6
parentc5cfdbe75fe43c9b342d332636155db6b6b9381c (diff)
downloadchromium_src-1a53f54afb599e5cbadd865b8ae6d8806b439d3e.zip
chromium_src-1a53f54afb599e5cbadd865b8ae6d8806b439d3e.tar.gz
chromium_src-1a53f54afb599e5cbadd865b8ae6d8806b439d3e.tar.bz2
Various pieces of infobar cleanup:
* Simplify some code in the views implementation. Remove an unnecessary animation Stop() call. Move other code to PlatformSpecificShow(), now that it exists, to try and simplify ViewHierarchyChanged(), and so that it executes "when we actually show the infobar" (doesn't really matter, the two are called right after each other). * Convert InfoBar::animation_ from a scoped_ptr<> to an object; it lives for the life of the InfoBar so there's no real reason to use a pointer. Also convert the const getter to returning a const ref. * Update a comment to be a bit more accurate, especially now that the views code doesn't stop the animation in ViewHierarchyChanged(). * Use |i| for an iterator instead of the verbose |infobar_iterator| BUG=none TEST=none Review URL: http://codereview.chromium.org/7190024 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@90080 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/tab_contents/infobar.cc24
-rw-r--r--chrome/browser/tab_contents/infobar.h13
-rw-r--r--chrome/browser/tab_contents/infobar_container.cc21
-rw-r--r--chrome/browser/ui/gtk/infobars/infobar_gtk.cc2
-rw-r--r--chrome/browser/ui/views/infobars/infobar_view.cc79
-rw-r--r--chrome/browser/ui/views/infobars/infobar_view.h6
6 files changed, 64 insertions, 81 deletions
diff --git a/chrome/browser/tab_contents/infobar.cc b/chrome/browser/tab_contents/infobar.cc
index 89c49ae..e5f5ece 100644
--- a/chrome/browser/tab_contents/infobar.cc
+++ b/chrome/browser/tab_contents/infobar.cc
@@ -43,7 +43,7 @@ InfoBar::InfoBar(TabContentsWrapper* owner, InfoBarDelegate* delegate)
: owner_(owner),
delegate_(delegate),
container_(NULL),
- ALLOW_THIS_IN_INITIALIZER_LIST(animation_(new ui::SlideAnimation(this))),
+ ALLOW_THIS_IN_INITIALIZER_LIST(animation_(this)),
arrow_height_(0),
arrow_target_height_(kDefaultArrowTargetHeight),
arrow_half_width_(0),
@@ -51,7 +51,7 @@ InfoBar::InfoBar(TabContentsWrapper* owner, InfoBarDelegate* delegate)
bar_target_height_(kDefaultBarTargetHeight) {
DCHECK(owner != NULL);
DCHECK(delegate != NULL);
- animation_->SetTweenType(ui::Tween::LINEAR);
+ animation_.SetTweenType(ui::Tween::LINEAR);
}
InfoBar::~InfoBar() {
@@ -60,9 +60,9 @@ InfoBar::~InfoBar() {
void InfoBar::Show(bool animate) {
PlatformSpecificShow(animate);
if (animate) {
- animation_->Show();
+ animation_.Show();
} else {
- animation_->Reset(1.0);
+ animation_.Reset(1.0);
AnimationEnded(NULL);
}
}
@@ -70,9 +70,9 @@ void InfoBar::Show(bool animate) {
void InfoBar::Hide(bool animate) {
PlatformSpecificHide(animate);
if (animate) {
- animation_->Hide();
+ animation_.Hide();
} else {
- animation_->Reset(0.0);
+ animation_.Reset(0.0);
AnimationEnded(NULL);
}
}
@@ -81,7 +81,7 @@ void InfoBar::SetArrowTargetHeight(int height) {
DCHECK_LE(height, kMaximumArrowTargetHeight);
// Once the closing animation starts, we ignore further requests to change the
// target height.
- if ((arrow_target_height_ != height) && !animation()->IsClosing()) {
+ if ((arrow_target_height_ != height) && !animation_.IsClosing()) {
arrow_target_height_ = height;
RecalculateHeights(false);
}
@@ -126,9 +126,9 @@ void InfoBar::RecalculateHeights(bool force_notify) {
// scaling each of these with the square root of the animation value causes a
// linear animation of the area, which matches the perception of the animation
// of the bar portion.
- double scale_factor = sqrt(animation_->GetCurrentValue());
+ double scale_factor = sqrt(animation_.GetCurrentValue());
arrow_height_ = static_cast<int>(arrow_target_height_ * scale_factor);
- if (animation_->is_animating()) {
+ if (animation_.is_animating()) {
arrow_half_width_ = static_cast<int>(std::min(arrow_target_height_,
kMaximumArrowTargetHalfWidth) * scale_factor);
} else {
@@ -147,7 +147,7 @@ void InfoBar::RecalculateHeights(bool force_notify) {
if (arrow_height_)
arrow_height_ += kSeparatorLineHeight;
- bar_height_ = animation_->CurrentValueBetween(0, bar_target_height_);
+ bar_height_ = animation_.CurrentValueBetween(0, bar_target_height_);
// Don't re-layout if nothing has changed, e.g. because the animation step was
// not large enough to actually change the heights by at least a pixel.
@@ -157,11 +157,11 @@ void InfoBar::RecalculateHeights(bool force_notify) {
PlatformSpecificOnHeightsRecalculated();
if (container_ && (heights_differ || force_notify))
- container_->OnInfoBarStateChanged(animation_->is_animating());
+ container_->OnInfoBarStateChanged(animation_.is_animating());
}
void InfoBar::MaybeDelete() {
- if (delegate_ && (animation_->GetCurrentValue() == 0.0)) {
+ if (delegate_ && (animation_.GetCurrentValue() == 0.0)) {
if (container_)
container_->RemoveInfoBar(this);
// Note that we only tell the delegate we're closed here, and not when we're
diff --git a/chrome/browser/tab_contents/infobar.h b/chrome/browser/tab_contents/infobar.h
index 17eccc5..dd78b46 100644
--- a/chrome/browser/tab_contents/infobar.h
+++ b/chrome/browser/tab_contents/infobar.h
@@ -13,6 +13,7 @@
#include "chrome/browser/tab_contents/infobar_delegate.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/base/animation/animation_delegate.h"
+#include "ui/base/animation/slide_animation.h"
#include "ui/gfx/size.h"
// TODO(sail): These functions should be static methods in the InfoBar class
@@ -26,10 +27,6 @@ SkColor GetInfoBarBottomColor(InfoBarDelegate::Type infobar_type);
class InfoBarContainer;
class TabContentsWrapper;
-namespace ui {
-class SlideAnimation;
-}
-
class InfoBar : public ui::AnimationDelegate {
public:
InfoBar(TabContentsWrapper* owner, InfoBarDelegate* delegate);
@@ -60,7 +57,7 @@ class InfoBar : public ui::AnimationDelegate {
// effect once the infobar is animating closed.
void SetArrowTargetHeight(int height);
- const ui::SlideAnimation* animation() const { return animation_.get(); }
+ const ui::SlideAnimation& animation() const { return animation_; }
int arrow_height() const { return arrow_height_; }
int arrow_target_height() const { return arrow_target_height_; }
int arrow_half_width() const { return arrow_half_width_; }
@@ -87,7 +84,7 @@ class InfoBar : public ui::AnimationDelegate {
const InfoBarContainer* container() const { return container_; }
InfoBarContainer* container() { return container_; }
- ui::SlideAnimation* animation() { return animation_.get(); }
+ ui::SlideAnimation* animation() { return &animation_; }
int bar_height() const { return bar_height_; }
int bar_target_height() const { return bar_target_height_; }
@@ -112,10 +109,10 @@ class InfoBar : public ui::AnimationDelegate {
// delete us) and closes the delegate.
void MaybeDelete();
- TabContentsWrapper* owner_; // TODO(pkasting): Transition to using this.
+ TabContentsWrapper* owner_;
InfoBarDelegate* delegate_;
InfoBarContainer* container_;
- scoped_ptr<ui::SlideAnimation> animation_;
+ ui::SlideAnimation animation_;
// The current and target heights of the arrow and bar portions, and half the
// current arrow width. (It's easier to work in half-widths as we draw the
diff --git a/chrome/browser/tab_contents/infobar_container.cc b/chrome/browser/tab_contents/infobar_container.cc
index d1d5dd6..bc1780b 100644
--- a/chrome/browser/tab_contents/infobar_container.cc
+++ b/chrome/browser/tab_contents/infobar_container.cc
@@ -35,11 +35,9 @@ void InfoBarContainer::ChangeTabContents(TabContentsWrapper* contents) {
while (!infobars_.empty()) {
InfoBar* infobar = infobars_.front();
- // NULL the container pointer first so that if the infobar is currently
- // animating, OnInfoBarStateChanged() won't get called; we'll manually
- // trigger this once for the whole set of changes below. This also prevents
- // InfoBar::MaybeDelete() from calling RemoveInfoBar() a second time if the
- // infobar happens to be at zero height (dunno if this can actually happen).
+ // NULL the container pointer first so that the infobar cannot call back to
+ // OnInfoBarStateChanged() for any reason; we'll manually trigger this once
+ // for the whole set of changes below.
infobar->set_container(NULL);
RemoveInfoBar(infobar);
}
@@ -104,11 +102,10 @@ void InfoBarContainer::OnInfoBarStateChanged(bool is_animating) {
}
void InfoBarContainer::RemoveInfoBar(InfoBar* infobar) {
- InfoBars::iterator infobar_iterator(std::find(infobars_.begin(),
- infobars_.end(), infobar));
- DCHECK(infobar_iterator != infobars_.end());
+ InfoBars::iterator i(std::find(infobars_.begin(), infobars_.end(), infobar));
+ DCHECK(i != infobars_.end());
PlatformSpecificRemoveInfoBar(infobar);
- infobars_.erase(infobar_iterator);
+ infobars_.erase(i);
}
void InfoBarContainer::RemoveAllInfoBarsForDestruction() {
@@ -200,16 +197,16 @@ int InfoBarContainer::ArrowTargetHeightForInfoBar(size_t infobar_index) const {
return 0;
if (infobar_index == 0)
return top_arrow_target_height_;
- const ui::SlideAnimation* first_infobar_animation =
+ const ui::SlideAnimation& first_infobar_animation =
const_cast<const InfoBar*>(infobars_.front())->animation();
- if ((infobar_index > 1) || first_infobar_animation->IsShowing())
+ if ((infobar_index > 1) || first_infobar_animation.IsShowing())
return InfoBar::kDefaultArrowTargetHeight;
// When the first infobar is animating closed, we animate the second infobar's
// arrow target height from the default to the top target height. Note that
// the animation values here are going from 1.0 -> 0.0 as the top bar closes.
return top_arrow_target_height_ + static_cast<int>(
(InfoBar::kDefaultArrowTargetHeight - top_arrow_target_height_) *
- first_infobar_animation->GetCurrentValue());
+ first_infobar_animation.GetCurrentValue());
}
#endif // TOOLKIT_VIEWS || defined(TOOLKIT_GTK)
diff --git a/chrome/browser/ui/gtk/infobars/infobar_gtk.cc b/chrome/browser/ui/gtk/infobars/infobar_gtk.cc
index 0b1c5f5..4774147 100644
--- a/chrome/browser/ui/gtk/infobars/infobar_gtk.cc
+++ b/chrome/browser/ui/gtk/infobars/infobar_gtk.cc
@@ -115,7 +115,7 @@ GdkColor InfoBarGtk::GetBorderColor() const {
}
int InfoBarGtk::AnimatingHeight() const {
- return animation()->is_animating() ? bar_target_height() : 0;
+ return animation().is_animating() ? bar_target_height() : 0;
}
SkColor InfoBarGtk::ConvertGetColor(ColorGetter getter) {
diff --git a/chrome/browser/ui/views/infobars/infobar_view.cc b/chrome/browser/ui/views/infobars/infobar_view.cc
index f21bcb1..a08e5e8 100644
--- a/chrome/browser/ui/views/infobars/infobar_view.cc
+++ b/chrome/browser/ui/views/infobars/infobar_view.cc
@@ -13,7 +13,6 @@
#include "grit/theme_resources_standard.h"
#include "third_party/skia/include/effects/SkGradientShader.h"
#include "ui/base/accessibility/accessible_view_state.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/canvas_skia_paint.h"
@@ -207,23 +206,6 @@ void InfoBarView::ViewHierarchyChanged(bool is_add, View* parent, View* child) {
if (child == this) {
if (is_add) {
-#if defined(OS_WIN)
- // When we're added to a view hierarchy within a widget, we create an
- // external focus tracker to track what was focused in case we obtain
- // focus so that we can restore focus when we're removed.
- views::Widget* widget = GetWidget();
- if (widget) {
- focus_tracker_.reset(
- new views::ExternalFocusTracker(this, GetFocusManager()));
- }
-#endif
- if (GetFocusManager())
- GetFocusManager()->AddFocusChangeListener(this);
- if (GetWidget()) {
- GetWidget()->NotifyAccessibilityEvent(
- this, ui::AccessibilityTypes::EVENT_ALERT, true);
- }
-
if (close_button_ == NULL) {
gfx::Image* image = delegate()->GetIcon();
if (image) {
@@ -246,10 +228,11 @@ void InfoBarView::ViewHierarchyChanged(bool is_add, View* parent, View* child) {
AddChildView(close_button_);
}
} else {
- DestroyFocusTracker(false);
- animation()->Stop();
- if (GetFocusManager())
- GetFocusManager()->RemoveFocusChangeListener(this);
+ // TODO(pkasting): Move this to PlatformSpecificHide() once that's
+ // guaranteed to be called each time we're hidden.
+ views::FocusManager* focus_manager = GetFocusManager();
+ if (focus_manager)
+ focus_manager->RemoveFocusChangeListener(this);
}
}
@@ -309,19 +292,38 @@ const InfoBarContainer::Delegate* InfoBarView::container_delegate() const {
return infobar_container ? infobar_container->delegate() : NULL;
}
+void InfoBarView::PlatformSpecificShow(bool animate) {
+ views::Widget* widget = GetWidget();
+ views::FocusManager* focus_manager = GetFocusManager();
+#if defined(OS_WIN)
+ // If we gain focus, we want to restore it to the previously-focused element
+ // when we're hidden. So when we're in a Widget, create a focus tracker so
+ // that if we gain focus we'll know what the previously-focused element was.
+ if (widget) {
+ focus_tracker_.reset(
+ new views::ExternalFocusTracker(this, focus_manager));
+ }
+#endif
+ if (focus_manager)
+ focus_manager->AddFocusChangeListener(this);
+ if (widget) {
+ widget->NotifyAccessibilityEvent(
+ this, ui::AccessibilityTypes::EVENT_ALERT, true);
+ }
+}
+
void InfoBarView::PlatformSpecificHide(bool animate) {
- if (!animate)
+#if defined(OS_WIN)
+ if (!animate || !focus_tracker_.get())
return;
- bool restore_focus = true;
-#if defined(OS_WIN)
- // Do not restore focus (and active state with it) on Windows if some other
- // top-level window became active.
- if (GetWidget() &&
- !ui::DoesWindowBelongToActiveWindow(GetWidget()->GetNativeView()))
- restore_focus = false;
-#endif // defined(OS_WIN)
- DestroyFocusTracker(restore_focus);
+ // Do not restore focus (and active state with it) if some other top-level
+ // window became active.
+ views::Widget* widget = GetWidget();
+ if (!widget || ui::DoesWindowBelongToActiveWindow(widget->GetNativeView()))
+ focus_tracker_->FocusLastFocusedExternalView();
+ focus_tracker_.reset();
+#endif
}
void InfoBarView::PlatformSpecificOnHeightsRecalculated() {
@@ -346,18 +348,9 @@ gfx::Size InfoBarView::GetPreferredSize() {
void InfoBarView::FocusWillChange(View* focused_before, View* focused_now) {
// This will trigger some screen readers to read the entire contents of this
// infobar.
- if (focused_before && focused_now && !this->Contains(focused_before) &&
- this->Contains(focused_now) && GetWidget()) {
+ if (focused_before && focused_now && !Contains(focused_before) &&
+ Contains(focused_now) && GetWidget()) {
GetWidget()->NotifyAccessibilityEvent(
this, ui::AccessibilityTypes::EVENT_ALERT, true);
}
}
-
-void InfoBarView::DestroyFocusTracker(bool restore_focus) {
- if (focus_tracker_ != NULL) {
- if (restore_focus)
- focus_tracker_->FocusLastFocusedExternalView();
- focus_tracker_->SetFocusManager(NULL);
- focus_tracker_.reset();
- }
-}
diff --git a/chrome/browser/ui/views/infobars/infobar_view.h b/chrome/browser/ui/views/infobars/infobar_view.h
index 89fba9f..5f1e856 100644
--- a/chrome/browser/ui/views/infobars/infobar_view.h
+++ b/chrome/browser/ui/views/infobars/infobar_view.h
@@ -87,6 +87,7 @@ class InfoBarView : public InfoBar,
static const int kHorizontalPadding;
// InfoBar:
+ virtual void PlatformSpecificShow(bool animate) OVERRIDE;
virtual void PlatformSpecificHide(bool animate) OVERRIDE;
virtual void PlatformSpecificOnHeightsRecalculated() OVERRIDE;
@@ -99,11 +100,6 @@ class InfoBarView : public InfoBar,
virtual void FocusWillChange(View* focused_before,
View* focused_now) OVERRIDE;
- // Destroys the external focus tracker, if present. If |restore_focus| is
- // true, restores focus to the view tracked by the focus tracker before doing
- // so.
- void DestroyFocusTracker(bool restore_focus);
-
// The optional icon at the left edge of the InfoBar.
views::ImageView* icon_;