Fix sizing issues in the tab modal signin flow.

Fix sizing issues in the tab modal signin flow.

This change increases the height of the sign in portion of the flow to
be tall enough to fit the content without a scrollbar. Longer content
doesn't break the flow but makes the scrollbar appear.

It also fixes the size of the details text in the sync confirmation
portion to a pixel value to prevent it from being affected by content
font size settings.

BUG=593499, 591131, 589656
Committed: https://crrev.com/0ac524ae54c5fc198593f34aa242ac504508f023
Cr-Commit-Position: refs/heads/master@{#385225}

[anthonyvd, 2016-03-16 21:04:10]

anthonyvd@chromium.org changed reviewers:
+ dbeam@chromium.org, rsesek@chromium.org, sky@chromium.org

[anthonyvd, 2016-03-16 21:04:11]

Hi everyone, can you please take a look at these small tweaks to the tab-modal
signin flow? Those are the result of some feedback we got from UI review.

Thanks!

rsesek@chromium.org: c/b/ui/cocoa
dbeam@chromium.org: c/b/resources
sky@chromium.org: c/b/ui/views

[Robert Sesek, 2016-03-16 21:19:50]

cocoa/ LGTM

[sky, 2016-03-16 21:56:00]

views LGTM

[Dan Beam, 2016-03-17 01:49:29]

i think this is just kicking the can down the road.

i think you should message document.body.scrollHeight to the browser via
chrome.send() once when the content is done/ready and change the native view's
preferredheight.

https://codereview.chromium.org/1814513002/diff/1/chrome/browser/resources/sy...
File chrome/browser/resources/sync_confirmation/sync_confirmation.css (right):

https://codereview.chromium.org/1814513002/diff/1/chrome/browser/resources/sy...
chrome/browser/resources/sync_confirmation/sync_confirmation.css:39: font-size:
13.5px;
wat

[anthonyvd, 2016-03-17 16:33:31]

There are multiple things that converge into making this change this specific
way:

1- The content size changes dynamically in the sign in flow and it doesn't
stretch its container. In this case, the issue isn't that we don't know the
height, it's that we would need a way to know when the extra elements are added
inside the embedded webview and resize the dialog at that point in time, which I
don't think is possible.

2- Changing the size of the webview with SetPreferredSize or changing the size
of the widget returned by constrained_window::ShowWebModalDialogViews after
their creation don't seem to have any effect. In any case, point 1 would prevent
us from knowing when and how the resize should happen.

3- These values are the result of manual testing in several languages, with and
without the extra elements that can appear. The design decision is that it's not
bad if there's a scrollbar sometimes, as long as it's not present in the
majority of cases, which this height achieves.

I totally agree that this fixed sizing isn't ideal. Unfortunately, I'm not aware
of any way of doing this that would allow content to gracefully resize itself
while it's displayed in a tab-modal dialog like this.

https://codereview.chromium.org/1814513002/diff/1/chrome/browser/resources/sy...
File chrome/browser/resources/sync_confirmation/sync_confirmation.css (right):

https://codereview.chromium.org/1814513002/diff/1/chrome/browser/resources/sy...
chrome/browser/resources/sync_confirmation/sync_confirmation.css:39: font-size:
13.5px;
On 2016/03/17 01:49:29, Dan Beam wrote:
> wat

Hrm, looks like I got confused. We actually want 13px.

Fixed.

[Dan Beam, 2016-03-17 22:37:26]

https://codereview.chromium.org/1814513002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/sync_confirmation/sync_confirmation.css (right):

https://codereview.chromium.org/1814513002/diff/20001/chrome/browser/resource...
chrome/browser/resources/sync_confirmation/sync_confirmation.css:39: font-size:
13px;
how does this differ from the effective font-size set by text_defaults_md.css
(81.25% of 16px is 13px)[1][2]?

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/ui/webui/resources...

[Dan Beam, 2016-03-17 22:40:01]

On 2016/03/17 16:33:31, anthonyvd wrote:
> There are multiple things that converge into making this change this specific
> way:
> 
> 1- The content size changes dynamically in the sign in flow and it doesn't
> stretch its container. In this case, the issue isn't that we don't know the
> height, it's that we would need a way to know when the extra elements are
added
> inside the embedded webview and resize the dialog at that point in time, which
I
> don't think is possible.

does the webview have resize events?  could we poll the size?

> 
> 2- Changing the size of the webview with SetPreferredSize or changing the size
> of the widget returned by constrained_window::ShowWebModalDialogViews after
> their creation don't seem to have any effect. In any case, point 1 would
prevent
> us from knowing when and how the resize should happen.

Did you call Layout()?

> 
> 3- These values are the result of manual testing in several languages, with
and
> without the extra elements that can appear. The design decision is that it's
not
> bad if there's a scrollbar sometimes, as long as it's not present in the
> majority of cases, which this height achieves.

can you explain "majority of cases"?  I hope you don't mean this:
http://blog.muipr.com/wp-content/uploads/2015/05/american-globe.jpg

[anthonyvd, 2016-03-18 17:54:30]

On 2016/03/17 22:40:01, Dan Beam wrote:
> On 2016/03/17 16:33:31, anthonyvd wrote:
> > There are multiple things that converge into making this change this
specific
> > way:
> > 
> > 1- The content size changes dynamically in the sign in flow and it doesn't
> > stretch its container. In this case, the issue isn't that we don't know the
> > height, it's that we would need a way to know when the extra elements are
> added
> > inside the embedded webview and resize the dialog at that point in time,
which
> I
> > don't think is possible.
> 
> does the webview have resize events?  could we poll the size?

Unfortunately, no. The webview itself doesn't resize, some of the content
becomes scrollable.

> 
> > 
> > 2- Changing the size of the webview with SetPreferredSize or changing the
size
> > of the widget returned by constrained_window::ShowWebModalDialogViews after
> > their creation don't seem to have any effect. In any case, point 1 would
> prevent
> > us from knowing when and how the resize should happen.
> 
> Did you call Layout()?

Ha! That's what I was missing. I used this to get the Sync Confirmation to
auto-resize to better handle different languages. Unfortunately I don't think I
can do the same for the sign in part of the flow.

> 
> > 
> > 3- These values are the result of manual testing in several languages, with
> and
> > without the extra elements that can appear. The design decision is that it's
> not
> > bad if there's a scrollbar sometimes, as long as it's not present in the
> > majority of cases, which this height achieves.
> 
> can you explain "majority of cases"?  I hope you don't mean this:
> http://blog.muipr.com/wp-content/uploads/2015/05/american-globe.jpg

Majority of cases means regular + error messages in most languages. It's not a
super clear cut definition, but German is supposed to be the longest language in
this dialog so I tested with that as well as French, English, Arabic, Russian,
and Japanese. In the longest case (German) there's still enough free space to
allow for an extra line at least, even with 2 error messages displayed at once,
which we consider a corner case.

Being a French-speaking Canadian myself, I totally understand what you're
talking about with that picture ;)

[anthonyvd, 2016-03-18 17:59:45]

Feedback addressed.

rsesek@, sky@, I made some bigger changes to the sync confirmation code to make
it size to its content. Can you please take another look?

Thanks everyone!

[anthonyvd, 2016-03-18 18:01:49]

https://codereview.chromium.org/1814513002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/sync_confirmation/sync_confirmation.css (right):

https://codereview.chromium.org/1814513002/diff/20001/chrome/browser/resource...
chrome/browser/resources/sync_confirmation/sync_confirmation.css:39: font-size:
13px;
On 2016/03/17 22:37:26, Dan Beam wrote:
> how does this differ from the effective font-size set by text_defaults_md.css
> (81.25% of 16px is 13px)[1][2]?
> 
> [1]
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> [2]
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/webui/resources...

This way, the font size doesn't change when Content Font Size is changed in
chrome://settings, which is what we want since this isn't "content".

[Dan Beam, 2016-03-18 18:10:08]

https://codereview.chromium.org/1814513002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/sync_confirmation/sync_confirmation.css (right):

https://codereview.chromium.org/1814513002/diff/20001/chrome/browser/resource...
chrome/browser/resources/sync_confirmation/sync_confirmation.css:39: font-size:
13px;
On 2016/03/18 18:01:48, anthonyvd wrote:
> On 2016/03/17 22:37:26, Dan Beam wrote:
> > how does this differ from the effective font-size set by
text_defaults_md.css
> > (81.25% of 16px is 13px)[1][2]?
> > 
> > [1]
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> > [2]
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/webui/resources...
> 
> This way, the font size doesn't change when Content Font Size is changed in
> chrome://settings, which is what we want since this isn't "content".

a) I'm not sure I agree with "this isn't content"
b) most people's eyes work poorly anywhere on the screen

https://codereview.chromium.org/1814513002/diff/80001/chrome/browser/ui/signi...
File chrome/browser/ui/signin_view_controller_delegate.cc (right):

https://codereview.chromium.org/1814513002/diff/80001/chrome/browser/ui/signi...
chrome/browser/ui/signin_view_controller_delegate.cc:28:
web_contents_->GetWebUI()->RegisterMessageCallback(
hmmm, this is kind of weird.  I wonder why RegisterMessageCallback() is even
public.  this should probably be done from the webui or its handlers.

https://codereview.chromium.org/1814513002/diff/80001/chrome/browser/ui/signi...
chrome/browser/ui/signin_view_controller_delegate.cc:81: if (args->GetInteger(0,
&height)) {
double height;
CHECK(args->GetInteger(0, &height));
ResizeNativeView(static_cast<int>(height));

or

double height;
bool success = args->GetInteger(0, &height);
DCHECK(success);
ResizeNativeView(static_cast<int>(height));

[anthonyvd, 2016-03-18 18:53:31]

On 2016/03/18 18:10:08, Dan Beam wrote:
>
https://codereview.chromium.org/1814513002/diff/20001/chrome/browser/resource...
> File chrome/browser/resources/sync_confirmation/sync_confirmation.css (right):
> 
>
https://codereview.chromium.org/1814513002/diff/20001/chrome/browser/resource...
> chrome/browser/resources/sync_confirmation/sync_confirmation.css:39:
font-size:
> 13px;
> On 2016/03/18 18:01:48, anthonyvd wrote:
> > On 2016/03/17 22:37:26, Dan Beam wrote:
> > > how does this differ from the effective font-size set by
> text_defaults_md.css
> > > (81.25% of 16px is 13px)[1][2]?
> > > 
> > > [1]
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> > > [2]
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/webui/resources...
> > 
> > This way, the font size doesn't change when Content Font Size is changed in
> > chrome://settings, which is what we want since this isn't "content".
> 
> a) I'm not sure I agree with "this isn't content"
> b) most people's eyes work poorly anywhere on the screen

It isn't content in the sense that, even though it's implemented in HTML, it's a
piece of Chrome UI which we do not scale with that setting. This is the
consistent solution imo. We also got that feedback from UI review when we showed
them this piece of UI: the consensus there was that this shouldn't scale with
content font size.

> 
>
https://codereview.chromium.org/1814513002/diff/80001/chrome/browser/ui/signi...
> File chrome/browser/ui/signin_view_controller_delegate.cc (right):
> 
>
https://codereview.chromium.org/1814513002/diff/80001/chrome/browser/ui/signi...
> chrome/browser/ui/signin_view_controller_delegate.cc:28:
> web_contents_->GetWebUI()->RegisterMessageCallback(
> hmmm, this is kind of weird.  I wonder why RegisterMessageCallback() is even
> public.  this should probably be done from the webui or its handlers.
> 
>
https://codereview.chromium.org/1814513002/diff/80001/chrome/browser/ui/signi...
> chrome/browser/ui/signin_view_controller_delegate.cc:81: if
(args->GetInteger(0,
> &height)) {
> double height;
> CHECK(args->GetInteger(0, &height));
> ResizeNativeView(static_cast<int>(height));
> 
> or
> 
> double height;
> bool success = args->GetInteger(0, &height);
> DCHECK(success);
> ResizeNativeView(static_cast<int>(height));

[anthonyvd, 2016-03-18 18:53:40]

https://codereview.chromium.org/1814513002/diff/80001/chrome/browser/ui/signi...
File chrome/browser/ui/signin_view_controller_delegate.cc (right):

https://codereview.chromium.org/1814513002/diff/80001/chrome/browser/ui/signi...
chrome/browser/ui/signin_view_controller_delegate.cc:28:
web_contents_->GetWebUI()->RegisterMessageCallback(
On 2016/03/18 18:10:08, Dan Beam wrote:
> hmmm, this is kind of weird.  I wonder why RegisterMessageCallback() is even
> public.  this should probably be done from the webui or its handlers.

Is there a canonical way to get this information from the handlers back to here,
where I can control the native views/windows?

https://codereview.chromium.org/1814513002/diff/80001/chrome/browser/ui/signi...
chrome/browser/ui/signin_view_controller_delegate.cc:81: if (args->GetInteger(0,
&height)) {
On 2016/03/18 18:10:08, Dan Beam wrote:
> double height;
> CHECK(args->GetInteger(0, &height));
> ResizeNativeView(static_cast<int>(height));
> 
> or
> 
> double height;
> bool success = args->GetInteger(0, &height);
> DCHECK(success);
> ResizeNativeView(static_cast<int>(height));

Done.

[Dan Beam, 2016-03-18 21:07:47]

https://codereview.chromium.org/1814513002/diff/80001/chrome/browser/ui/signi...
File chrome/browser/ui/signin_view_controller_delegate.cc (right):

https://codereview.chromium.org/1814513002/diff/80001/chrome/browser/ui/signi...
chrome/browser/ui/signin_view_controller_delegate.cc:28:
web_contents_->GetWebUI()->RegisterMessageCallback(
On 2016/03/18 18:53:40, anthonyvd wrote:
> On 2016/03/18 18:10:08, Dan Beam wrote:
> > hmmm, this is kind of weird.  I wonder why RegisterMessageCallback() is even
> > public.  this should probably be done from the webui or its handlers.
> 
> Is there a canonical way to get this information from the handlers back to
here,
> where I can control the native views/windows?

overall: I think it's just more common to handle messages... in "message
handlers", especially when one already exists for this webui:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

re: your question of "how does the handler access the delegate?"

if we either followed this precedent:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

  SigninViewController* ResizeSigninNativeView(int height) {
    signin_view_controller_.ResizeNativeView(height);
    // or signin_view_controller->delegate()->ResizeNativeView(height);
  }

or instead of mirroring the API, exposed a mutable pointer to the controller:

  SigninViewController* sign_in_view_controller() {
    return &sign_in_view_controller_;
  }

and did the same thing in the controller:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

either by creating a passthrough to that specific methods (copying API):

  void SigninViewController::ResizeNativeView(int height) {
    signin_view_controller_delegate_->ResizeNativeView(height);
  }

or by exposing the delegate:

  SigninViewControllerDelegate* signin_view_controller_delegate() {
    return signin_view_controller_delegate_;
  }

the handler could would look like:

  void SyncConfirmationHandler::HandleResizeNativeView(
      const base::ListValue* args) {
    double height_arg;
    bool success = args->GetDouble(0, &height_arg);
    DCHECK(success);

    Browser* browser = GetDesktopBrowser();
    int height = static_cast<int>(height_arg);

    // any of these
    browser->ResizeSigninNativeView(height);
    browser->sign_in_view_controller()->ResizeNativeView(height);
    browser->sign_in_view_controller()->delegate()->ResizeNativeView(height);
}

[anthonyvd, 2016-04-04 14:15:56]

Hi everyone. I'm back from vacation so I addressed feedback. 

There have been significant changes since sky@ and rsesek@ l-g-t-m'd this so
it'd be awesome if you guys could have another quick look.

Thanks!

https://codereview.chromium.org/1814513002/diff/80001/chrome/browser/ui/signi...
File chrome/browser/ui/signin_view_controller_delegate.cc (right):

https://codereview.chromium.org/1814513002/diff/80001/chrome/browser/ui/signi...
chrome/browser/ui/signin_view_controller_delegate.cc:28:
web_contents_->GetWebUI()->RegisterMessageCallback(
On 2016/03/18 21:07:47, Dan Beam wrote:
> On 2016/03/18 18:53:40, anthonyvd wrote:
> > On 2016/03/18 18:10:08, Dan Beam wrote:
> > > hmmm, this is kind of weird.  I wonder why RegisterMessageCallback() is
even
> > > public.  this should probably be done from the webui or its handlers.
> > 
> > Is there a canonical way to get this information from the handlers back to
> here,
> > where I can control the native views/windows?
> 
> overall: I think it's just more common to handle messages... in "message
> handlers", especially when one already exists for this webui:
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...
> 
> re: your question of "how does the handler access the delegate?"
> 
> if we either followed this precedent:
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...
> 
>   SigninViewController* ResizeSigninNativeView(int height) {
>     signin_view_controller_.ResizeNativeView(height);
>     // or signin_view_controller->delegate()->ResizeNativeView(height);
>   }
> 
> or instead of mirroring the API, exposed a mutable pointer to the controller:
> 
>   SigninViewController* sign_in_view_controller() {
>     return &sign_in_view_controller_;
>   }
> 
> and did the same thing in the controller:
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...
> 
> either by creating a passthrough to that specific methods (copying API):
> 
>   void SigninViewController::ResizeNativeView(int height) {
>     signin_view_controller_delegate_->ResizeNativeView(height);
>   }
> 
> or by exposing the delegate:
> 
>   SigninViewControllerDelegate* signin_view_controller_delegate() {
>     return signin_view_controller_delegate_;
>   }
> 
> the handler could would look like:
> 
>   void SyncConfirmationHandler::HandleResizeNativeView(
>       const base::ListValue* args) {
>     double height_arg;
>     bool success = args->GetDouble(0, &height_arg);
>     DCHECK(success);
> 
>     Browser* browser = GetDesktopBrowser();
>     int height = static_cast<int>(height_arg);
> 
>     // any of these
>     browser->ResizeSigninNativeView(height);
>     browser->sign_in_view_controller()->ResizeNativeView(height);
>     browser->sign_in_view_controller()->delegate()->ResizeNativeView(height);
> }

Thanks for the detailed input. I wasn't too sure what was preferred between
having handlers in a non-webui-handler class vs exposing browser member
variables.

I went with exposing the view controller and its delegate. Let me know how this
looks!

[Robert Sesek, 2016-04-04 14:54:34]

https://codereview.chromium.org/1814513002/diff/140001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.h
(right):

https://codereview.chromium.org/1814513002/diff/140001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.h:75: bool
wait_for_size_;
Can you document what these variables are? It's not immediately clear what
wait_for_size_ means and how it affects the other state in this class.

Some of that commentary should also move up to the ctor comment or class comment
so users of this class know what to pass for wait_for_size.

[sky, 2016-04-04 15:04:02]

https://codereview.chromium.org/1814513002/diff/140001/chrome/browser/ui/sign...
File chrome/browser/ui/signin_view_controller_delegate.h (right):

https://codereview.chromium.org/1814513002/diff/140001/chrome/browser/ui/sign...
chrome/browser/ui/signin_view_controller_delegate.h:45: virtual void
ResizeNativeView(int height) = 0;
Document units of height.

https://codereview.chromium.org/1814513002/diff/140001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.cc
(right):

https://codereview.chromium.org/1814513002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.cc:85:
DisplayModal();
How do you know browser_ is valid by the time you got here? Active web contents
may differ from what was active at the time the
SigninViewControllerDelegateViews was created. Do you care?

https://codereview.chromium.org/1814513002/diff/140001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.h
(right):

https://codereview.chromium.org/1814513002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.h:39:
bool wait_for_size);
Document what wait_For_size means.

[anthonyvd, 2016-04-04 20:35:24]

https://codereview.chromium.org/1814513002/diff/140001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.h
(right):

https://codereview.chromium.org/1814513002/diff/140001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.h:75: bool
wait_for_size_;
On 2016/04/04 14:54:34, Robert Sesek wrote:
> Can you document what these variables are? It's not immediately clear what
> wait_for_size_ means and how it affects the other state in this class.
> 
> Some of that commentary should also move up to the ctor comment or class
comment
> so users of this class know what to pass for wait_for_size.

Done.

https://codereview.chromium.org/1814513002/diff/140001/chrome/browser/ui/sign...
File chrome/browser/ui/signin_view_controller_delegate.h (right):

https://codereview.chromium.org/1814513002/diff/140001/chrome/browser/ui/sign...
chrome/browser/ui/signin_view_controller_delegate.h:45: virtual void
ResizeNativeView(int height) = 0;
On 2016/04/04 15:04:02, sky wrote:
> Document units of height.

Done now on the base class.

https://codereview.chromium.org/1814513002/diff/140001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.cc
(right):

https://codereview.chromium.org/1814513002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.cc:85:
DisplayModal();
On 2016/04/04 15:04:02, sky wrote:
> How do you know browser_ is valid by the time you got here? Active web
contents
> may differ from what was active at the time the
> SigninViewControllerDelegateViews was created. Do you care?

Since this is owned by SigninViewController, which is itself owned by Browser, I
assume that browser_ has to be valid for this object to still exist. Does that
make sense?

The active web contents differing between this and creation is fine. We want the
window to hang from the web contents that is active at the moment it's
displayed.

https://codereview.chromium.org/1814513002/diff/140001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.h
(right):

https://codereview.chromium.org/1814513002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.h:39:
bool wait_for_size);
On 2016/04/04 15:04:02, sky wrote:
> Document what wait_For_size means.

Done.

[Robert Sesek, 2016-04-04 21:45:13]

lgtm

[sky, 2016-04-04 22:48:44]

LGTM

[Dan Beam, 2016-04-05 06:42:41]

lgtm

https://codereview.chromium.org/1814513002/diff/160001/chrome/browser/resourc...
File chrome/browser/resources/sync_confirmation/sync_confirmation.js (right):

https://codereview.chromium.org/1814513002/diff/160001/chrome/browser/resourc...
chrome/browser/resources/sync_confirmation/sync_confirmation.js:26:
chrome.send('resizeNativeView', [document.body.scrollHeight]);
nit: why do you need both 'initialized' and 'resizeNativeView' as the code
currently is?  could you do 'initializeWithHeight'?

[anthonyvd, 2016-04-05 14:24:48]

https://codereview.chromium.org/1814513002/diff/160001/chrome/browser/resourc...
File chrome/browser/resources/sync_confirmation/sync_confirmation.js (right):

https://codereview.chromium.org/1814513002/diff/160001/chrome/browser/resourc...
chrome/browser/resources/sync_confirmation/sync_confirmation.js:26:
chrome.send('resizeNativeView', [document.body.scrollHeight]);
On 2016/04/05 06:42:41, Dan Beam wrote:
> nit: why do you need both 'initialized' and 'resizeNativeView' as the code
> currently is?  could you do 'initializeWithHeight'?

Good point, they don't need to be separate. Done.

[anthonyvd, 2016-04-05 18:11:01]

The CQ bit was checked by anthonyvd@chromium.org

[anthonyvd, 2016-04-05 18:11:02]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org,
dbeam@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1814513002/#ps200001
(title: "Fix unit tests")

[commit-bot: I haz the power, 2016-04-05 18:11:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1814513002/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1814513002/200001

[commit-bot: I haz the power, 2016-04-05 18:18:01]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-04-05 18:19:13]

Description was changed from

==========
Fix sizing issues in the tab modal signin flow.

This change increases the height of the sign in portion of the flow to
be tall enough to fit the content without a scrollbar. Longer content
doesn't break the flow but makes the scrollbar appear.

It also fixes the size of the details text in the sync confirmation
portion to a pixel value to prevent it from being affected by content
font size settings.

BUG=593499,591131,589656
==========

to

==========
Fix sizing issues in the tab modal signin flow.

This change increases the height of the sign in portion of the flow to
be tall enough to fit the content without a scrollbar. Longer content
doesn't break the flow but makes the scrollbar appear.

It also fixes the size of the details text in the sync confirmation
portion to a pixel value to prevent it from being affected by content
font size settings.

BUG=593499,591131,589656

Committed: https://crrev.com/0ac524ae54c5fc198593f34aa242ac504508f023
Cr-Commit-Position: refs/heads/master@{#385225}
==========

[commit-bot: I haz the power, 2016-04-05 18:19:14]

Patchset 11 (id:??) landed as
https://crrev.com/0ac524ae54c5fc198593f34aa242ac504508f023
Cr-Commit-Position: refs/heads/master@{#385225}