It's not just the work in VrShellGl, the "viewport aware" branch activates
a second layer in GVR which needs to be composited there every frame, and I
think that's expensive even if nothing got drawn into it.

Does this affect M62 also, or was the issue introduced after the branch? If applicable, we should try to backport the fix once available.

hah. good catch. I think it probably affects M62 too. Let me see if I can find an easy fix and merge it back to M62.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f87da667a00fb447247652ee7d5c90390cc4ca0a

commit f87da667a00fb447247652ee7d5c90390cc4ca0a
Author: Biao She <bshe@chromium.org>
Date: Mon Sep 11 23:12:15 2017

GetViewportAwareElements should be empty when all child elements are invisible

GetViewportAwareElements always return a non empty list because we always have
two visible ViewportAwareRoot elements (one for ChromeVR and one for WebVR) in
our hierarchy.

We should exclude ViewportAwareRoot element from the list returned by the
function. It is a no visual element. So it shouldn't trigger code path that is
used to draw viewport aware elements.

Bug:  763738 
Change-Id: I29c14b60e3fc16e224c5e540aa438f25e1c871ef
Reviewed-on: https://chromium-review.googlesource.com/661260
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Commit-Queue: Biao She <bshe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501082}
[modify] https://crrev.com/f87da667a00fb447247652ee7d5c90390cc4ca0a/chrome/browser/vr/ui_scene.cc
[modify] https://crrev.com/f87da667a00fb447247652ee7d5c90390cc4ca0a/chrome/browser/vr/ui_scene_unittest.cc

This affects WebVR preformance. So request merge back to M62 if possible. Thanks!

using Merge-Request-62 not Merge-Rejected-62 :p

FYI, the fix has a significant effect on performance https://chromeperf.appspot.com/report?sid=7c02adfc32f3a06f2ae04dd9ae7a74d5c8323aa55cd7d0f89953aca2178e1411 (ignore the odd massive spikes - that's a different issue) (need to log in with your @google account to see)

+4 fps on average
-3 ms on frame time
-4.5 ms on pose prediction

Thanks for the data!
Given the performance impact. Making it a p1 instead of p2.

Your change meets the bar and is auto-approved for M62. Please go ahead and merge the CL to branch 3202 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe406f47d73a0ac6e90a7e1129f8add2b44a4e53

commit fe406f47d73a0ac6e90a7e1129f8add2b44a4e53
Author: Biao She <bshe@chromium.org>
Date: Wed Sep 13 16:11:00 2017

GetViewportAwareElements should be empty when all child elements are invisible

GetViewportAwareElements always return a non empty list because we always have
two visible ViewportAwareRoot elements (one for ChromeVR and one for WebVR) in
our hierarchy.

We should exclude ViewportAwareRoot element from the list returned by the
function. It is a no visual element. So it shouldn't trigger code path that is
used to draw viewport aware elements.

TBR=bshe@chromium.org

(cherry picked from commit f87da667a00fb447247652ee7d5c90390cc4ca0a)

Bug:  763738 
Change-Id: I29c14b60e3fc16e224c5e540aa438f25e1c871ef
Reviewed-on: https://chromium-review.googlesource.com/661260
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Commit-Queue: Biao She <bshe@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#501082}
Reviewed-on: https://chromium-review.googlesource.com/664959
Reviewed-by: Biao She <bshe@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#203}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/fe406f47d73a0ac6e90a7e1129f8add2b44a4e53/chrome/browser/vr/ui_scene.cc
[modify] https://crrev.com/fe406f47d73a0ac6e90a7e1129f8add2b44a4e53/chrome/browser/vr/ui_scene_unittest.cc