New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 769508 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 26
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-11-26
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 477150



Sign in to add a comment

fast/css/acid2* layout tests fail with text diff when *both* PlzNavigate and --site-per-process are used

Project Member Reported by lukasza@chromium.org, Sep 27 2017

Issue description

REPRO STEPS:

$ DISPLAY=:20 third_party/WebKit/Tools/Scripts/run-webkit-tests -t gn --no-retry --additional-drt-flag=--site-per-process fast/css/acid2*


Note that

- The test is run with --site-per-process flag (passed via --additional-drt-flag=...)

- The test passes after adding --additional-drt-flag=--disable-browser-side-navigation


Example text diffs:

$ cat out/gn/layout-test-results/fast/css/acid2-diff.txt 
--- /usr/local/google/home/lukasza/src/chromium3/src/out/gn/layout-test-results/fast/css/acid2-expected.txt
+++ /usr/local/google/home/lukasza/src/chromium3/src/out/gn/layout-test-results/fast/css/acid2-actual.txt
@@ -58,11 +58,11 @@
 layer at (84,2712) size 144x24 backgroundClip at (0,0) size 800x600 clip at (0,0) size 800x600
   LayoutBlockFlow (positioned) {DIV} at (48,72) size 144x24 [bgcolor=#FF0000]
     LayoutBlockFlow {DIV} at (0,0) size 144x0
-      LayoutInline {OBJECT} at (0,0) size 131x15
-        LayoutInline {OBJECT} at (0,0) size 131x15
-          LayoutImage {OBJECT} at (13,0) size 131x24 [border: none (12px solid #000000) none]
+      LayoutInline {OBJECT} at (0,0) size 90x15
     LayoutBlockFlow (floating) {DIV} at (0,0) size 144x24 [border: none (12px solid #FF0000) none (12px solid #000000)]
     LayoutBlockFlow {DIV} at (0,0) size 144x24 [border: none (24px solid #FFFF00)]
+layer at (138,2712) size 90x30 backgroundClip at (0,0) size 800x600 clip at (0,0) size 800x600
+  LayoutEmbeddedObject {OBJECT} at (54,0) size 90x30
 layer at (84,2784) size 144x24 backgroundClip at (0,0) size 800x600 clip at (0,0) size 800x600
   LayoutBlockFlow (relative positioned) {DIV} at (0,0) size 144x24 [bgcolor=#000000]
 layer at (96,2784) size 120x24 backgroundClip at (0,0) size 800x600 clip at (0,0) size 800x600

$ cat out/gn/layout-test-results/fast/css/acid2-pixel-diff.txt 
--- /usr/local/google/home/lukasza/src/chromium3/src/out/gn/layout-test-results/fast/css/acid2-pixel-expected.txt
+++ /usr/local/google/home/lukasza/src/chromium3/src/out/gn/layout-test-results/fast/css/acid2-pixel-actual.txt
@@ -58,11 +58,11 @@
 layer at (84,144) size 144x24
   LayoutBlockFlow (positioned) {DIV} at (48,72) size 144x24 [bgcolor=#FF0000]
     LayoutBlockFlow {DIV} at (0,0) size 144x0
-      LayoutInline {OBJECT} at (0,0) size 131x15
-        LayoutInline {OBJECT} at (0,0) size 131x15
-          LayoutImage {OBJECT} at (13,0) size 131x24 [border: none (12px solid #000000) none]
+      LayoutInline {OBJECT} at (0,0) size 90x15
     LayoutBlockFlow (floating) {DIV} at (0,0) size 144x24 [border: none (12px solid #FF0000) none (12px solid #000000)]
     LayoutBlockFlow {DIV} at (0,0) size 144x24 [border: none (24px solid #FFFF00)]
+layer at (138,144) size 90x30
+  LayoutEmbeddedObject {OBJECT} at (54,0) size 90x30
 layer at (84,216) size 144x24
   LayoutBlockFlow (relative positioned) {DIV} at (0,0) size 144x24 [bgcolor=#000000]
 layer at (96,216) size 120x24




 

Comment 1 by nasko@chromium.org, Sep 27 2017

Cc: nasko@chromium.org
Owner: arthurso...@chromium.org
Reassigning to arthursonzogni@, as I won't have time to look into these failures soon.
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 28 2017

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

commit d100bb101d90652a502b50edc67b4676c870cce4
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Thu Sep 28 02:43:04 2017

Exclusions for PlzNavigate issues found on the Site Isolation Win bot.

NOTRY=true

Bug:  769502 
Bug:  769508 
Change-Id: Ib894feba443f5bbfbcd133f0ad73b9dd12e962ab
Tbr: alexmos@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/688223
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504875}
[modify] https://crrev.com/d100bb101d90652a502b50edc67b4676c870cce4/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 5 2018

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

commit 16295fe01a1264e488dc0f7837f53e6f01e74743
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Mon Mar 05 22:18:14 2018

Remove test exceptions for tests that have "healed" themselves

Bug: 477150,  789781 ,  769508 ,  661725 
Bug: 611232,  745881 ,  801992 
Change-Id: If90fb04e97513b99716afd844a2a77ca0905ab3d
Reviewed-on: https://chromium-review.googlesource.com/942316
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540958}
[modify] https://crrev.com/16295fe01a1264e488dc0f7837f53e6f01e74743/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

Status: Assigned (was: Untriaged)
AFAICT we can't yet close this bug, because there are remaining failures in fast/css/acid2.html
Blocking: 477150
Labels: -Pri-3 Test-Layout Pri-2
Cc: mstensho@chromium.org
Any updates here? Failing acid makes me nervous, and now --site-per-process is apparently enabled by default for tests.
Hmm... doesn't seem to fail at all:

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

(I noticed that it didn't fail locally, so I wanted to test this)

Is it flaky, or what's the issue?
I don't remember what was the failure mode back in March (in #c4), but I think the test wasn't flaky (I think back in March I tried to run each of the tests from the old FlagExpectations/site-per-process).  If the test passes now, I think it is a good idea to remove test expectations for this test and monitor the bots (hoping that the test continues to non-flakily pass everywhere, including more exotic bots).  I've also commented on your CL @ https://crrev.com/c/1345010 (thanks for trying out the test and uploading the CL!).
Thank you! I'm attempting to land the CL now. Time to close this bug, then (when the CL lands)?
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 21

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

commit b9df950d88596228555ec021a8298dd723da9b1b
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Wed Nov 21 10:28:55 2018

Acid2 is passing - don't mark it as failing.

Bug:  769508 
Change-Id: I32e58b403f2f0e279091b7209aedaa69c4e9b5cb
Reviewed-on: https://chromium-review.googlesource.com/c/1345010
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609986}
[modify] https://crrev.com/b9df950d88596228555ec021a8298dd723da9b1b/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/b9df950d88596228555ec021a8298dd723da9b1b/third_party/WebKit/LayoutTests/VirtualTestSuites

NextAction: 2018-11-26
RE: Time to close this bug, then (when the CL lands)?

Yes, I think so, but maybe let's wait a day or two to see if the flakiness dashboard picks anything up?

FWIW, I've tried looking at https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_layout_tests&tests=fast%2Fcss%2Facid2.html and no failures are reported, but the test has been enabled very recently - not sure how many runs it got on the bots at this point.
The NextAction date has arrived: 2018-11-26
Status: Fixed (was: Assigned)

Sign in to add a comment