virtual/scalefactor200/fast/hidpi/static/validation-bubble-appearance-hidpi.html failing on WebKit Mac10.13 (retina) |
||||
Issue descriptionThis started here: https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.13%20%28retina%29/263 . Failure looks to be because of golden images not matching: 9:34:53.543 49202 "/b/s/w/ir/out/Release/image_diff --diff /b/s/w/itqSin9p/tmp_EMj8D/actual.png /b/s/w/itqSin9p/tmp_EMj8D/expected.png /b/s/w/itqSin9p/tmp_EMj8D/diff.png" took 0.20s 09:34:53.546 49154 [1087/5973] virtual/scalefactor200/fast/hidpi/static/validation-bubble-appearance-hidpi.html failed unexpectedly (image diff) 09:34:53.545 49202 worker/6 virtual/scalefactor200/fast/hidpi/static/validation-bubble-appearance-hidpi.html failed: https://chromium-review.googlesource.com/c/chromium/src/+/1150673 changes around code related to ScreenInfo, which seems like it may have impacted this. I'm going to revert
,
Jul 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ae08f33b34db0a95b188315f64f6bda747e77351 commit ae08f33b34db0a95b188315f64f6bda747e77351 Author: Scott Violet <sky@chromium.org> Date: Fri Jul 27 17:37:23 2018 Revert "Move some of VisualProperties to RenderWidget::Init()." This reverts commit 628bdce65237022927e4694d55cb4df8fd0af297. Reason for revert: virtual/scalefactor200/fast/hidpi/static/validation-bubble-appearance-hidpi.html started failing on WebKit Mac10.13 (retina) and this patch is on the blame list and seems related. Output isn't that useful, other than to say golden images don't match: 9:34:53.543 49202 "/b/s/w/ir/out/Release/image_diff --diff /b/s/w/itqSin9p/tmp_EMj8D/actual.png /b/s/w/itqSin9p/tmp_EMj8D/expected.png /b/s/w/itqSin9p/tmp_EMj8D/diff.png" took 0.20s 09:34:53.546 49154 [1087/5973] virtual/scalefactor200/fast/hidpi/static/validation-bubble-appearance-hidpi.html failed unexpectedly (image diff) 09:34:53.545 49202 worker/6 virtual/scalefactor200/fast/hidpi/static/validation-bubble-appearance-hidpi.html failed: Original change's description: > Move some of VisualProperties to RenderWidget::Init(). > > VisualProperties are passed to the RenderViewImpl::Create() method via > the IPC params, then are passed along to the RenderWidget, but in a > lot of different steps. > > 1st, screen info is passed to the RenderWidget constructor. > 2nd, the display mode is set directly on the RenderWidget. > [ a bunch of other initialization here ] > 3rd, the whole set of properties is passed to > OnSynchronizeVisualProperties like an IPC was just received. > > The 3rd step sends the screen info and the display mode again, though > they won't be changed of course. > > This combines step 1 and 2 into one step where they are passed to the > RenderWidget::Init() method. Then we can look at combining with step 3. > > R=ajwong@chromium.org, avi@chromium.org > > Bug: 419087 > Change-Id: Ifcb8eaa0915bb235172e8c5b7bee188e863c18c3 > Reviewed-on: https://chromium-review.googlesource.com/1150673 > Commit-Queue: danakj <danakj@chromium.org> > Reviewed-by: Albert J. Wong <ajwong@chromium.org> > Reviewed-by: Avi Drissman <avi@chromium.org> > Cr-Commit-Position: refs/heads/master@{#578660} TBR=avi@chromium.org,ajwong@chromium.org,danakj@chromium.org Change-Id: Ife47ce2124a0a5baffaa2dd08b3dfe7de79cc568 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 419087, 868444 Reviewed-on: https://chromium-review.googlesource.com/1153398 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#578694} [modify] https://crrev.com/ae08f33b34db0a95b188315f64f6bda747e77351/content/renderer/render_view_impl.cc [modify] https://crrev.com/ae08f33b34db0a95b188315f64f6bda747e77351/content/renderer/render_widget.cc [modify] https://crrev.com/ae08f33b34db0a95b188315f64f6bda747e77351/content/renderer/render_widget.h [modify] https://crrev.com/ae08f33b34db0a95b188315f64f6bda747e77351/content/renderer/render_widget_fullscreen_pepper.cc [modify] https://crrev.com/ae08f33b34db0a95b188315f64f6bda747e77351/content/renderer/render_widget_fullscreen_pepper.h [modify] https://crrev.com/ae08f33b34db0a95b188315f64f6bda747e77351/content/renderer/render_widget_unittest.cc [modify] https://crrev.com/ae08f33b34db0a95b188315f64f6bda747e77351/content/test/layouttest_support.cc
,
Jul 27
The bot did go green, so it looks like Avi's patch did trigger this. Removing Sheriff-Chromium.
,
Jul 27
As the bot is green, I'm also moving to Fixed.
,
Jul 27
Um... 628bdce65237022927e4694d55cb4df8fd0af297 is not me. I reviewed it, but it was Dana. Also, given the revert worked, let's call this fixed. Dana knows that it failed and is working out why (see https://chromium-review.googlesource.com/c/chromium/src/+/1153311).
,
Jul 27
Sorry about that Avi. |
||||
►
Sign in to add a comment |
||||
Comment 1 by sky@chromium.org
, Jul 27