Guard against division by zero for useless scroll bars.

Guard against division by zero for useless scroll bars.

This patch adds protection against division by zero when a scroll bar's
thumb is the full size of the track (i.e. the scroll bar is useless).

BUG=591933
Review URL: https://codereview.chromium.org/1769553003

Cr-Commit-Position: refs/heads/master@{#379437}

(cherry-picked from commit 5c6639a0a0b75d2696f6b3c9a5a47c8dc6096df8)

R=sky@chromium.org

Committed: https://chromium.googlesource.com/chromium/src/+/87686973fe8171dfbc515c81e50b9602b9a8cf12

[Reilly Grant (use Gerrit), 2016-03-07 21:57:19]

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

[Reilly Grant (use Gerrit), 2016-03-07 21:57:19]

Please review this merge.

[sky, 2016-03-07 23:50:55]

Generally we just TBR merges.

On Mon, Mar 7, 2016 at 1:57 PM,  <reillyg@chromium.org> wrote:
> Reviewers: sky
> CL: https://codereview.chromium.org/1772023002/
>
> Message:
> Please review this merge.
>
> Description:
> Guard against division by zero for useless scroll bars.
>
> This patch adds protection against division by zero when a scroll bar's
> thumb is the full size of the track (i.e. the scroll bar is useless).
>
> BUG=591933
>
> Review URL: https://codereview.chromium.org/1769553003
>
> Cr-Commit-Position: refs/heads/master@{#379437}
>
> (cherry-picked from commit 5c6639a0a0b75d2696f6b3c9a5a47c8dc6096df8)
>
> Base URL: https://chromium.googlesource.com/chromium/src.git@2661
>
> Affected files (+31, -2 lines):
> M ui/views/controls/scrollbar/base_scroll_bar.h
> M ui/views/controls/scrollbar/base_scroll_bar.cc
> M ui/views/controls/scrollbar/scrollbar_unittest.cc
>
>
> Index: ui/views/controls/scrollbar/base_scroll_bar.cc
> diff --git a/ui/views/controls/scrollbar/base_scroll_bar.cc
> b/ui/views/controls/scrollbar/base_scroll_bar.cc
> index
>
be0740f6b2cb3b8c6edefefd317ea0e11fac0346..e9333e1458c81e97e8522c04364e4177fc1b0ab7
> 100644
> --- a/ui/views/controls/scrollbar/base_scroll_bar.cc
> +++ b/ui/views/controls/scrollbar/base_scroll_bar.cc
> @@ -413,7 +413,8 @@ void BaseScrollBar::Update(int viewport_size,
> // Thumb Height and Thumb Pos.
> // The height of the thumb is the ratio of the Viewport height to the
> // content size multiplied by the height of the thumb track.
> - double ratio = static_cast<double>(viewport_size) / contents_size_;
> + double ratio =
> + std::min(1.0, static_cast<double>(viewport_size) / contents_size_);
> int thumb_size = static_cast<int>(ratio * GetTrackSize());
> thumb_->SetSize(thumb_size);
>
> @@ -505,10 +506,13 @@ int BaseScrollBar::CalculateThumbPosition(int
> contents_scroll_offset) const {
> int BaseScrollBar::CalculateContentsOffset(int thumb_position,
> bool scroll_to_middle) const {
> int thumb_size = thumb_->GetSize();
> + int track_size = GetTrackSize();
> + if (track_size == thumb_size)
> + return 0;
> if (scroll_to_middle)
> thumb_position = thumb_position - (thumb_size / 2);
> return (thumb_position * (contents_size_ - viewport_size_)) /
> - (GetTrackSize() - thumb_size);
> + (track_size - thumb_size);
> }
>
> void BaseScrollBar::SetThumbTrackState(CustomButton::ButtonState state) {
> Index: ui/views/controls/scrollbar/base_scroll_bar.h
> diff --git a/ui/views/controls/scrollbar/base_scroll_bar.h
> b/ui/views/controls/scrollbar/base_scroll_bar.h
> index
>
f54a88081786aea444617189a6ba477bfc16e837..178fdc1e9081a2a00551a01713028a18981e703f
> 100644
> --- a/ui/views/controls/scrollbar/base_scroll_bar.h
> +++ b/ui/views/controls/scrollbar/base_scroll_bar.h
> @@ -115,6 +115,7 @@ class VIEWS_EXPORT BaseScrollBar : public ScrollBar,
>
> private:
> FRIEND_TEST_ALL_PREFIXES(NativeScrollBarTest, ScrollBarFitsToBottom);
> + FRIEND_TEST_ALL_PREFIXES(NativeScrollBarTest, ThumbFullLengthOfTrack);
> int GetThumbSizeForTest();
>
> // Changes to 'pushed' state and starts a timer to scroll repeatedly.
> Index: ui/views/controls/scrollbar/scrollbar_unittest.cc
> diff --git a/ui/views/controls/scrollbar/scrollbar_unittest.cc
> b/ui/views/controls/scrollbar/scrollbar_unittest.cc
> index
>
4bfab107428badafa9674de437d5b1508c0c0628..84471929234f8271eb7746b6f8bc893eb3f96548
> 100644
> --- a/ui/views/controls/scrollbar/scrollbar_unittest.cc
> +++ b/ui/views/controls/scrollbar/scrollbar_unittest.cc
> @@ -175,4 +175,28 @@ TEST_F(NativeScrollBarTest,
> ScrollToEndAfterShrinkAndExpand) {
> EXPECT_TRUE(scrollbar_->ScrollByContentsOffset(-1));
> }
>
> +TEST_F(NativeScrollBarTest, ThumbFullLengthOfTrack) {
> + // Shrink content so that it fits within the viewport.
> + scrollbar_->Update(100, 10, 0);
> + EXPECT_EQ(scrollbar_->GetTrackBounds().width(),
> + scrollbar_->GetThumbSizeForTest());
> + // Emulate a click on the full size scroll bar.
> + scrollbar_->ScrollToThumbPosition(0, false);
> + EXPECT_EQ(0, scrollbar_->GetPosition());
> + // Emulate a key down.
> + scrollbar_->ScrollByAmount(BaseScrollBar::SCROLL_NEXT_LINE);
> + EXPECT_EQ(0, scrollbar_->GetPosition());
> +
> + // Expand content so that it fits *exactly* within the viewport.
> + scrollbar_->Update(100, 100, 0);
> + EXPECT_EQ(scrollbar_->GetTrackBounds().width(),
> + scrollbar_->GetThumbSizeForTest());
> + // Emulate a click on the full size scroll bar.
> + scrollbar_->ScrollToThumbPosition(0, false);
> + EXPECT_EQ(0, scrollbar_->GetPosition());
> + // Emulate a key down.
> + scrollbar_->ScrollByAmount(BaseScrollBar::SCROLL_NEXT_LINE);
> + EXPECT_EQ(0, scrollbar_->GetPosition());
> +}
> +
> } // namespace views
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sky, 2016-03-07 23:51:04]

LGTM

[Reilly Grant (use Gerrit), 2016-03-07 23:57:55]

Description was changed from

==========
Guard against division by zero for useless scroll bars.

This patch adds protection against division by zero when a scroll bar's
thumb is the full size of the track (i.e. the scroll bar is useless).

BUG=591933

Review URL: https://codereview.chromium.org/1769553003

Cr-Commit-Position: refs/heads/master@{#379437}

(cherry-picked from commit 5c6639a0a0b75d2696f6b3c9a5a47c8dc6096df8)
==========

to

==========
Guard against division by zero for useless scroll bars.

This patch adds protection against division by zero when a scroll bar's
thumb is the full size of the track (i.e. the scroll bar is useless).

BUG=591933

Review URL: https://codereview.chromium.org/1769553003

Cr-Commit-Position: refs/heads/master@{#379437}

(cherry-picked from commit 5c6639a0a0b75d2696f6b3c9a5a47c8dc6096df8)

R=sky@chromium.org

Committed:
https://chromium.googlesource.com/chromium/src/+/87686973fe8171dfbc515c81e50b...
==========

[Reilly Grant (use Gerrit), 2016-03-07 23:57:56]

Committed patchset #1 (id:1) manually as
87686973fe8171dfbc515c81e50b9602b9a8cf12 (presubmit successful).