1000722 - Many B2G reftests fail after enabling OOP, because partial drawWindow calls are drawing too small an area

Status: RESOLVED FIXED

(Testing :: Reftest, defect)

Description

[Vincent Chen [:vichen]]

Follow: Bug 922680 Comment 106
After enable OOP on B2G reftest, there are many reftests fail. After regress among patches (http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5405d6f4e3c6&tochange=92dd49bd29b6), the fails is between the two patches.

Fail: Apr 15 13:08:13 c7c4c0d0f613 https://tbpl.mozilla.org/?tree=Try&rev=d823e000d59c
Pass: Apr 15 13:08:13 92dd49bd29b6 https://tbpl.mozilla.org/?tree=Try&rev=d097b88f2b72 

Please focus on R4-R15

Comment 1

[Vincent Chen [:vichen]]

dbaron,
As the patches between d823e000d59c and d097b88f2b72 are submitted by you, would please check first.

Comment 2

[Vincent Chen [:vichen]]

(In reply to Vincent Chen [:vichen] from comment #1)
> dbaron,
> As the patches between d823e000d59c and d097b88f2b72 are submitted by you,
> would please check first.

Update: patches between c7c4c0d0f613 and 92dd49bd29b6

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Those patches re-enabled a feature of the reftest harness that was accidentally broken; see bug 995410.  With that bug fixed, the reftest harness again uses partial drawWindow calls to redraw only the invalid region, instead of redrawing the entire screen.

The other bugs in that range (bug 995661 and bug 995721) fixed bugs where partial-window drawWindow calls were broken.  It looks like this set of failures shows another way in which a partial window canvas drawWindow call is broken.  Many of the failures in comment 0's "Fail" line are the same tests that I saw on other platforms when testing bug 995410 without bug 995661 or bug 995721.

In this case, the problem looks somewhat different from bug 995661 and bug 995721, though.  It appears that the area being redrawn is slightly too small.  (layout/reftests/transform/dynamic-addremove-1a.html is perhaps a good example test to debug; it's one of the failures in R15.  It shows failures around the edge of the box that moved, and also around the edge of the window.)

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Bug 1018381 seems to cover the same problem as this bug.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Also, bug 922680 comment 97 links to some useful history on how we got to this point, I think.

Comment 6

[Andrew Halberstadt [:ahal]]

Once this is fixed we should re-enable all the tests that were disabled by bug 981477 and re-triage them on try. To do that we can backout all the patches that landed in that bug, which in order they should be backed out in are:

https://hg.mozilla.org/mozilla-central/rev/e1d54d299ab3
https://hg.mozilla.org/mozilla-central/rev/d4d29af7b3d0
https://hg.mozilla.org/mozilla-central/rev/c4eebcb1c390
https://hg.mozilla.org/mozilla-central/rev/b36a923d9b4a
https://hg.mozilla.org/mozilla-central/rev/f05fbe6261ac

Comment 7

[Andrew Halberstadt [:ahal]]

Actually that middle changeset looks like it shouldn't be backed out.. so:

https://hg.mozilla.org/mozilla-central/rev/e1d54d299ab3
https://hg.mozilla.org/mozilla-central/rev/d4d29af7b3d0
https://hg.mozilla.org/mozilla-central/rev/b36a923d9b4a
https://hg.mozilla.org/mozilla-central/rev/f05fbe6261ac

Comment 8

[u459114]

Take it

Comment 9

[u459114]

update what I found so far.

By looking over comment at bug 100722 and bug 1018381. Here are actions:
1. Remove all "random-if" failure type in tests/layout/reftests/flexbox
2. submit a try run
   Try result: https://tbpl.mozilla.org/?tree=Try&rev=f7c9eef86098
3. Many failures(because of #1). I think flexbox-dyn-changePadding-1a is 
   really trivial one. Look into this failure first.
   Pass criteria: == flexbox-dyn-changePadding-1a.xhtml flexbox-dyn-changePadding-1-ref.xhtml
4. Look into TEST-UNEXPECTED-FAIL of flexbox-dyn-changePadding-1a.xhtml
   https://tbpl.mozilla.org/php/getParsedLog.php?id=41871625&tree=Try&full=1#error4

   From the log:
   06:44:40     INFO -  REFTEST INFO | Saved log: [CONTENT] Rect: 8 8 108 108
   
   I "think", if the value of Rect change to  {8, 8, 110, 110}, thing should goes right.
   There is no build env at home. Need to confirm this assumption after entering 
   office tomorrow.

   Rect value is set here:
   http://dxr.mozilla.org/mozilla-central/source/layout/tools/reftest/reftest-content.js#946
   Suspect: round problem here:
   http://dxr.mozilla.org/mozilla-central/source/layout/tools/reftest/reftest-content.js#942

Comment 10

[u459114]

My description in comment 9 is not correct.
Size of <div id="tweakMe"> is (100, 100)

06:44:40     INFO -  REFTEST INFO | Saved log: DoDrawWindow 8,8,100,100
ctx.drawWindow(gContainingWindow,8,8,100,100,"rgb(255,255,255)", gDrawWindowFlags);

Size of this drawing is correct. Need look into gecko side to figure out why 2 pixels miss on right/ bottom boundary of <div id="tweakMe">

Comment 11

[u459114]

peter,
since you are working on bug 846421, in which you have to trace into drawWindow call flow in gecko. Do you have any idea on this problem?

Comment 12

[u459114]

Attachment: Diagram: 2 pixels border

Comment 13

[u459114]

Attachment: Diagram: Diff of invalid area and captured area

Comment 14

[Peter Chang[:pchang]]

(In reply to C.J. Ku[:cjku] from comment #11)
> peter,
> since you are working on bug 846421, in which you have to trace into
> drawWindow call flow in gecko. Do you have any idea on this problem?

It would be good to dump the root frame during the drawWindow call.
You can define DEBUG_FRAME_DUMP from [1] and add "rootFrame->DumpFrameTree()" before [2].

[1]http://dxr.mozilla.org/mozilla-central/source/layout/generic/nsFrameList.h#16
[2]http://dxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp?from=nsPresShell.cpp&case=true#4836

Comment 15

[u459114]

Ahifting two pixels while drawWindow
  http://dxr.mozilla.org/mozilla-central/source/layout/tools/reftest/reftest.js#1454
  ctx.drawWindow(gContainingWindow, x + 2, y +2, w, h, "rgb(255,255,255)",

Before change
https://tbpl.mozilla.org/?tree=Try&rev=f7c9eef86098
R10 failed.

After change
https://tbpl.mozilla.org/?tree=Try&rev=4c9f1a75ec28
R10 pass.

Comment 16

[u459114]

Attachment: Part 1: Enable random-if test in flexbox

Comment 17

[u459114]

Attachment: Part 2: Set margin/padding of remote iframe to 0

Comment 18

[u459114]

attachment 8441998 [details] [diff] [review]: remove random-if failure type for testing purpose
attachment 8442000 [details] [diff] [review]: set margin and padding value of the remote iframe as 0. And set border as none at the same time

Try result
https://tbpl.mozilla.org/?tree=Try&rev=bd09c215e547

Comment 19

[u459114]

Attachment: Part 2: Set margin/padding of remote iframe to 0

Correct typo

Comment 20

[u459114]

Try result
https://tbpl.mozilla.org/?tree=Try&rev=ab277942fbb5

Comment 21

[u459114]

Comment on attachment 8442002 [details] [diff] [review]
Part 2: Set margin/padding of remote iframe to 0

Hi dbaron,
I think the problem cause by padding of remote-iframe, which is created in OOP case only. 
After applying attachment 8441998 [details] [diff] [review] and attachment 8442002 [details] [diff] [review], all test cases in flexbox passed at local side.

Please help to review this patch, thanks

Comment 22

[u459114]

Full try
https://tbpl.mozilla.org/?tree=Try&rev=fe63872d22ce

Comment 23

[u459114]

(In reply to C.J. Ku[:cjku] from comment #22)
> Full try
> https://tbpl.mozilla.org/?tree=Try&rev=fe63872d22ce
Most Red color are caused by turning on frame tree dump
"Output exceeded 52428800 bytes, remaining output has been truncated (output was 52435769 bytes)"

Otherwise, test result looks good.

Comment 24

[u459114]

https://tbpl.mozilla.org/?tree=Try&rev=fe63872d22ce

Orange in R12 is an new one after apply this patch.

Comment 25

[u459114]

(In reply to C.J. Ku[:cjku] from comment #24)
> https://tbpl.mozilla.org/?tree=Try&rev=fe63872d22ce
> 
> Orange in R12 is an new one after apply this patch.

Looks like this patch make this fail-if test case pass.
fails-if(B2G) == menclose-2-circle.html menclose-2-circle-ref.html
INFO -  REFTEST TEST-UNEXPECTED-PASS | http://10.0.2.2:8888/tests/layout/reftests/mathml/menclose-2-circle.html | image comparison (==)

Comment 26

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8442002 [details] [diff] [review]
Part 2: Set margin/padding of remote iframe to 0

r=dbaron.  Thanks.

(It seems a bit curious that this causes a problem only for the OOP case, though.)

Once this is landed, the random-if() and skip-if() annotations due to this bug should also be removed.

Comment 27

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to David Baron [:dbaron] (busy May 31-June 15) (UTC-7) from comment #26)
> (It seems a bit curious that this causes a problem only for the OOP case,
> though.)

Perhaps that's because there's a bug in a codepath used by drawWindow that's incorrectly subtracting this offset?

Comment 28

[u459114]

(In reply to David Baron [:dbaron] (busy May 31-June 15) (UTC-7) from comment #26)
> Comment on attachment 8442002 [details] [diff] [review]
> Part 2: Set margin/padding of remote iframe to 0
> 
> r=dbaron.  Thanks.
Will do. Thank you for reviewing
> (It seems a bit curious that this causes a problem only for the OOP case,
> though.)
> 
> Once this is landed, the random-if() and skip-if() annotations due to this
> bug should also be removed.
That's a thing that I am going to figure out.
I suspect that is because default css value of iframe and xul:browser is different 
OOP
  gBrowser = gContainingWindow.document.createElementNS(XHTML_NS, "iframe");
Non-OOP
  gBrowser = gContainingWindow.document.createElementNS(XUL_NS, "xul:browser");

Comment 29

[u459114]

Attachment: Part 1: Fix wrong snapshot area

carry r+

Comment 30

[u459114]

Attachment: Part 2: backout e1d54d299ab3

Comment 31

[u459114]

Attachment: Part 3: backout d4d29af7b3d0

Comment 32

[u459114]

Attachment: Part 4: backout b36a923d9b4a

Comment 33

[u459114]

Attachment: Part 5: backout f05fbe6261ac

Comment 34

[u459114]

(In reply to Andrew Halberstadt [:ahal] from comment #7)
> Actually that middle changeset looks like it shouldn't be backed out.. so:
> 
> https://hg.mozilla.org/mozilla-central/rev/e1d54d299ab3
> https://hg.mozilla.org/mozilla-central/rev/d4d29af7b3d0
> https://hg.mozilla.org/mozilla-central/rev/b36a923d9b4a
> https://hg.mozilla.org/mozilla-central/rev/f05fbe6261ac
There are two conflicts while backout f05fbe6261ac, had merge back into backout_f05fbe6261ac

Try result:
https://tbpl.mozilla.org/?tree=Try&rev=d7023aa6b9bc

Comment 35

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Comment on attachment 8442618 [details] [diff] [review]
Part 5: backout f05fbe6261ac

Review of attachment 8442618 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/reftests/flexbox/reftest.list
@@ +118,2 @@
>  == flexbox-inlinecontent-horiz-3a.xhtml flexbox-inlinecontent-horiz-3-ref.xhtml
> +<<<<<<< local

This patch does not merge correctly. That's why it causes bug 1027501. However bug 1027501 is a potential error, it still needs to be fixed.

::: layout/xul/reftest/reftest.list
@@ +1,1 @@
> +<<<<<<< local

Ditto.

Comment 36

[u459114]

Attachment: Part 5: backout f05fbe6261ac

Correct merge conflict

Comment 37

[u459114]

(In reply to Ting-Yu Lin [:TYLin] (UTC+8) from comment #35)
> Comment on attachment 8442618 [details] [diff] [review]
> Part 5: backout f05fbe6261ac
> 
> Review of attachment 8442618 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/reftests/flexbox/reftest.list
> @@ +118,2 @@
> >  == flexbox-inlinecontent-horiz-3a.xhtml flexbox-inlinecontent-horiz-3-ref.xhtml
> > +<<<<<<< local
> 
> This patch does not merge correctly. That's why it causes bug 1027501.
> However bug 1027501 is a potential error, it still needs to be fixed.
> 
> ::: layout/xul/reftest/reftest.list
> @@ +1,1 @@
> > +<<<<<<< local
> 
> Ditto.

Thinks!! 
Fix merge conflict and submit a new try again
https://tbpl.mozilla.org/?tree=Try&rev=07cb963d9951

Comment 38

[u459114]

Attachment: Part 5: backout f05fbe6261ac

Comment 39

[u459114]

reftest-remote only try result:
https://tbpl.mozilla.org/?tree=Try&rev=95635d80c450

full reftest try result(base on the latest version)
https://tbpl.mozilla.org/?tree=Try&rev=b360e1cc611c

Comment 40

[u459114]

Attachment: Part 2: backout e1d54d299ab3

Comment 41

[u459114]

Attachment: Part 5: backout f05fbe6261ac

Comment 42

[u459114]

Try result of rolling back random-if patches in bug 981477
https://tbpl.mozilla.org/?tree=Try&rev=b360e1cc611c 

1. A skip item which is also fixed by this patch
a. skip-if(B2G&&browserIsRemote) == box-ordinal-with-out-of-flow-1.html ...

2. Need to file a bug to trace status
layout/reftests/bugs/reftest.list
[Unknown] random-if(Android||(B2G&&browserIsRemote)) == 728983-1.html 728983-1-ref.html 
[scrollbar] random-if(B2G&&browserIsRemote) == 960822.html 960822-ref.html..
[scrollbar] random-if(B2G&&browserIsRemote) == 381497-n.html 381497-f.html ..

3. There are associated bugs on XP. But we may still need to file issues for B2G
[Bug 943863 XP] random-if(B2G&&browserIsRemote) == box-sizing-replaced-002.xht ..
[Bug 935850 XP] random-if(B2G&&browserIsRemote) == box-sizing-replaced-003.xht ..
[Bug 629416 XP] random-if(B2G&&browserIsRemote) == 413928-1.html 413928-1-ref.html
[Bug 629416 XP] random-if(B2G&&browserIsRemote) == 413928-2.html 413928-2-ref.html

4. Know issues which still not assign
[Bug 982547] random-if(Android) skip-if(B2G&&browserIsRemote) == box-sizing-replaced-001.xht..
[Bug 988759] random-if(B2G&&browserIsRemote) == multipleinsertionpoints-ref2.xhtml ...
[Bug 988763] random-if(B2G&&browserIsRemote) == referenced-from-binding-01.html ...
[Bug 988765] random-if(B2G&&browserIsRemote) == 166591-dynamic-1.html ...
[Bug 972697] skip-if(B2G&&browserIsRemote) include font-inflation/reftest.list

Comment 43

[u459114]

(In reply to Andrew Halberstadt [:ahal] from comment #7)
> Actually that middle changeset looks like it shouldn't be backed out.. so:
> 
> https://hg.mozilla.org/mozilla-central/rev/e1d54d299ab3
> https://hg.mozilla.org/mozilla-central/rev/d4d29af7b3d0
> https://hg.mozilla.org/mozilla-central/rev/b36a923d9b4a
> https://hg.mozilla.org/mozilla-central/rev/f05fbe6261ac

Hi ahal,
Back out these four patches re-enable many failed test cases which are not fixed yet. For example, XUL test case and font-inflation ones.

I am going to roll back these four patches and manually disable failed ones in this bug, may I have your opinion here? Thanks.

Comment 44

[u459114]

These four test cases are also pass after this fix

layout/reftests/mathml/reftest.list
  fails-if(B2G&&browserIsRemote) == tablespacing-4.html tablespacing-4-ref.html
  fails-if(B2G&&browserIsRemote) == tablespacing-5.html tablespacing-5-ref.html
  fails-if(B2G&&browserIsRemote) == tablespacing-5a.html tablespacing-5a-ref.html
  fails-if(B2G&&browserIsRemote) == tablespacing-6.html tablespacing-6-ref.html

Comment 46

[Andrew Halberstadt [:ahal]]

(In reply to C.J. Ku[:cjku] from comment #43)
> Hi ahal,
> Back out these four patches re-enable many failed test cases which are not
> fixed yet. For example, XUL test case and font-inflation ones.
> 
> I am going to roll back these four patches and manually disable failed ones
> in this bug, may I have your opinion here? Thanks.

Yes I think that's the best way to do it. Though you might as well land part 1 now (assuming it doesn't cause any orange).

Comment 47

[u459114]

(In reply to Andrew Halberstadt [:ahal] from comment #46)
> Yes I think that's the best way to do it. Though you might as well land part
> 1 now (assuming it doesn't cause any orange).
Unfortunately, it does cause some orange.
For example, these four reftest failed
fails-if(B2G&&browserIsRemote) == tablespacing-4.html tablespacing-4-ref.html
fails-if(B2G&&browserIsRemote) == tablespacing-5.html tablespacing-5-ref.html
fails-if(B2G&&browserIsRemote) == tablespacing-5a.html tablespacing-5a-ref.html
fails-if(B2G&&browserIsRemote) == tablespacing-6.html tablespacing-6-ref.html

I need to remove some "fails-if(B2G&&browserIsRemote)" before check in this patch.

Comment 48

[u459114]

Attachment: Part 5: backout f05fbe6261ac

Comment 49

[u459114]

Attachment: Part 5: backout f05fbe6261ac

Comment 50

[u459114]

Attachment: Part 5: backout f05fbe6261ac

Comment 51

[u459114]

Attachment: Part 5: backout f05fbe6261ac

Disable more xul test case

Comment 52

[u459114]

Attachment: Part 5: backout f05fbe6261ac

Try result:
https://tbpl.mozilla.org/?tree=Try&rev=6122be44353e

Comment 53

[u459114]

Comment on attachment 8443390 [details] [diff] [review]
Part 5: backout f05fbe6261ac

Hi ahal,
Please help to review these patch, thanks.

Except attachment 8443390 [details] [diff] [review], all the other tree are just roll back change in bug 981477.

Comment 54

[u459114]

Full try
https://tbpl.mozilla.org/?tree=Try&rev=3bc428b79534

Comment 55

[u459114]

Attachment: Part 5: backout f05fbe6261ac

Oops...more xul found :(

Comment 56

[Andrew Halberstadt [:ahal]]

C.J, could you fold all the backout patches into one? It would make it a lot easier to review.

Comment 57

[u459114]

Sure, doing, wait a minute :)

Comment 58

[u459114]

Attachment: Part 2: rollback disable test in bug 981477

Comment 59

[u459114]

Attachment: Part 2: rollback disable test in bug 981477

remove trail space

Comment 60

[u459114]

Comment on attachment 8443491 [details] [diff] [review]
Part 2: rollback disable test in bug 981477

Combined patch

Comment 61

[u459114]

Attachment: Part 2: rollback disable test in bug 981477

add email and reviewer in patch

Comment 62

[Andrew Halberstadt [:ahal]]

Comment on attachment 8443514 [details] [diff] [review]
Part 2: rollback disable test in bug 981477

Review of attachment 8443514 [details] [diff] [review]:
-----------------------------------------------------------------

This is great, thanks! I did a last try push here:
https://tbpl.mozilla.org/?tree=Try&rev=ba91e89c106e

We probably shouldn't land this until each job has been re-triggered at least 5 times to see if there are any intermittents still hiding around.

::: layout/reftests/mathml/reftest.list
@@ +101,5 @@
>  == mpadded-1-2.html mpadded-1-2-ref.html
>  == mpadded-6.html mpadded-6-ref.html
> +fails-if(B2G) == mpadded-7.html mpadded-7-ref.html # B2G: slight character width variation
> +fails-if(B2G) == mpadded-8.html mpadded-8-ref.html # B2G: slight character width variation
> +fails-if(B2G) == mpadded-9.html mpadded-9-ref.html # B2G: slight character width variation

I assume these were here before? Otherwise are you sure it fails in the non-oop case too?

Comment 63

[u459114]

(In reply to Andrew Halberstadt [:ahal] from comment #62)
> ::: layout/reftests/mathml/reftest.list
> @@ +101,5 @@
> >  == mpadded-1-2.html mpadded-1-2-ref.html
> >  == mpadded-6.html mpadded-6-ref.html
> > +fails-if(B2G) == mpadded-7.html mpadded-7-ref.html # B2G: slight character width variation
> > +fails-if(B2G) == mpadded-8.html mpadded-8-ref.html # B2G: slight character width variation
> > +fails-if(B2G) == mpadded-9.html mpadded-9-ref.html # B2G: slight character width variation
> 
> I assume these were here before? Otherwise are you sure it fails in the
> non-oop case too?
Yes, "fails-if(B2G)" is replaced by "random-if(B2G&&browserIsRemote)" at f05fbe6261ac, 
https://hg.mozilla.org/mozilla-central/rev/f05fbe6261ac#l41.57

Comment 64

[u459114]

(In reply to Andrew Halberstadt [:ahal] from comment #62)
> This is great, thanks! I did a last try push here:
> https://tbpl.mozilla.org/?tree=Try&rev=ba91e89c106e
ahal, I notice you didn't push attachment 8442612 [details] [diff] [review] in this try round. Without part 1, many test cases will be fail.

Comment 65

[Andrew Halberstadt [:ahal]]

Oh right, good catch! Here's a new one:
https://tbpl.mozilla.org/?tree=Try&rev=7490caf3ad02

Comment 66

[u459114]

(In reply to Andrew Halberstadt [:ahal] from comment #65)
> Oh right, good catch! Here's a new one:
> https://tbpl.mozilla.org/?tree=Try&rev=7490caf3ad02

R9
REFTEST TEST-UNEXPECTED-FAIL
layout/reftests/css-ui-valid/select/reftest.list
-random-if(Android||B2G) == textarea-dyn-disabled.html textarea-ref.html
+random-if(Android) == textarea-dyn-disabled.html textarea-ref.html
Still fail. "Part 1" does not fix this problem

R10
REFTEST TEST-UNEXPECTED-FAIL
layout/reftests/mathml/reftest.list
-random-if(B2G&&browserIsRemote) == multiscripts-1.html multiscripts-1-ref.html # B2G - slight height variation from font metrics
+== multiscripts-1.html multiscripts-1-ref.html # B2G - slight height variation from font metrics
Still fail. I am going to rollback this change.

R10
REFTEST TEST-UNEXPECTED-PASS
fails-if(B2G) == menclose-2-circle.html menclose-2-circle-ref.html
Looks like "Part 1" also fix this bug. I am going to remove fails-if

R17
Bug 906716 - Intermittent B2G emulator "timed out after 1000 seconds of no output"
I never meet this problem in my office time. I don't this failure is relative to applied patches

Comment 67

[u459114]

Attachment: Part 2: rollback disable test in bug 981477

https://tbpl.mozilla.org/?tree=Try&rev=540986eaaba3

random-if(Android||B2G) == textarea-dyn-disabled.html textarea-ref.html
random-if(B2G&&browserIsRemote) == multiscripts-1.html multiscripts-1-ref.html
== menclose-2-circle.html menclose-2-circle-ref.html

Comment 68

[u459114]

All pass in the 1st try
Trigger the 2nd try
https://tbpl.mozilla.org/?tree=Try&rev=bdc5c7079566

Comment 69

[Andrew Halberstadt [:ahal]]

Awesome! For future reference, you can re-trigger a job without doing a new push by clicking the blue '+' in the bottom left (after first clicking a job). I went ahead and re-triggered the jobs on the previous push a few more times.

Comment 70

[Andrew Halberstadt [:ahal]]

That looks really green! I think we are good to land.

Comment 71

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e56739ee253b
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f0396c873f3

Comment 72

[Ed Morley [:emorley]]

Backed out for causing a 90% failure rate in reftest-18 of form:
https://tbpl.mozilla.org/php/getParsedLog.php?id=42357000&tree=Mozilla-Inbound
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=reftest-18&fromchange=6dbb4388563b&tochange=7b174d47f3cc

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/4c3883620096
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/7b174d47f3cc

Comment 73

[u459114]

<svg xmlns="http://www.w3.org/2000/svg" version="1.1" class="reftest-wait"
     style="border: 10px solid lime; padding: 10px; background: lime;">
<rect id="r" width="50" height="50" fill="red"/>
<rect id="f" y="100" width="50" height="50" fill="red"/>
<rect x="100" y="100" width="50" height="50" fill="red"/>
<rect x="200" y="100" width="50" height="50" fill="red"/>
border: 10px + padding: 10px + 200(x pos) + 50(width) = 270

06:37:24     INFO -  REFTEST INFO | Saved log: [CONTENT] Rect: 20 20 270 170
06:37:24     INFO -  REFTEST INFO | Saved log: Updating canvas for invalidation
06:37:24     INFO -  REFTEST INFO | Saved log: DoDrawWindow 20,20,250,150

MozReftestInvalidate fired and invalidate area is correct.

This fail is not relative to Part 1. 
Part 2 remove random-if failure type cause this problem.

Comment 74

[u459114]

Hi Ahal,
REFTEST TEST-UNEXPECTED-FAIL | http://10.0.2.2:8888/tests/layout/reftests/svg/outer-svg-border-and-padding-01.svg | image comparison (==), max difference: 255, number of differing pixels: 10000
Return code: 1

Before we have attachment 8442612 [details] [diff] [review], this test is 100% fail. After that, this test case turn out to be random failed.
I prefer change failure type back to random-if and file another bug to fix this timing issue, how do you think?

Comment 75

[Andrew Halberstadt [:ahal]]

That sounds good to me. Better to land this before more stuff breaks on us.

Comment 76

[u459114]

Attachment: Part 2: rollback disable test in bug 981477

Change outer-svg-border-and-padding-01.svg  back to random-if

Comment 77

[u459114]

attachment 8445284 [details] [diff] [review] 
1. Rollback outer-svg-border-and-padding-01 reftest to random-if
2. remove trailer space of non-scaling-stroke-01.svg test item

Comment 78

[u459114]

Try result
https://tbpl.mozilla.org/?tree=Try&rev=0768befa7ea5

Comment 79

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/489c599d5c40
https://hg.mozilla.org/integration/b2g-inbound/rev/861fbef8e95c

Comment 80

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/489c599d5c40
https://hg.mozilla.org/mozilla-central/rev/861fbef8e95c

Comment 81

[Ryan VanderMeulen [:RyanVM]]

This is generally green on Aurora, but it's hitting timeouts in dynamic-use-02.svg ~20% of the time. I'm not sure if it's a consequence of tests shuffling around between chunks or something else, but it's interesting to me that it's not hitting trunk AFAICT.
https://tbpl.mozilla.org/?tree=Try&rev=e5b2f13043af

https://tbpl.mozilla.org/php/getParsedLog.php?id=42440680&tree=Try

Comment 82

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7130f4a8e2bb
https://hg.mozilla.org/releases/mozilla-aurora/rev/019279c9a206

Comment 83

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Thanks for getting this fixed and landed.