Gracefully handle extremely small windows in tab-modal sign in flow.

Gracefully handle extremely small windows in tab-modal sign in flow.

This allows the sign in flow to display scroll bars for very small
Chrome windows on Linux and Windows.

BUG=617733
TEST=
1. Enable the Password Separated Sign In Flow in chrome://flags
2. Resize the Chrome window to be smaller than 600 pixels in height
3. In the User Menu, click Sign In to Chrome
4. The Sign In flow content and the Sync Confirmation dialog should
   both display scrollbars allowing interaction with the content.
5. In large windows, no scroll bar should be seen.

Committed: https://crrev.com/66d94fd2fee2dd2a080c9bb5d29bec349c71dfa2
Cr-Commit-Position: refs/heads/master@{#400504}

[anthonyvd, 2016-06-15 16:56:28]

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

[anthonyvd, 2016-06-15 16:56:28]

Hi sky@,

Can you PTAL at this small change? It restrict the dialog height of the
tab-modal sign in flow when the Chrome window is very small. This allows the
content to gracefully render a scrollbar and leave the dialogs usable.

Thanks!

[sky, 2016-06-16 16:13:29]

https://codereview.chromium.org/2059363004/diff/1/chrome/browser/ui/views/pro...
File chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.cc
(right):

https://codereview.chromium.org/2059363004/diff/1/chrome/browser/ui/views/pro...
chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.cc:144:
auto max_height = browser
'int' is one character shorter than 'auto' and is much clearer.

https://codereview.chromium.org/2059363004/diff/1/chrome/browser/ui/views/pro...
chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.cc:147:
->GetMaximumDialogSize().height();
Using Browser here means I have to think about lifetime. Could you instead
supply the height so I don't have to worry about lifetime? Supplying the height
also minimizes dependencies.

[anthonyvd, 2016-06-17 18:51:49]

https://codereview.chromium.org/2059363004/diff/1/chrome/browser/ui/views/pro...
File chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.cc
(right):

https://codereview.chromium.org/2059363004/diff/1/chrome/browser/ui/views/pro...
chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.cc:144:
auto max_height = browser
On 2016/06/16 at 16:13:28, sky wrote:
> 'int' is one character shorter than 'auto' and is much clearer.

Done.

https://codereview.chromium.org/2059363004/diff/1/chrome/browser/ui/views/pro...
chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.cc:147:
->GetMaximumDialogSize().height();
On 2016/06/16 at 16:13:28, sky wrote:
> Using Browser here means I have to think about lifetime. Could you instead
supply the height so I don't have to worry about lifetime? Supplying the height
also minimizes dependencies.

This class already uses Browser for many other things though. For example, the
caller of this function below
(SigninViewControllerDelegate::CreateModalSigninDelegate) already receives a
browser, which is in turn passed to the SigninViewControllerDelegateViews ctor.
The class then stores the Browser* and uses it later to get the active web
contents. I don't think I can break this dependency, so using Browser here isn't
introducing a new lifetime problem, or a new dependency.

Ultimately, this delegate is owned by SigninViewController, which is owned by
Browser, so there shouldn't be too many lifetime considerations.

WDYT?

[sky, 2016-06-17 19:50:41]

Fair enough, LGTM

[anthonyvd, 2016-06-17 20:00:34]

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

[commit-bot: I haz the power, 2016-06-17 20:01:00]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2059363004/20001

[commit-bot: I haz the power, 2016-06-17 20:43:44]

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

[commit-bot: I haz the power, 2016-06-17 20:43:45]

Dry run: This issue passed the CQ dry run.

[anthonyvd, 2016-06-17 21:04:47]

The CQ bit was checked by anthonyvd@chromium.org

[commit-bot: I haz the power, 2016-06-17 21:05:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2059363004/20001

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

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-17 21:11:49]

Description was changed from

==========
Gracefully handle extremely small windows in tab-modal sign in flow.

This allows the sign in flow to display scroll bars for very small
Chrome windows on Linux and Windows.

BUG=617733
TEST=
1. Enable the Password Separated Sign In Flow in chrome://flags
2. Resize the Chrome window to be smaller than 600 pixels in height
3. In the User Menu, click Sign In to Chrome
4. The Sign In flow content and the Sync Confirmation dialog should
   both display scrollbars allowing interaction with the content.
5. In large windows, no scroll bar should be seen.
==========

to

==========
Gracefully handle extremely small windows in tab-modal sign in flow.

This allows the sign in flow to display scroll bars for very small
Chrome windows on Linux and Windows.

BUG=617733
TEST=
1. Enable the Password Separated Sign In Flow in chrome://flags
2. Resize the Chrome window to be smaller than 600 pixels in height
3. In the User Menu, click Sign In to Chrome
4. The Sign In flow content and the Sync Confirmation dialog should
   both display scrollbars allowing interaction with the content.
5. In large windows, no scroll bar should be seen.

Committed: https://crrev.com/66d94fd2fee2dd2a080c9bb5d29bec349c71dfa2
Cr-Commit-Position: refs/heads/master@{#400504}
==========

[commit-bot: I haz the power, 2016-06-17 21:11:50]

Patchset 2 (id:??) landed as
https://crrev.com/66d94fd2fee2dd2a080c9bb5d29bec349c71dfa2
Cr-Commit-Position: refs/heads/master@{#400504}