arc: Implement scrollable OptIn window content.

arc: Implement scrollable OptIn window content.

Support low resolutions by making OptIn content scrollable in case
Chrome desktop resolution is lower than required for normal OptIn
display. This also implements automatic window bounds update on
desktop resoluton change.

BUG=b/29817065
TEST=Manually on devices.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/f8e016bfad2544663aab1bdcccd652477cbfbfe2
Cr-Commit-Position: refs/heads/master@{#404947}

[khmel, 2016-07-12 21:08:45]

Description was changed from

==========
arc: Implement scrollable OptIn window content.

Support low resolutions by making OptIn content scrollable in case
Chrome desktop resolution is lower than required for normal OptIn
display. This also implements automatic window bounds update on
desktop resoluton change.

BUG=b/29817065
TEST=Manually on devices.
==========

to

==========
arc: Implement scrollable OptIn window content.

Support low resolutions by making OptIn content scrollable in case
Chrome desktop resolution is lower than required for normal OptIn
display. This also implements automatic window bounds update on
desktop resoluton change.

BUG=b/29817065
TEST=Manually on devices.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[khmel, 2016-07-12 21:15:37]

khmel@chromium.org changed reviewers:
+ xiyuan@chromium.org

[khmel, 2016-07-12 21:15:38]

Hi Xiyuan,

PTAL

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/arc_support/background.js (right):

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/background.js:363: css: { files:
['lso.css'] },
To make scroll bars in one style.

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/background.js:398:
lsoView.style.width = '100%';
Again workaround for webview 'animation'. Fixed width does not work well when
screen size goes below default layout.

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/background.js:517:
setWindowBounds();
Chrome keeps window previous position. We need reset bounds if current screen
resolutions allows us to do.

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/arc_support/main.css (right):

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/main.css:35: overflow: hidden;
Could not get rid of additional scrollbars even current layout assumes no
scrolling context. Probably yet another surprise from webview.

[xiyuan, 2016-07-12 21:50:22]

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/arc_support/background.js (right):

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/background.js:72: var innerWidth =
960;
Looks like |innerWidth| and |innerHeight| are constants. We should make them
looks like a JS const,

i.e.

/**
 * ...
 * @const {number}
 */
var INNER_WIDTH = 960;

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/background.js:273:
console.log(lsoView.src);
Can we remove this?

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/background.js:331: var
decorationWidht = appWindow.outerBounds.width -
decorationWidht -> decorationWidth

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/background.js:363: css: { files:
['lso.css'] },
On 2016/07/12 21:15:37, khmel wrote:
> To make scroll bars in one style.

Acknowledged.

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/background.js:398:
lsoView.style.width = '100%';
On 2016/07/12 21:15:37, khmel wrote:
> Again workaround for webview 'animation'. Fixed width does not work well when
> screen size goes below default layout.

Acknowledged.

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/background.js:517:
setWindowBounds();
On 2016/07/12 21:15:37, khmel wrote:
> Chrome keeps window previous position. We need reset bounds if current screen
> resolutions allows us to do.

Acknowledged.

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/arc_support/main.css (right):

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/main.css:35: overflow: hidden;
On 2016/07/12 21:15:37, khmel wrote:
> Could not get rid of additional scrollbars even current layout assumes no
> scrolling context. Probably yet another surprise from webview.

Acknowledged.

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/main.css:55: height: 388px;
388px because default height 688px - image header height 300px ? Maybe add a
comment of how we get this number.

[khmel, 2016-07-12 22:05:10]

Thanks Xiyuan for review.

PTAL

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/arc_support/background.js (right):

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/background.js:72: var innerWidth =
960;
On 2016/07/12 21:50:21, xiyuan wrote:
> Looks like |innerWidth| and |innerHeight| are constants. We should make them
> looks like a JS const,
> 
> i.e.
> 
> /**
>  * ...
>  * @const {number}
>  */
> var INNER_WIDTH = 960;

Good point, thanks.

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/background.js:273:
console.log(lsoView.src);
On 2016/07/12 21:50:21, xiyuan wrote:
> Can we remove this?

Yep, forgot to upload this.
DONE

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/background.js:331: var
decorationWidht = appWindow.outerBounds.width -
On 2016/07/12 21:50:21, xiyuan wrote:
> decorationWidht -> decorationWidth

Done.

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/arc_support/main.css (right):

https://codereview.chromium.org/2142933006/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/arc_support/main.css:55: height: 388px;
On 2016/07/12 21:50:21, xiyuan wrote:
> 388px because default height 688px - image header height 300px ? Maybe add a
> comment of how we get this number.

Done.

[xiyuan, 2016-07-12 22:06:26]

lgtm

[khmel, 2016-07-12 22:09:23]

The CQ bit was checked by khmel@chromium.org

[commit-bot: I haz the power, 2016-07-12 22:11:10]

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-07-13 03:16:57]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-13 03:17:35]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-13 03:18:46]

Description was changed from

==========
arc: Implement scrollable OptIn window content.

Support low resolutions by making OptIn content scrollable in case
Chrome desktop resolution is lower than required for normal OptIn
display. This also implements automatic window bounds update on
desktop resoluton change.

BUG=b/29817065
TEST=Manually on devices.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
arc: Implement scrollable OptIn window content.

Support low resolutions by making OptIn content scrollable in case
Chrome desktop resolution is lower than required for normal OptIn
display. This also implements automatic window bounds update on
desktop resoluton change.

BUG=b/29817065
TEST=Manually on devices.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/f8e016bfad2544663aab1bdcccd652477cbfbfe2
Cr-Commit-Position: refs/heads/master@{#404947}
==========

[commit-bot: I haz the power, 2016-07-13 03:18:47]

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