+fsamuel@

I believe the pathways that propagate device scale factor to OOPIFs changed as part of the new viz surface sync work.

Is this possibly related to

https://bugs.chromium.org/p/chromium/issues/detail?id=778443

?

Oddly enough this works fine for <webview> but is broken for OOPIFs on this page. I'm not yet sure why but I do suspect this is a surface sync bug. Assigning myself to it.

I've verified that device scale factor does get propagated correctly from parent to child and a new LocalSurfaceId is allocated on the way done. This bug seems specific to using zoom for device scale factor. --enable-use-zoom-for-dsf=false makes the problem go away. I'm continuing to investigate.

OK, I honestly have no idea what's going on but I verified this happens even if I disable surface sync via --disable-features="SurfaceSynchronization" so I'm passing this along to a DSF expert: oshima@.

We found a bug & a fix. Looking into how we can test this now.

That's great!  Any updates on the test?

CL under review

https://chromium-review.googlesource.com/c/chromium/src/+/882301

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

commit 3c07b33798715fee176f564bc53a341489b02956
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Tue Feb 06 04:56:50 2018

Update the scale of the RenderWidget for ooip frame when dsf changes

BUG= 803442 
TEST=Covered by unittest. Also tested manually.

Change-Id: I1b77c6d589596f66ed1bc9135d6be26318df872a
Reviewed-on: https://chromium-review.googlesource.com/882301
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534627}
[modify] https://crrev.com/3c07b33798715fee176f564bc53a341489b02956/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/3c07b33798715fee176f564bc53a341489b02956/content/renderer/render_view_impl.cc
[modify] https://crrev.com/3c07b33798715fee176f564bc53a341489b02956/content/renderer/render_view_impl.h
[modify] https://crrev.com/3c07b33798715fee176f564bc53a341489b02956/content/renderer/render_widget.cc
[modify] https://crrev.com/3c07b33798715fee176f564bc53a341489b02956/content/renderer/render_widget.h
[modify] https://crrev.com/3c07b33798715fee176f564bc53a341489b02956/content/test/BUILD.gn
[modify] https://crrev.com/3c07b33798715fee176f564bc53a341489b02956/ui/aura/test/test_screen.cc

 Issue 807112  has been merged into this issue.

oshima@, how risky is this change for merging back in M65?

I made this change soon after branch cut, so the risk should be low.
I could cherry pick cleanly and it seems to be working (I built and tested the  build on branch)

Do you want to merge to 65?

The CL looks fine to merge for me, however it does move code around and I wonder if other merges would be impacted. Since it has been on canary for a while, I support merging it in M65.

Forgot to add proper label, doing so now.

This bug requires manual review: We are only 12 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

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

Rejecting merge to M65 per offline chat with nasko@ as we're very close to M65 stable promotion to minimize the risk. Thank you.