Commit 224861a6 authored by Mark Cogan's avatar Mark Cogan Committed by Commit Bot

[iOS] Stop computing illegal TabGridPage values.

The GetPageFromScrollView() utility function in TabGridViewController
computes a TabGridPage value (an enum in the range 0..2) by dividing the
scroll view offset by its page width and then lround()ing. This can, it
turns out, produce values outside of the 0..2 range, which is not caught
at runtime.

This causes problems because the currentPageViewController property getter
uses a switch() based on a TabGridPage value. Because the switch is over
an enum, there's no default case. But because this enum is just a typedef
for an int, there's no guarantees that all enum values are valid. When a
TabGridPage value of, say, 3 (or MAXINT, or whatever) is set for
_currentPage, none of the switch cases in currentPageViewController are
hit, so it doesn't set a return value. This means that at runtime the
return value is random memory, which means crashes of various kinds.

This CL is the quick fix: it clamps the value of the page computation to
the defined range of the TabGridPage typedef. This is suitable for merging
into release.

The better fix, which is to change TabGridPage to an enum class and stop
inferring discrete values using just arithmetic, will be in a follow-up
CL, which will probably touch too much code for convenient merging, but
which can land on trunk for future releases.

Bug: 979683
Change-Id: If817d6c6ca0e657d5a49efbc2169601e44b29bdb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1728882Reviewed-by: default avataredchin <edchin@chromium.org>
Commit-Queue: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682753}
parent 767625c7
...@@ -80,7 +80,12 @@ void RecordPageChangeInteraction(TabSwitcherPageChangeInteraction interaction) { ...@@ -80,7 +80,12 @@ void RecordPageChangeInteraction(TabSwitcherPageChangeInteraction interaction) {
// Computes the page from the offset and width of |scrollView|. // Computes the page from the offset and width of |scrollView|.
TabGridPage GetPageFromScrollView(UIScrollView* scrollView) { TabGridPage GetPageFromScrollView(UIScrollView* scrollView) {
CGFloat pageWidth = scrollView.frame.size.width; CGFloat pageWidth = scrollView.frame.size.width;
NSUInteger page = lround(scrollView.contentOffset.x / pageWidth); CGFloat offset = scrollView.contentOffset.x;
NSUInteger page = lround(offset / pageWidth);
// Fence |page| to valid values; page values of 3 (rounded up from 2.5) are
// possible, as are large int values if |pageWidth| is somehow very small.
page = page < TabGridPageIncognitoTabs ? TabGridPageIncognitoTabs : page;
page = page > TabGridPageRemoteTabs ? TabGridPageRemoteTabs : page;
if (UseRTLLayout()) { if (UseRTLLayout()) {
// In RTL, page indexes are inverted, so subtract |page| from the highest- // In RTL, page indexes are inverted, so subtract |page| from the highest-
// index TabGridPage value. // index TabGridPage value.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment