Replace default page scale limits in PageScaleConstraintsSet.

Replace default page scale limits in PageScaleConstraintsSet.

This fixes crbug.com/411933 where overlay scrollbars in ChromeOS would show
up on the new tab page even though they weren't needed. This was occurring
because the page scale factor limits, though set in RenderViewImpl::Initialize,
were ineffective until a layout occured since Blink's PageScaleConstraintsSet
wouldn't have computed the final constraints. Thus, the limits used were Blink's
default limits, set to hardcoded values in PageScaleConstraintsSet. This caused
the scrollbar in the compositor to think the page was zoomed in and displayed
the scrollbars.

This patch replaces the uncomputed final constraints with "all 1" values for
minimum, maximum and initial scale, rather than using the (Android-specific)
defaults.

BUG=411933
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181848

[bokan, 2014-09-11 16:47:17]

bokan@chromium.org changed reviewers:
+ aelias@chromium.org

[bokan, 2014-09-11 16:47:19]

Alexandre,

PTAL. I think we're going to want to merge this to M38 so this is kind of a
quick fix with minimal impact. I've filed crbug.com/413285 on myself to clean
this up long term.

Thanks.

[aelias_OOO_until_Jul13, 2014-09-11 17:07:07]

On 2014/09/11 16:47:19, bokan wrote:
> Alexandre,
> 
> PTAL. I think we're going to want to merge this to M38 so this is kind of a
> quick fix with minimal impact. I've filed crbug.com/413285 on myself to clean
> this up long term.
> 
> Thanks.

I don't think the settings are initialized yet in the constructor so I think
this would alter Android behavior?

[bokan, 2014-09-11 17:54:41]

On 2014/09/11 17:07:07, aelias wrote:
> On 2014/09/11 16:47:19, bokan wrote:
> > Alexandre,
> > 
> > PTAL. I think we're going to want to merge this to M38 so this is kind of a
> > quick fix with minimal impact. I've filed crbug.com/413285 on myself to
clean
> > this up long term.
> > 
> > Thanks.
> 
> I don't think the settings are initialized yet in the constructor so I think
> this would alter Android behavior?

Ah, yes, you're right.

As far as I can tell, the only non-test caller of this method is the desktop
initialization path in RenderViewImpl. Given it doesn't break any tests, is the
updated patch ok for now?

[aelias_OOO_until_Jul13, 2014-09-11 18:35:05]

On 2014/09/11 17:54:41, bokan wrote:
> On 2014/09/11 17:07:07, aelias wrote:
> > On 2014/09/11 16:47:19, bokan wrote:
> > > Alexandre,
> > > 
> > > PTAL. I think we're going to want to merge this to M38 so this is kind of
a
> > > quick fix with minimal impact. I've filed crbug.com/413285 on myself to
> clean
> > > this up long term.
> > > 
> > > Thanks.
> > 
> > I don't think the settings are initialized yet in the constructor so I think
> > this would alter Android behavior?
> 
> Ah, yes, you're right.
> 
> As far as I can tell, the only non-test caller of this method is the desktop
> initialization path in RenderViewImpl. Given it doesn't break any tests, is
the
> updated patch ok for now?

No, sorry, seems too sketchy to me.  Would it work to change the initial values
of final minpagescale and maxpagescale to 1 in the constraints set constructor? 
I think that might even be a true nonhacky fix as I don't expect any problem on
Android from that.

[bokan, 2014-09-11 19:00:50]

On 2014/09/11 18:35:05, aelias wrote:
> On 2014/09/11 17:54:41, bokan wrote:
> > On 2014/09/11 17:07:07, aelias wrote:
> > > On 2014/09/11 16:47:19, bokan wrote:
> > > > Alexandre,
> > > > 
> > > > PTAL. I think we're going to want to merge this to M38 so this is kind
of
> a
> > > > quick fix with minimal impact. I've filed crbug.com/413285 on myself to
> > clean
> > > > this up long term.
> > > > 
> > > > Thanks.
> > > 
> > > I don't think the settings are initialized yet in the constructor so I
think
> > > this would alter Android behavior?
> > 
> > Ah, yes, you're right.
> > 
> > As far as I can tell, the only non-test caller of this method is the desktop
> > initialization path in RenderViewImpl. Given it doesn't break any tests, is
> the
> > updated patch ok for now?
> 
> No, sorry, seems too sketchy to me.  Would it work to change the initial
values
> of final minpagescale and maxpagescale to 1 in the constraints set
constructor? 
> I think that might even be a true nonhacky fix as I don't expect any problem
on
> Android from that.

Thanks, the above fix does indeed seem to work on both platforms.

[aelias_OOO_until_Jul13, 2014-09-11 19:28:06]

lgtm modulo out-of-date patch description

[bokan, 2014-09-11 19:34:36]

The CQ bit was checked by bokan@chromium.org

[commit-bot: I haz the power, 2014-09-11 19:34:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/567663002/40001

[commit-bot: I haz the power, 2014-09-11 20:42:01]

Committed patchset #3 (id:40001) as 181848