Made root translation layer the root when using pinch virtual viewport.

Made root translation layer the root when using pinch virtual viewport.

The "root" translation layer was previously below the overlap controls
host layer. This meant it was actually below the pinch viewport's scale
layer. We really want the root translation layer to be at the root so that
DevTools' screen emulation feature can scale and offset the entire tree.

BUG=370035
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175496

[bokan, 2014-05-30 18:50:18]

Alexandre: ptal
Ian: mind double checking I'm not doing anything fishy compositor-wise? I've
made sure it doesn't break those two bugs you gave me earlier in the week.

[dgozman, 2014-05-30 19:09:42]

I see that you put overlay layer (which is usually an inspector overlay) under
the root graphics layer, so it's transformed exactly as page contents. That's
right! Looks good from DevTools side.

[bokan, 2014-05-30 19:22:37]

On 2014/05/30 19:09:42, dgozman wrote:
> I see that you put overlay layer (which is usually an inspector overlay) under
> the root graphics layer, so it's transformed exactly as page contents. That's
> right! Looks good from DevTools side.

That's right, the overlay stays under the overflow controls host so it'll get
the screen emulation and pinch transforms applied.

[aelias_OOO_until_Jul13, 2014-06-03 01:11:22]

The layer was optional because we want to avoid the layer tree bloat on every
frame from a rarely used feature.  Is there a reason why it's harder now to keep
it optional?

[bokan, 2014-06-03 16:14:58]

On 2014/06/03 01:11:22, aelias wrote:
> The layer was optional because we want to avoid the layer tree bloat on every
> frame from a rarely used feature.  Is there a reason why it's harder now to
keep
> it optional?

There were comments saying it was optional, but it looks like it was
inadvertently always being created since setRootGraphicsLayer calls
updateRootLayerTransform. Is saving a layer worth the moderate added complexity?
Additionally, I'm hoping to make a case for this to merge back to 36. Because of
that, changing it to be actually optional should be a follow up patch.

[aelias_OOO_until_Jul13, 2014-06-03 19:02:49]

I'd say it's worth the added complexity given that it affects the critical
scrolling path.  But lgtm for this given merge case and non-regression.

[bokan, 2014-06-03 19:17:47]

Thanks.

Ian, could you PTAL at Source/core? Thanks.

[Ian Vollick, 2014-06-04 13:22:12]

lgtm2

https://codereview.chromium.org/308183002/diff/1/Source/core/frame/PinchViewp...
File Source/core/frame/PinchViewport.h (right):

https://codereview.chromium.org/308183002/diff/1/Source/core/frame/PinchViewp...
Source/core/frame/PinchViewport.h:77: return m_rootTransformLayer.get();
why two accessors for the same value?

[bokan, 2014-06-04 15:12:25]

https://codereview.chromium.org/308183002/diff/1/Source/core/frame/PinchViewp...
File Source/core/frame/PinchViewport.h (right):

https://codereview.chromium.org/308183002/diff/1/Source/core/frame/PinchViewp...
Source/core/frame/PinchViewport.h:77: return m_rootTransformLayer.get();
On 2014/06/04 13:22:12, Ian Vollick wrote:
> why two accessors for the same value?

You're right, this is overly verbose. Removed.

[bokan, 2014-06-04 15:41:54]

The CQ bit was checked by bokan@chromium.org

[commit-bot: I haz the power, 2014-06-04 15:42:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bokan@chromium.org/308183002/40001

[commit-bot: I haz the power, 2014-06-04 17:18:54]

Change committed as 175496