Could be a device scale thing with OOPIFs.  We've fixed some of those recently, and I'm not sure if that fix is in M52 or not, or if this is a separate issue.  (I can test on M53 when I get back to the office.)

I assume this is on a high DPI Pixel?

Assigning to wjmaclean@ since he's looked at similar issues.

Yeah, this is a Pixel 2.

Has anyone tested yet to see if non-Pixel devices running the same versions work as intended?

Assuming it *does* work ok on a dsf=1 device, then this looks like a device-scale-factor issue, and we have a fix in the works (for  https://crbug.com/614215 , but let's keep this as a separate bug until we can confirm otherwise).

Comment 3: I don't have a non-Pixel ChromeOS device to test on, but I set my resolution to 2560x1700 (Native) and the bug went away-- the Hangouts panels are the correct width and the scrollbars are visible.  So yes, it does seem like a device scale factor issue.  (Also, wow, those are some tiny pixels in Native mode!)

Hopefully your fix for  issue 614215  will fix this as well.

Marking as confirmed, as well, since I was able to repro the bug on 52.0.2743.0 Dev with --isolate-extensions enabled.

I've done some more triage here.

1) Seems to only occur when device scale factor != 1
2) does *not* occur with --use-cross-process-frames-for-guests
3) my CL for fixing device scale factor on OOPIF does *not* fix this issue

I'm continuing to investigate ...

+ oshima@

This seems to be related to UseZoomForDSF ... I can recreate this with desktop (linux) Chrome with the following flags:

--isolate-extensions --enable-use-zoom-for-dsf --force-device-scale-factor=2

Omitting any one of these flags causes the problem to not reproduce, so RenderWidgetHostViewChildFrame/CrossProcessFrameConnector/ChildFrameCompositingHelper are likely involved somehow. I'm guessing that a size in RenderWidgetHostViewGuest is including the scale factor a second time, causing the content to be twice (or device_scale_factor) as big as the window expects.

My current guess is that somehow the backing is getting the device scale factor twice, and thus being drawn bigger than it should be. I'm digging around in ChildFrameCompositingHelper::OnSetSurface() and also the physical_backing_size values in RenderWidget, though so far I haven't discovered where the mismatch is.

Ooops, didn't mean RenderWidgetHostViewGuest in the previous comment ... rather just one of RWHVCF/CPFC/CFCH ...

Thanks for investigating!  Is UseZoomForDSF limited to ChromeOS (apart from the --enable-use-zoom-for-dsf flag)?  I'm curious if this needs to be a blocking issue for launching --isolate-extensions on Win/Mac/Linux.

I *think* it's CrOS specific ... but oshima@ can better answer that question. 

This is launched on Windows on m53. +besp@ who worked on it.

Yeah it should be on for Windows too. Is there an easy way to make an OOPIF to test?

Comment 12: Start Chrome with --site-per-process and visit http://csreis.github.io/tests/cross-site-iframe-simple.html.

Looks like it repros on Windows too (54.0.2826.0).

Deleted: iframe-oopif.PNG 42.8 KB

	iframe-oopif.PNG 42.8 KB View Download

Thanks; I can confirm it as well.  The OOPIF draws at the wrong scale factor if you use --force-device-scale-factor=2.  That means things like the Hangouts extension will be broken on high-DPI Windows devices with --isolate-extensions.

As a result, this blocks the launch of --isolate-extensions.  wjmaclean@ / oshima@: Do you think we can get a fix for this ASAP for M54?

I'm planning on spending my day tomorrow trying to track it down. Almost certainly when the cause is found it will be a simple fix, but finding exactly where it goes wrong may or may not be challenging ...

I was wondering if it might be extension specific, as I have tested the new OOPIF DSF code on CrOS (both on device and on Linux desktop using the ash-window geometry flag, and regular pages seemed to work ok.

Ok, so to confirm, when I build for CrOS on Linux, and run it as

out/CrOS/chrome --ash-host-window-bounds=950+0-1000x600,1920+0-2000x1200*2 --site-per-process --no-sandbox

it works. I'm assuming that --enable-use-zoom-for-dsf is automatically 'on' in this case, though DSF still works (with the oopif DSF patch included) even if I explicitly include the flag.

Is there something different about using --ash-host-window-bounds as opposed to --force-device-scale-factor?

Last comment for tonight: the example from csreis.github when I try it out, it seems like the DSF is being correctly set on the subframe (inspect window.devicePixelRatio both in and out of the frame), but somehow the framerect size is not adjusted properly (when you drag the example between the two ash windows, the relative magnification of the text in and out of the frame is the same, as evidenced by the first line of test, but the frame contents re-layout to a much wider size hence the change in appearance).

I'm guessing this is about the subframe content thinking it's in a framerect that is bigger (by device_scale_factor) than it ought to be. I'll investigate places where we might need to use IsUseZoomForDSFEnabled() to scale/un-scale a rect for a subframe ... oshima@ if you have any hints where I should look for this I'd appreciate your thoughts.

I've run out of time for working on this today, though I've made some progress.

When IsUseZoomForDSFEnabled() is true, changes to device scale factor (dsf) are causing the content to see window.innerWidth/Height change by dsf, when they should really be constant, hence the re-layout issue.

It seems that the renderer code that is sending frame rects to CrossProcessFrameConnector::OnFrameRectChanged() is sending it in pixels (viewport coords), and not in DIP as the browser expects (Window coords). If one fixes this, then window.innerWidth/Height stay constant (as they should), and no re-layout occurs, but only a fraction of the frame is displayed (where the fraction seems to be 1/dsf in each direction). But what is rendered is correct: links can be clicked, and highlight when moused-over, and scrolling works, etc.

So the next step seems to be figuring out the partial rendering, which I suspect has to do with the surface creation when IsUseZoomForDSFEnabled() is true. In ChildFrameCompositingHelper, it seems suspicious that the value of buffer_size_ never changes, and that we are sending a DIP frame size to surface_layer->SetSurfaceId() when it expects (I think) pixel-size instead. But I haven't yet figured out the exact inter-relationship between the scales and sizes in this class yet. It seems possible this class has never before had to deal with non-1.0 scale values, so there may be some weirdness here ... I'm not sure. With the fix to the CPFC::OnFrameRectChanged() values in place, it seems that the frame_size passed into CFCH::OnSetSurface() never seems to change, nor does scale (it gets reset to 1.0 when IsUseZoomForDSFEnabled() is true), meaning it's not surprising the surface size remains unchanged even when dsf changes.

I'll continue to work on this when I get back on the 22nd, but if anyone wants to take a crack at the surface aspect in the meantime, hopefull what I've described here helps.

Thanks for investigating and posting the update!

I'm hoping we can get this resolved before the 22nd, because that's much too close to the branch cut.  kenrb@ or oshima@, would you be able to help with the fix next week?

I'll take a look.

Thanks Lucas!

I have a WIP patch that fixes this, but the fix doesn't seem quite right.

The problem happens because here https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_impl.cc?rcl=1471276009&l=593 , we pass the screen size instead of size in DIPs to the renderer. RWHV::GetRequestedRendererSize() is implemented here https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_view_base.cc?rcl=1471276009&l=132 , and returns the size of the view bounds.

However, the comment in GetViewBounds() in RWHV here https://cs.chromium.org/chromium/src/content/public/browser/render_widget_host_view.h?sq=package:chromium&rcl=1471252828&l=118 says that this should've been in screen coordinates, which is what RWHVChildFrame is doing.

So I could either override the value in GetRequestedRendererSize() for child frames, or try to change GetViewBounds() to return size in DIPs for RWHVCF.

oshima@, any thoughts on this?

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

commit 1f9011c2bae856ae3c0c30f10f04e5c027319189
Author: lfg <lfg@chromium.org>
Date: Wed Aug 17 21:18:42 2016

Use the correct scale factor when rendering RemoteFrames with UseZoomForDSF.

The view bounds reported by RenderWidgetHostViewChildFrame must return the frame size without the device scale factor applied.

BUG= 621020 

Review-Url: https://codereview.chromium.org/2248823003
Cr-Commit-Position: refs/heads/master@{#412639}

[modify] https://crrev.com/1f9011c2bae856ae3c0c30f10f04e5c027319189/content/renderer/render_frame_proxy.cc
[modify] https://crrev.com/1f9011c2bae856ae3c0c30f10f04e5c027319189/content/renderer/render_frame_proxy.h
[modify] https://crrev.com/1f9011c2bae856ae3c0c30f10f04e5c027319189/content/renderer/render_widget.h
[modify] https://crrev.com/1f9011c2bae856ae3c0c30f10f04e5c027319189/third_party/WebKit/Source/web/WebRemoteFrameImpl.cpp
[modify] https://crrev.com/1f9011c2bae856ae3c0c30f10f04e5c027319189/third_party/WebKit/public/web/WebRemoteFrameClient.h