Decouple VR Shell DPR and CSS size from Physical Displays.

Decouple VR Shell DPR and CSS size from Physical Displays.

This CL introduces two virtual displays for VR Shell, one for the content window, and one for the background UI window (which is made as small as possible to save on memory).

The UI now dynamically controls its size, and the content defaults to a reasonable size regular web browsing (in VR).

BUG=643480
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/4cc2259e5139ce40daa4230b25a4ee7183c23d1b
Cr-Commit-Position: refs/heads/master@{#435702}

[mthiesse, 2016-10-20 19:53:41]

mthiesse@chromium.org changed reviewers:
+ bshe@chromium.org, dtrainor@chromium.org, oshima@chromium.org

[mthiesse, 2016-10-20 19:53:42]

dtrainor@chromium.org: Please review changes in chrome/android

oshima@chromium.org: Please review changes in ui/display/

bshe@chromium.org: Please review changes in chrome/browser/android/vr_shell/

[mthiesse, 2016-10-20 21:55:48]

Patchset #2 (id:20001) has been deleted

[mthiesse, 2016-10-20 22:01:37]

https://codereview.chromium.org/2428383006/diff/1/ui/display/android/screen_a...
File ui/display/android/screen_android.cc (right):

https://codereview.chromium.org/2428383006/diff/1/ui/display/android/screen_a...
ui/display/android/screen_android.cc:29: if (vr_display_map_.size() > 0) {
Whoops meant to revert some changes in this file that were for some debugging.
Will re-upload soon.

[mthiesse, 2016-10-20 23:28:52]

mthiesse@chromium.org changed reviewers:
+ boliu@chromium.org

[mthiesse, 2016-10-20 23:28:52]

Alright, PTAL.

Sorry about that, it turns out the debug statements I meant to remove were load
bearing, so I've added some more changes.

+boliu, please review content/browser changes.

[boliu, 2016-10-20 23:38:44]

screen_android change isn't going in the direction intended by crbug.com/625089.
I want native screen_android things should be updated directly by java
DisplayAndroid[Manager], and [Shared]DeviceDisplayInfo needs to be nuked from
orbit.

then for this CL, we can provide a java API to construct a DisplayAndroid for
VR, and native side should not need any special handling

[boliu, 2016-10-20 23:40:50]

On 2016/10/20 23:38:44, boliu wrote:
> screen_android change isn't going in the direction intended by
crbug.com/625089.
> I want native screen_android things should be updated directly by java
> DisplayAndroid[Manager], and [Shared]DeviceDisplayInfo needs to be nuked from
> orbit.

maybe I'll have time to work on that next week.. but if you are comfortable
doing that refactor, you are welcome to :) but please let me know if you do

> 
> then for this CL, we can provide a java API to construct a DisplayAndroid for
> VR, and native side should not need any special handling

[oshima, 2016-10-24 20:01:10]

https://codereview.chromium.org/2428383006/diff/40001/ui/display/android/scre...
File ui/display/android/screen_android.cc (right):

https://codereview.chromium.org/2428383006/diff/40001/ui/display/android/scre...
ui/display/android/screen_android.cc:56: display_map_[view] = display;
nit: emplace(view, display)

https://codereview.chromium.org/2428383006/diff/40001/ui/display/android/scre...
ui/display/android/screen_android.cc:71: }
nit:

if (cond)
 return GetPrimaryDislpay();
return iter-second;

is more common in chromium.

https://codereview.chromium.org/2428383006/diff/40001/ui/display/android/scre...
File ui/display/android/screen_android.h (right):

https://codereview.chromium.org/2428383006/diff/40001/ui/display/android/scre...
ui/display/android/screen_android.h:23: 
nit:  // Screen:

Please group overridden methods in one block as other implementations do.

https://codereview.chromium.org/2428383006/diff/40001/ui/display/android/scre...
ui/display/android/screen_android.h:52: std::map<gfx::NativeView, Display>
display_map_;
optional: unordered_map

[David Trainor- moved to gerrit, 2016-10-26 00:47:51]

I'm ok with the changes to chrome/android.  I've been waiting for the discussion
on the other files to die down.  But chrome/android lgtm since I'll be out for
the rest of the week.

[mthiesse, 2016-11-24 18:36:11]

Description was changed from

==========
Decouple VR Shell DPR and CSS size from Android Views.

BUG=643480
==========

to

==========
Decouple VR Shell DPR and CSS size from Android Views.

BUG=643480
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[mthiesse, 2016-11-24 18:37:02]

Patchset #3 (id:60001) has been deleted

[mthiesse, 2016-11-29 20:03:31]

mthiesse@chromium.org changed reviewers:
+ bajones@chromium.org, tedchoc@chromium.org
- oshima@chromium.org

[mthiesse, 2016-11-29 20:03:32]

Sorry for the delay, blocking refactors have been completed. PTAL.

-oshima who's no longer needed, no offense :P

tedchoc@chromium.org: Please review changes in ui/snapshot
bajones@chromium.org: Please review incredibly hacky changes in
third_party/WebKit/Source/modules/vr/. Also please hurry up and land the new
compositing path ;)
bshe@chromium.org: Please review */vr_shell/ changes.

cc cjgrant for opinions on js/css changes.

[mthiesse, 2016-11-29 20:07:37]

Description was changed from

==========
Decouple VR Shell DPR and CSS size from Android Views.

BUG=643480
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Decouple VR Shell DPR and CSS size from Android Views.

This CL introduces two virtual displays for VR Shell, one for the content
window, and one for the background UI window (which is made as small as possible
to save on memory).

The UI now dynamically controls its size, and the content defaults to a
reasonable size regular web browsing (in VR).

BUG=643480
==========

[mthiesse, 2016-11-29 20:07:55]

Description was changed from

==========
Decouple VR Shell DPR and CSS size from Android Views.

This CL introduces two virtual displays for VR Shell, one for the content
window, and one for the background UI window (which is made as small as possible
to save on memory).

The UI now dynamically controls its size, and the content defaults to a
reasonable size regular web browsing (in VR).

BUG=643480
==========

to

==========
Decouple VR Shell DPR and CSS size from Physical Displays.

This CL introduces two virtual displays for VR Shell, one for the content
window, and one for the background UI window (which is made as small as possible
to save on memory).

The UI now dynamically controls its size, and the content defaults to a
reasonable size regular web browsing (in VR).

BUG=643480
==========

[cjgrant, 2016-11-29 20:28:05]

cjgrant@chromium.org changed reviewers:
+ cjgrant@chromium.org

[cjgrant, 2016-11-29 20:28:07]

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/android...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/android...
chrome/browser/android/vr_shell/vr_shell.cc:1025: void
VrShell::SetContentCssSize(float width, float height, float dpr) {
Just a suggestion for consideration...  What about:

enum Contents {
  MAIN_CONTENTS,
  UI_CONTENTS
};
VrShell::SetCssSize(Contents contents, width, ...)
VrShell::UiBoundsChanged(Contents contents, JNI ...)

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/vr_shell/vr_shell_ui.css (left):

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/resourc...
chrome/browser/resources/vr_shell/vr_shell_ui.css:10: --scaleFactor: 4;
Generally, updating the UI could be a separate change that happens after the
decoupling lands, right?

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/vr_shell/vr_shell_ui_api.js (right):

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/resourc...
chrome/browser/resources/vr_shell/vr_shell_ui_api.js:104: * Triggers an Action.
This needs to be updated.

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc (right):

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/ui/webu...
chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc:81:
CHECK(args->GetDouble(0, &width));
Might be clearer to pass a dictionary of 'width', 'height', 'dpr', in the first
list item, and drop the length check.  Just a thought.

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/ui/webu...
chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc:85:
vr_shell_->SetUiCssSize(width, height, dpr);
Does this not need to be synchronized on the rendering thread?

[mthiesse, 2016-11-29 21:06:33]

Description was changed from

==========
Decouple VR Shell DPR and CSS size from Physical Displays.

This CL introduces two virtual displays for VR Shell, one for the content
window, and one for the background UI window (which is made as small as possible
to save on memory).

The UI now dynamically controls its size, and the content defaults to a
reasonable size regular web browsing (in VR).

BUG=643480
==========

to

==========
Decouple VR Shell DPR and CSS size from Physical Displays.

This CL introduces two virtual displays for VR Shell, one for the content
window, and one for the background UI window (which is made as small as possible
to save on memory).

The UI now dynamically controls its size, and the content defaults to a
reasonable size regular web browsing (in VR).

BUG=643480
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[mthiesse, 2016-11-29 21:06:54]

Patchset #5 (id:120001) has been deleted

[mthiesse, 2016-11-29 21:07:01]

Patchset #4 (id:100001) has been deleted

[mthiesse, 2016-11-29 21:07:09]

Patchset #3 (id:80001) has been deleted

[mthiesse, 2016-11-29 21:07:22]

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/android...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/android...
chrome/browser/android/vr_shell/vr_shell.cc:1025: void
VrShell::SetContentCssSize(float width, float height, float dpr) {
On 2016/11/29 20:28:06, cjgrant wrote:
> Just a suggestion for consideration...  What about:
> 
> enum Contents {
>   MAIN_CONTENTS,
>   UI_CONTENTS
> };
> VrShell::SetCssSize(Contents contents, width, ...)
> VrShell::UiBoundsChanged(Contents contents, JNI ...)
> 

Mild preference for keeping as-is. Sharing enums between java/c++ is messy, and
for the BoundsChanged functions, they'd effectively be two different functions
anyways, just nested inside an if block.

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/vr_shell/vr_shell_ui.css (left):

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/resourc...
chrome/browser/resources/vr_shell/vr_shell_ui.css:10: --scaleFactor: 4;
On 2016/11/29 20:28:06, cjgrant wrote:
> Generally, updating the UI could be a separate change that happens after the
> decoupling lands, right?

In this case I would prefer not to update the UI in a separate change. Without
the changes here the UI would be so pixelated as to be totally unintelligible.

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/vr_shell/vr_shell_ui_api.js (right):

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/resourc...
chrome/browser/resources/vr_shell/vr_shell_ui_api.js:104: * Triggers an Action.
On 2016/11/29 20:28:06, cjgrant wrote:
> This needs to be updated.

Done.

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc (right):

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/ui/webu...
chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc:81:
CHECK(args->GetDouble(0, &width));
On 2016/11/29 20:28:06, cjgrant wrote:
> Might be clearer to pass a dictionary of 'width', 'height', 'dpr', in the
first
> list item, and drop the length check.  Just a thought.

Length check is equivalent to checking a dictionary is present at all. If you
prefer passing named values I can change it, but for a call this simple I don't
think it matters.

https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/ui/webu...
chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc:85:
vr_shell_->SetUiCssSize(width, height, dpr);
On 2016/11/29 20:28:06, cjgrant wrote:
> Does this not need to be synchronized on the rendering thread?

Yes and No. This part does not, it has to be called on the UI thread. The
callback VrShell::UiBoundsChanged should really be synchronized with the
rendering thread, but that's not so simple and I was planning on addressing it
in a followup patch. The problem is we don't know when the renderer actually
gives us a frame with the new size, which might be any number of frames in the
future. Right now we draw some small number of incorrect frames when changing
size. I'll throw a TODO in VrShell::UiBoundsChanged for now.

[cjgrant, 2016-11-29 21:39:33]

On 2016/11/29 21:07:22, mthiesse wrote:
>
https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/android...
> File chrome/browser/android/vr_shell/vr_shell.cc (right):
> 
>
https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/android...
> chrome/browser/android/vr_shell/vr_shell.cc:1025: void
> VrShell::SetContentCssSize(float width, float height, float dpr) {
> On 2016/11/29 20:28:06, cjgrant wrote:
> > Just a suggestion for consideration...  What about:
> > 
> > enum Contents {
> >   MAIN_CONTENTS,
> >   UI_CONTENTS
> > };
> > VrShell::SetCssSize(Contents contents, width, ...)
> > VrShell::UiBoundsChanged(Contents contents, JNI ...)
> > 
> 
> Mild preference for keeping as-is. Sharing enums between java/c++ is messy,
and
> for the BoundsChanged functions, they'd effectively be two different functions
> anyways, just nested inside an if block.
> 
>
https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/resourc...
> File chrome/browser/resources/vr_shell/vr_shell_ui.css (left):
> 
>
https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/resourc...
> chrome/browser/resources/vr_shell/vr_shell_ui.css:10: --scaleFactor: 4;
> On 2016/11/29 20:28:06, cjgrant wrote:
> > Generally, updating the UI could be a separate change that happens after the
> > decoupling lands, right?
> 
> In this case I would prefer not to update the UI in a separate change. Without
> the changes here the UI would be so pixelated as to be totally unintelligible.
> 
>
https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/resourc...
> File chrome/browser/resources/vr_shell/vr_shell_ui_api.js (right):
> 
>
https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/resourc...
> chrome/browser/resources/vr_shell/vr_shell_ui_api.js:104: * Triggers an
Action.
> On 2016/11/29 20:28:06, cjgrant wrote:
> > This needs to be updated.
> 
> Done.
> 
>
https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/ui/webu...
> File chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc (right):
> 
>
https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/ui/webu...
> chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc:81:
> CHECK(args->GetDouble(0, &width));
> On 2016/11/29 20:28:06, cjgrant wrote:
> > Might be clearer to pass a dictionary of 'width', 'height', 'dpr', in the
> first
> > list item, and drop the length check.  Just a thought.
> 
> Length check is equivalent to checking a dictionary is present at all. If you
> prefer passing named values I can change it, but for a call this simple I
don't
> think it matters.
> 
>
https://codereview.chromium.org/2428383006/diff/140001/chrome/browser/ui/webu...
> chrome/browser/ui/webui/vr_shell/vr_shell_ui_message_handler.cc:85:
> vr_shell_->SetUiCssSize(width, height, dpr);
> On 2016/11/29 20:28:06, cjgrant wrote:
> > Does this not need to be synchronized on the rendering thread?
> 
> Yes and No. This part does not, it has to be called on the UI thread. The
> callback VrShell::UiBoundsChanged should really be synchronized with the
> rendering thread, but that's not so simple and I was planning on addressing it
> in a followup patch. The problem is we don't know when the renderer actually
> gives us a frame with the new size, which might be any number of frames in the
> future. Right now we draw some small number of incorrect frames when changing
> size. I'll throw a TODO in VrShell::UiBoundsChanged for now.

lgtm on the UI front.

[bajones, 2016-11-29 23:22:15]

modules/vr/ LGTM, though this just stresses how much we need to get away from
the Fullscreen hack.

[mthiesse, 2016-11-29 23:27:21]

boliu@ please review content/browser/ changes, missed that file in my earlier
request.

[Ted C, 2016-11-30 00:12:04]

On 2016/11/29 23:27:21, mthiesse wrote:
> boliu@ please review content/browser/ changes, missed that file in my earlier
> request.

ui/snapshot - lgtm

[boliu, 2016-11-30 21:51:59]

On 2016/11/29 23:27:21, mthiesse wrote:
> boliu@ please review content/browser/ changes, missed that file in my earlier
> request.

lgtm

[bshe, 2016-11-30 23:00:47]

lgtm with nits

https://codereview.chromium.org/2428383006/diff/160001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
(right):

https://codereview.chromium.org/2428383006/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java:162:
setUiCssSize(DEFAULT_UI_WIDTH, DEFAULT_UI_HEIGHT, DEFAULT_DPR);
can line 161 -172 moved after we create mNativeVrShell? So we dont need to
manully call them after create nativeVrShell?

https://codereview.chromium.org/2428383006/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java:166:
dpr = 1f;
nit: avoid magic number

https://codereview.chromium.org/2428383006/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java:181:
nativeUIBoundsChanged(mNativeVrShell, mUiSurfaceWidth, mUiSurfaceHeight, 1f);
ditto

https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/android...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/android...
chrome/browser/android/vr_shell/vr_shell.cc:956: // actually sending us a
texture with the updated size somehow.
This TODO is confusing. Do you mean you want to synchronize the values with
renderer? IIRC, WasResized should sync these value on renderer side though IPC.

https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/resourc...
File chrome/browser/resources/vr_shell/vr_shell_ui.css (right):

https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/resourc...
chrome/browser/resources/vr_shell/vr_shell_ui.css:32:
#webvr-not-secure-permanent {
nit: remove this empty selector

https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/resourc...
chrome/browser/resources/vr_shell/vr_shell_ui.css:65:
#webvr-not-secure-transient {
ditto

https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/resourc...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/resourc...
chrome/browser/resources/vr_shell/vr_shell_ui.js:416: let uiElement =
document.querySelector('#ui');
nit: use uiRootElement directly?

[mthiesse, 2016-11-30 23:25:06]

https://codereview.chromium.org/2428383006/diff/160001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
(right):

https://codereview.chromium.org/2428383006/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java:162:
setUiCssSize(DEFAULT_UI_WIDTH, DEFAULT_UI_HEIGHT, DEFAULT_DPR);
On 2016/11/30 23:00:46, bshe wrote:
> can line 161 -172 moved after we create mNativeVrShell? So we dont need to
> manully call them after create nativeVrShell?

If you'd like I can split up initialization into more steps, but I do it like
this so that we only load the UI contents after the display size has been set.
nativeInit starts loading the UI contents.

https://codereview.chromium.org/2428383006/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java:166:
dpr = 1f;
On 2016/11/30 23:00:46, bshe wrote:
> nit: avoid magic number

Done.

https://codereview.chromium.org/2428383006/diff/160001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java:181:
nativeUIBoundsChanged(mNativeVrShell, mUiSurfaceWidth, mUiSurfaceHeight, 1f);
On 2016/11/30 23:00:46, bshe wrote:
> ditto

Good catch, that was a typo.

https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/android...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/android...
chrome/browser/android/vr_shell/vr_shell.cc:956: // actually sending us a
texture with the updated size somehow.
On 2016/11/30 23:00:46, bshe wrote:
> This TODO is confusing. Do you mean you want to synchronize the values with
> renderer? IIRC, WasResized should sync these value on renderer side though
IPC.

Moved into MainFrameWasResized, and updated the comment.

https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/resourc...
File chrome/browser/resources/vr_shell/vr_shell_ui.css (right):

https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/resourc...
chrome/browser/resources/vr_shell/vr_shell_ui.css:32:
#webvr-not-secure-permanent {
On 2016/11/30 23:00:46, bshe wrote:
> nit: remove this empty selector

Done.

https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/resourc...
chrome/browser/resources/vr_shell/vr_shell_ui.css:65:
#webvr-not-secure-transient {
On 2016/11/30 23:00:46, bshe wrote:
> ditto

Done.

https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/resourc...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/resourc...
chrome/browser/resources/vr_shell/vr_shell_ui.js:416: let uiElement =
document.querySelector('#ui');
On 2016/11/30 23:00:46, bshe wrote:
> nit: use uiRootElement directly?

Done.

[bshe, 2016-12-01 19:13:45]

On 2016/11/30 23:25:06, mthiesse wrote:
>
https://codereview.chromium.org/2428383006/diff/160001/chrome/android/java/sr...
> File
> chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
> (right):
> 
>
https://codereview.chromium.org/2428383006/diff/160001/chrome/android/java/sr...
>
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java:162:
> setUiCssSize(DEFAULT_UI_WIDTH, DEFAULT_UI_HEIGHT, DEFAULT_DPR);
> On 2016/11/30 23:00:46, bshe wrote:
> > can line 161 -172 moved after we create mNativeVrShell? So we dont need to
> > manully call them after create nativeVrShell?
> 
> If you'd like I can split up initialization into more steps, but I do it like
> this so that we only load the UI contents after the display size has been set.
> nativeInit starts loading the UI contents.
> 
>
https://codereview.chromium.org/2428383006/diff/160001/chrome/android/java/sr...
>
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java:166:
> dpr = 1f;
> On 2016/11/30 23:00:46, bshe wrote:
> > nit: avoid magic number
> 
> Done.
> 
>
https://codereview.chromium.org/2428383006/diff/160001/chrome/android/java/sr...
>
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java:181:
> nativeUIBoundsChanged(mNativeVrShell, mUiSurfaceWidth, mUiSurfaceHeight, 1f);
> On 2016/11/30 23:00:46, bshe wrote:
> > ditto
> 
> Good catch, that was a typo.
> 
>
https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/android...
> File chrome/browser/android/vr_shell/vr_shell.cc (right):
> 
>
https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/android...
> chrome/browser/android/vr_shell/vr_shell.cc:956: // actually sending us a
> texture with the updated size somehow.
> On 2016/11/30 23:00:46, bshe wrote:
> > This TODO is confusing. Do you mean you want to synchronize the values with
> > renderer? IIRC, WasResized should sync these value on renderer side though
> IPC.
> 
> Moved into MainFrameWasResized, and updated the comment.
> 
>
https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/resourc...
> File chrome/browser/resources/vr_shell/vr_shell_ui.css (right):
> 
>
https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/resourc...
> chrome/browser/resources/vr_shell/vr_shell_ui.css:32:
> #webvr-not-secure-permanent {
> On 2016/11/30 23:00:46, bshe wrote:
> > nit: remove this empty selector
> 
> Done.
> 
>
https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/resourc...
> chrome/browser/resources/vr_shell/vr_shell_ui.css:65:
> #webvr-not-secure-transient {
> On 2016/11/30 23:00:46, bshe wrote:
> > ditto
> 
> Done.
> 
>
https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/resourc...
> File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):
> 
>
https://codereview.chromium.org/2428383006/diff/160001/chrome/browser/resourc...
> chrome/browser/resources/vr_shell/vr_shell_ui.js:416: let uiElement =
> document.querySelector('#ui');
> On 2016/11/30 23:00:46, bshe wrote:
> > nit: use uiRootElement directly?
> 
> Done.

lgtm

[mthiesse, 2016-12-01 19:17:43]

The CQ bit was checked by mthiesse@chromium.org

[mthiesse, 2016-12-01 19:17:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org, bajones@chromium.org, boliu@chromium.org,
tedchoc@chromium.org, cjgrant@chromium.org
Link to the patchset: https://codereview.chromium.org/2428383006/#ps180001
(title: "Address bshe comments + minor fix")

[commit-bot: I haz the power, 2016-12-01 19:18:12]

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-12-01 20:46:29]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1480619863052590,
"parent_rev": "93c45c2b2d4edb8021adc0b8b091067d2aae4602", "commit_rev":
"e68d07d7aeab53b9dcec23fd154352a996d8e5b8"}

[commit-bot: I haz the power, 2016-12-01 20:46:55]

Committed patchset #5 (id:180001)

[commit-bot: I haz the power, 2016-12-01 20:49:14]

Description was changed from

==========
Decouple VR Shell DPR and CSS size from Physical Displays.

This CL introduces two virtual displays for VR Shell, one for the content
window, and one for the background UI window (which is made as small as possible
to save on memory).

The UI now dynamically controls its size, and the content defaults to a
reasonable size regular web browsing (in VR).

BUG=643480
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Decouple VR Shell DPR and CSS size from Physical Displays.

This CL introduces two virtual displays for VR Shell, one for the content
window, and one for the background UI window (which is made as small as possible
to save on memory).

The UI now dynamically controls its size, and the content defaults to a
reasonable size regular web browsing (in VR).

BUG=643480
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/4cc2259e5139ce40daa4230b25a4ee7183c23d1b
Cr-Commit-Position: refs/heads/master@{#435702}
==========

[commit-bot: I haz the power, 2016-12-01 20:49:16]

Patchset 5 (id:??) landed as
https://crrev.com/4cc2259e5139ce40daa4230b25a4ee7183c23d1b
Cr-Commit-Position: refs/heads/master@{#435702}