[VR Shell] Fix hidden elements being hit-testable.

[VR Shell] Fix hidden elements being hit-testable.

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

Review-Url: https://codereview.chromium.org/2710593008
Cr-Commit-Position: refs/heads/master@{#452920}
Committed: https://chromium.googlesource.com/chromium/src/+/6cb5aaabcff899c2399082578944c9f8a88cee86

[mthiesse, 2017-02-23 22:31:59]

Description was changed from

==========
[VR Shell] Fix hidden elements being hit-testable.

BUG=
==========

to

==========
[VR Shell] Fix hidden elements being hit-testable.

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

[mthiesse, 2017-02-23 22:32:41]

mthiesse@chromium.org changed reviewers:
+ bshe@chromium.org, cjgrant@chromium.org, tiborg@chromium.org

[mthiesse, 2017-02-23 22:32:42]

PTAL, whoever gets to this first. Trivial change.

[tiborg, 2017-02-23 22:39:56]

https://codereview.chromium.org/2710593008/diff/1/chrome/browser/resources/vr...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2710593008/diff/1/chrome/browser/resources/vr...
chrome/browser/resources/vr_shell/vr_shell_ui.js:57:
backPlane.setVisible(false);
Doesn't this need to be visible?

[mthiesse, 2017-02-23 22:52:58]

https://codereview.chromium.org/2710593008/diff/1/chrome/browser/resources/vr...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2710593008/diff/1/chrome/browser/resources/vr...
chrome/browser/resources/vr_shell/vr_shell_ui.js:57:
backPlane.setVisible(false);
On 2017/02/23 22:39:56, tiborg wrote:
> Doesn't this need to be visible?

Oops yes

[tiborg, 2017-02-24 00:17:29]

lgtm

[cjgrant, 2017-02-24 14:25:24]

lgtm

https://codereview.chromium.org/2710593008/diff/20001/chrome/browser/resource...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2710593008/diff/20001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.js:57: backPlane.setVisible(true);
Why not remove setVisible() and setHitTestable() completely, as they are the
defaults?  The comment says "an invisible", and the first setter is
visibility(true). :)  The comment could also use a tweak.

[mthiesse, 2017-02-24 15:30:09]

https://codereview.chromium.org/2710593008/diff/20001/chrome/browser/resource...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2710593008/diff/20001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.js:57: backPlane.setVisible(true);
On 2017/02/24 14:25:23, cjgrant wrote:
> Why not remove setVisible() and setHitTestable() completely, as they are the
> defaults?  The comment says "an invisible", and the first setter is
> visibility(true). :)  The comment could also use a tweak.

Done.

[mthiesse, 2017-02-24 15:30:31]

The CQ bit was checked by mthiesse@chromium.org

[mthiesse, 2017-02-24 15:30:31]

The patchset sent to the CQ was uploaded after l-g-t-m from tiborg@chromium.org,
cjgrant@chromium.org
Link to the patchset: https://codereview.chromium.org/2710593008/#ps40001
(title: "Address comments")

[commit-bot: I haz the power, 2017-02-24 15:30:43]

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, 2017-02-24 15:30:44]

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

[commit-bot: I haz the power, 2017-02-24 15:30:45]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
Committers are members of the group "project-chromium-committers".
Note that this has nothing to do with OWNERS files.

[bshe, 2017-02-24 16:18:12]

On 2017/02/24 15:30:45, commit-bot: I haz the power wrote:
> No L-G-T-M from a valid reviewer yet. 
> CQ run can only be started by full committers or once the patch has
> received an L-G-T-M from a full committer.
> Even if an L-G-T-M may have been provided, it was from a non-committer,
> _not_ a full super star committer.
> Committers are members of the group "project-chromium-committers".
> Note that this has nothing to do with OWNERS files.

rubber stamp lgtm

[mthiesse, 2017-02-24 16:21:00]

The CQ bit was checked by mthiesse@chromium.org

[commit-bot: I haz the power, 2017-02-24 16:21:40]

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, 2017-02-24 17:04:12]

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

[commit-bot: I haz the power, 2017-02-24 17:04:13]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[mthiesse, 2017-02-24 17:09:06]

The CQ bit was checked by mthiesse@chromium.org

[commit-bot: I haz the power, 2017-02-24 17:09:22]

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, 2017-02-24 18:12:47]

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

[commit-bot: I haz the power, 2017-02-24 18:12:48]

Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[mthiesse, 2017-02-24 18:57:27]

The CQ bit was checked by mthiesse@chromium.org

[mthiesse, 2017-02-24 18:57:27]

The patchset sent to the CQ was uploaded after l-g-t-m from tiborg@chromium.org,
bshe@chromium.org, cjgrant@chromium.org
Link to the patchset: https://codereview.chromium.org/2710593008/#ps60001
(title: "Fix closure compiler error")

[commit-bot: I haz the power, 2017-02-24 18:58:02]

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, 2017-02-24 19:11:27]

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

[commit-bot: I haz the power, 2017-02-24 19:11:28]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mthiesse, 2017-02-24 19:43:35]

The CQ bit was checked by mthiesse@chromium.org

[commit-bot: I haz the power, 2017-02-24 19:44:07]

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, 2017-02-24 20:01:18]

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

[commit-bot: I haz the power, 2017-02-24 20:01:20]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mthiesse, 2017-02-24 20:09:19]

The CQ bit was checked by mthiesse@chromium.org

[commit-bot: I haz the power, 2017-02-24 20:09:59]

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, 2017-02-24 21:17:02]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1487966959645700,
"parent_rev": "421c3a3a8e73cf590f69624b15e8187a0ec413ba", "commit_rev":
"6cb5aaabcff899c2399082578944c9f8a88cee86"}

[commit-bot: I haz the power, 2017-02-24 21:17:45]

Description was changed from

==========
[VR Shell] Fix hidden elements being hit-testable.

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

to

==========
[VR Shell] Fix hidden elements being hit-testable.

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

Review-Url: https://codereview.chromium.org/2710593008
Cr-Commit-Position: refs/heads/master@{#452920}
Committed:
https://chromium.googlesource.com/chromium/src/+/6cb5aaabcff899c2399082578944...
==========

[commit-bot: I haz the power, 2017-02-24 21:17:46]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/6cb5aaabcff899c2399082578944...