Invalidate the text autosizer on meta viewport settings changes

Invalidate the text autosizer on meta viewport settings changes

This patch is a followup to [1] where we noticed that viewport settings
were not invalidating the text autosizer. The text autosizer depends on
how the viewport is specified so we need to invalidate it on changes. A
test has been updated to show this works.

[1] https://crrev.com/1229b1414d64c314666d2fb3bf47706f2b5e947b

Committed: https://crrev.com/795773c25806d52ae98b77d2cdd27d0325319b0f
Cr-Commit-Position: refs/heads/master@{#419378}

[pdr., 2016-09-14 18:11:58]

pdr@chromium.org changed reviewers:
+ skobes@chromium.org

[pdr., 2016-09-14 18:12:03]

The CQ bit was checked by pdr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-14 18:12:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-14 20:43:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-14 20:43:03]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[rune, 2016-09-15 08:43:17]

rune@opera.com changed reviewers:
+ rune@opera.com

[rune, 2016-09-15 08:43:18]

https://codereview.chromium.org/2341843002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/page/Page.cpp (right):

https://codereview.chromium.org/2341843002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/Page.cpp:347:
textAutosizer->updatePageInfoInAllFrames();
Wouldn't this also be an issue when you set the viewport meta late. That is,
after you have layed out once?

If so, I think this should be handled where the viewport is updated.

[skobes, 2016-09-16 01:19:57]

https://codereview.chromium.org/2341843002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/page/Page.cpp (right):

https://codereview.chromium.org/2341843002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/Page.cpp:347:
textAutosizer->updatePageInfoInAllFrames();
On 2016/09/15 08:43:18, rune wrote:
> Wouldn't this also be an issue when you set the viewport meta late. That is,
> after you have layed out once?
> 
> If so, I think this should be handled where the viewport is updated.

It looks like we already call updatePageInfoInAllFrames from
WebViewImpl::updatePageDefinedViewportConstraints, which gets called when the
viewport meta changes.

But in that case I am not sure why we also need it here?

[pdr., 2016-09-17 05:14:54]

https://codereview.chromium.org/2341843002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/page/Page.cpp (right):

https://codereview.chromium.org/2341843002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/Page.cpp:347:
textAutosizer->updatePageInfoInAllFrames();
On 2016/09/16 at 01:19:57, skobes wrote:
> On 2016/09/15 08:43:18, rune wrote:
> > Wouldn't this also be an issue when you set the viewport meta late. That is,
> > after you have layed out once?
> > 
> > If so, I think this should be handled where the viewport is updated.
> 
> It looks like we already call updatePageInfoInAllFrames from
WebViewImpl::updatePageDefinedViewportConstraints, which gets called when the
viewport meta changes.
> 
> But in that case I am not sure why we also need it here?

This should only be called for settings changes like in the test, but not for
actual viewport changes from users.

We do update the text autosizer in updatePageDefinedViewportConstraints but
there's an early out before then if the viewport setting is not enabled. We
could update the autosizer when the viewport setting is not enabled too, but
that seems heavyweight for a bug that only occurs on settings changes.

[skobes, 2016-09-17 05:35:56]

lgtm

[rune, 2016-09-17 14:08:16]

lgtm

[pdr., 2016-09-17 17:20:59]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2016-09-17 17:21:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-17 19:42:45]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-09-17 19:45:49]

Description was changed from

==========
Invalidate the text autosizer on meta viewport settings changes

This patch is a followup to [1] where we noticed that viewport settings
were not invalidating the text autosizer. The text autosizer depends on
how the viewport is specified so we need to invalidate it on changes. A
test has been updated to show this works.

[1] https://crrev.com/1229b1414d64c314666d2fb3bf47706f2b5e947b
==========

to

==========
Invalidate the text autosizer on meta viewport settings changes

This patch is a followup to [1] where we noticed that viewport settings
were not invalidating the text autosizer. The text autosizer depends on
how the viewport is specified so we need to invalidate it on changes. A
test has been updated to show this works.

[1] https://crrev.com/1229b1414d64c314666d2fb3bf47706f2b5e947b

Committed: https://crrev.com/795773c25806d52ae98b77d2cdd27d0325319b0f
Cr-Commit-Position: refs/heads/master@{#419378}
==========

[commit-bot: I haz the power, 2016-09-17 19:45:51]

Patchset 1 (id:??) landed as
https://crrev.com/795773c25806d52ae98b77d2cdd27d0325319b0f
Cr-Commit-Position: refs/heads/master@{#419378}