Misc. Tab-related cleanup.

Misc. Tab-related cleanup.

* Pass in the AnimationContainer directly instead of having a setter, since the only place that ever sets this does it immediately on creation.  This allows Tab to not have to worry about if the animation container changes or disappears over time.
* Comment fix
* Create animations unconditionally in constructor instead of dynamically as needed.  Animations aren't that big or expensive to create, and doing this avoids the need to remember to null-check everywhere in the code.
* Inline some constants right above the sole block where they're used
* Add 4-arg constructor for MultiAnimation::Parts and use constructor delegation to minimize the duplicated code footprint
* Reorder a conditional to remove nesting
* When converting from double to int, round instead of truncating; use ToRoundedInt() as much as possible
* Make boolean expression somewhat less confusing
* Rename some constants to more closely match their use
* Better structure for #ifdefs in TabStrip::GetInactiveAlpha()

BUG=none
TEST=none

Committed: https://crrev.com/158f43592244183d9f8e2666b27e3637fa1dfc21
Cr-Commit-Position: refs/heads/master@{#377281}

[Peter Kasting, 2016-02-19 00:16:32]

Description was changed from

==========
Misc. Tab-related cleanup.

BUG=none
TEST=none
==========

to

==========
Misc. Tab-related cleanup.

* Pass in the AnimationContainer directly instead of having a setter, since the
only place that ever sets this does it immediately on creation.  This allows Tab
to not have to worry about if the animation container changes or disappears over
time.
* Comment fix
* Create animations unconditionally in constructor instead of dynamically as
needed.  Animations aren't that big or expensive to create, and doing this
avoids the need to remember to null-check everywhere in the code.
* Inline some constants right above the sole block where they're used
* Add 4-arg constructor for MultiAnimation::Parts and use constructor delegation
to minimize the duplicated code footprint
* Reorder a conditional to remove nesting
* When converting from double to int, round instead of truncating; use
ToRoundedInt() as much as possible
* Make boolean expression somewhat less confusing
* Rename some constants to more closely match their use

BUG=none
TEST=none
==========

[Peter Kasting, 2016-02-19 00:16:32]

pkasting@chromium.org changed reviewers:
+ sky@chromium.org

[sky, 2016-02-19 05:01:05]

LGTM

https://codereview.chromium.org/1710253003/diff/1/chrome/browser/ui/views/tab...
File chrome/browser/ui/views/tabs/tab.cc (right):

https://codereview.chromium.org/1710253003/diff/1/chrome/browser/ui/views/tab...
chrome/browser/ui/views/tabs/tab.cc:642: crash_icon_animation_->Stop();
Indenting is off here (looks like it was before too), and Stop() early outs if
!animating.

[Peter Kasting, 2016-02-22 07:39:05]

https://codereview.chromium.org/1710253003/diff/1/chrome/browser/ui/views/tab...
File chrome/browser/ui/views/tabs/tab.cc (right):

https://codereview.chromium.org/1710253003/diff/1/chrome/browser/ui/views/tab...
chrome/browser/ui/views/tabs/tab.cc:642: crash_icon_animation_->Stop();
On 2016/02/19 05:01:04, sky wrote:
> Indenting is off here (looks like it was before too), and Stop() early outs if
> !animating.

But we know that is_animating() is true from the conditional.  So there's no
difference.  Right?  Or am I misunderstanding your concern?

[sky, 2016-02-22 16:42:09]

https://codereview.chromium.org/1710253003/diff/1/chrome/browser/ui/views/tab...
File chrome/browser/ui/views/tabs/tab.cc (right):

https://codereview.chromium.org/1710253003/diff/1/chrome/browser/ui/views/tab...
chrome/browser/ui/views/tabs/tab.cc:642: crash_icon_animation_->Stop();
On 2016/02/22 07:39:05, Peter Kasting wrote:
> On 2016/02/19 05:01:04, sky wrote:
> > Indenting is off here (looks like it was before too), and Stop() early outs
if
> > !animating.
> 
> But we know that is_animating() is true from the conditional.  So there's no
> difference.  Right?  Or am I misunderstanding your concern?

Sorry if I wasn't clear. I'm suggesting there is no point in the conditional.
Call Stop() unconditionally as Stop() does nothing if not animating.

[Peter Kasting, 2016-02-24 12:23:35]

Description was changed from

==========
Misc. Tab-related cleanup.

* Pass in the AnimationContainer directly instead of having a setter, since the
only place that ever sets this does it immediately on creation.  This allows Tab
to not have to worry about if the animation container changes or disappears over
time.
* Comment fix
* Create animations unconditionally in constructor instead of dynamically as
needed.  Animations aren't that big or expensive to create, and doing this
avoids the need to remember to null-check everywhere in the code.
* Inline some constants right above the sole block where they're used
* Add 4-arg constructor for MultiAnimation::Parts and use constructor delegation
to minimize the duplicated code footprint
* Reorder a conditional to remove nesting
* When converting from double to int, round instead of truncating; use
ToRoundedInt() as much as possible
* Make boolean expression somewhat less confusing
* Rename some constants to more closely match their use

BUG=none
TEST=none
==========

to

==========
Misc. Tab-related cleanup.

* Pass in the AnimationContainer directly instead of having a setter, since the
only place that ever sets this does it immediately on creation.  This allows Tab
to not have to worry about if the animation container changes or disappears over
time.
* Comment fix
* Create animations unconditionally in constructor instead of dynamically as
needed.  Animations aren't that big or expensive to create, and doing this
avoids the need to remember to null-check everywhere in the code.
* Inline some constants right above the sole block where they're used
* Add 4-arg constructor for MultiAnimation::Parts and use constructor delegation
to minimize the duplicated code footprint
* Reorder a conditional to remove nesting
* When converting from double to int, round instead of truncating; use
ToRoundedInt() as much as possible
* Make boolean expression somewhat less confusing
* Rename some constants to more closely match their use
* Better structure for #ifdefs in TabStrip::GetInactiveAlpha()

BUG=none
TEST=none
==========

[Peter Kasting, 2016-02-24 12:24:49]

https://codereview.chromium.org/1710253003/diff/1/chrome/browser/ui/views/tab...
File chrome/browser/ui/views/tabs/tab.cc (right):

https://codereview.chromium.org/1710253003/diff/1/chrome/browser/ui/views/tab...
chrome/browser/ui/views/tabs/tab.cc:642: crash_icon_animation_->Stop();
On 2016/02/22 16:42:08, sky wrote:
> On 2016/02/22 07:39:05, Peter Kasting wrote:
> > On 2016/02/19 05:01:04, sky wrote:
> > > Indenting is off here (looks like it was before too), and Stop() early
outs
> if
> > > !animating.
> > 
> > But we know that is_animating() is true from the conditional.  So there's no
> > difference.  Right?  Or am I misunderstanding your concern?
> 
> Sorry if I wasn't clear. I'm suggesting there is no point in the conditional.
> Call Stop() unconditionally as Stop() does nothing if not animating.

Done.

[Peter Kasting, 2016-02-24 12:24:55]

The CQ bit was checked by pkasting@chromium.org

[Peter Kasting, 2016-02-24 12:24:56]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1710253003/#ps40001
(title: "Better structure for #ifdefs")

[commit-bot: I haz the power, 2016-02-24 12:25:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1710253003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1710253003/40001

[commit-bot: I haz the power, 2016-02-24 13:39:21]

Description was changed from

==========
Misc. Tab-related cleanup.

* Pass in the AnimationContainer directly instead of having a setter, since the
only place that ever sets this does it immediately on creation.  This allows Tab
to not have to worry about if the animation container changes or disappears over
time.
* Comment fix
* Create animations unconditionally in constructor instead of dynamically as
needed.  Animations aren't that big or expensive to create, and doing this
avoids the need to remember to null-check everywhere in the code.
* Inline some constants right above the sole block where they're used
* Add 4-arg constructor for MultiAnimation::Parts and use constructor delegation
to minimize the duplicated code footprint
* Reorder a conditional to remove nesting
* When converting from double to int, round instead of truncating; use
ToRoundedInt() as much as possible
* Make boolean expression somewhat less confusing
* Rename some constants to more closely match their use
* Better structure for #ifdefs in TabStrip::GetInactiveAlpha()

BUG=none
TEST=none
==========

to

==========
Misc. Tab-related cleanup.

* Pass in the AnimationContainer directly instead of having a setter, since the
only place that ever sets this does it immediately on creation.  This allows Tab
to not have to worry about if the animation container changes or disappears over
time.
* Comment fix
* Create animations unconditionally in constructor instead of dynamically as
needed.  Animations aren't that big or expensive to create, and doing this
avoids the need to remember to null-check everywhere in the code.
* Inline some constants right above the sole block where they're used
* Add 4-arg constructor for MultiAnimation::Parts and use constructor delegation
to minimize the duplicated code footprint
* Reorder a conditional to remove nesting
* When converting from double to int, round instead of truncating; use
ToRoundedInt() as much as possible
* Make boolean expression somewhat less confusing
* Rename some constants to more closely match their use
* Better structure for #ifdefs in TabStrip::GetInactiveAlpha()

BUG=none
TEST=none
==========

[commit-bot: I haz the power, 2016-02-24 13:39:23]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-24 13:40:20]

Description was changed from

==========
Misc. Tab-related cleanup.

* Pass in the AnimationContainer directly instead of having a setter, since the
only place that ever sets this does it immediately on creation.  This allows Tab
to not have to worry about if the animation container changes or disappears over
time.
* Comment fix
* Create animations unconditionally in constructor instead of dynamically as
needed.  Animations aren't that big or expensive to create, and doing this
avoids the need to remember to null-check everywhere in the code.
* Inline some constants right above the sole block where they're used
* Add 4-arg constructor for MultiAnimation::Parts and use constructor delegation
to minimize the duplicated code footprint
* Reorder a conditional to remove nesting
* When converting from double to int, round instead of truncating; use
ToRoundedInt() as much as possible
* Make boolean expression somewhat less confusing
* Rename some constants to more closely match their use
* Better structure for #ifdefs in TabStrip::GetInactiveAlpha()

BUG=none
TEST=none
==========

to

==========
Misc. Tab-related cleanup.

* Pass in the AnimationContainer directly instead of having a setter, since the
only place that ever sets this does it immediately on creation.  This allows Tab
to not have to worry about if the animation container changes or disappears over
time.
* Comment fix
* Create animations unconditionally in constructor instead of dynamically as
needed.  Animations aren't that big or expensive to create, and doing this
avoids the need to remember to null-check everywhere in the code.
* Inline some constants right above the sole block where they're used
* Add 4-arg constructor for MultiAnimation::Parts and use constructor delegation
to minimize the duplicated code footprint
* Reorder a conditional to remove nesting
* When converting from double to int, round instead of truncating; use
ToRoundedInt() as much as possible
* Make boolean expression somewhat less confusing
* Rename some constants to more closely match their use
* Better structure for #ifdefs in TabStrip::GetInactiveAlpha()

BUG=none
TEST=none

Committed: https://crrev.com/158f43592244183d9f8e2666b27e3637fa1dfc21
Cr-Commit-Position: refs/heads/master@{#377281}
==========

[commit-bot: I haz the power, 2016-02-24 13:40:21]

Patchset 3 (id:??) landed as
https://crrev.com/158f43592244183d9f8e2666b27e3637fa1dfc21
Cr-Commit-Position: refs/heads/master@{#377281}