Closed
Bug 1000722
Opened 10 years ago
Closed 9 years ago
Many B2G reftests fail after enabling OOP, because partial drawWindow calls are drawing too small an area
Categories
(Testing :: Reftest, defect)
Tracking
(firefox31 unaffected, firefox32 fixed, firefox33 fixed, b2g-v1.4 unaffected, b2g-v2.0 fixed, b2g-v2.1 fixed)
RESOLVED
FIXED
mozilla33
Tracking | Status | |
---|---|---|
firefox31 | --- | unaffected |
firefox32 | --- | fixed |
firefox33 | --- | fixed |
b2g-v1.4 | --- | unaffected |
b2g-v2.0 | --- | fixed |
b2g-v2.1 | --- | fixed |
People
(Reporter: vichen, Assigned: u459114)
References
Details
Attachments
(4 files, 21 obsolete files)
112.82 KB,
image/png
|
Details | |
8.10 KB,
image/png
|
Details | |
1.35 KB,
patch
|
u459114
:
review+
|
Details | Diff | Splinter Review |
238.55 KB,
patch
|
u459114
:
review+
|
Details | Diff | Splinter Review |
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
Reporter | ||
Comment 1•10 years ago
|
||
dbaron, As the patches between d823e000d59c and d097b88f2b72 are submitted by you, would please check first.
Flags: needinfo?(dbaron)
Reporter | ||
Comment 2•10 years ago
|
||
(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•10 years ago
|
||
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.)
Flags: needinfo?(dbaron)
Comment 4•9 years ago
|
||
Bug 1018381 seems to cover the same problem as this bug.
Comment 5•9 years ago
|
||
Also, bug 922680 comment 97 links to some useful history on how we got to this point, I think.
Updated•9 years ago
|
Summary: Many reftests fails after enable OOP → Many B2G reftests fail after enabling OOP, because partial drawWindow calls are drawing too small an area
Comment 6•9 years ago
|
||
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•9 years ago
|
||
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
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
Assignee | ||
Comment 10•9 years ago
|
||
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">
Assignee | ||
Comment 11•9 years ago
|
||
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?
Flags: needinfo?(pchang)
Assignee | ||
Comment 12•9 years ago
|
||
Assignee | ||
Comment 13•9 years ago
|
||
Comment 14•9 years ago
|
||
(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
Flags: needinfo?(pchang)
Assignee | ||
Comment 15•9 years ago
|
||
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.
Assignee | ||
Comment 16•9 years ago
|
||
Assignee | ||
Comment 17•9 years ago
|
||
Assignee | ||
Comment 18•9 years ago
|
||
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
Assignee | ||
Comment 20•9 years ago
|
||
Try result https://tbpl.mozilla.org/?tree=Try&rev=ab277942fbb5
Assignee | ||
Comment 21•9 years ago
|
||
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
Attachment #8442002 -
Flags: review?(dbaron)
Attachment #8442002 -
Attachment is patch: true
Assignee | ||
Comment 22•9 years ago
|
||
Full try https://tbpl.mozilla.org/?tree=Try&rev=fe63872d22ce
Assignee | ||
Comment 23•9 years ago
|
||
(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.
Assignee | ||
Comment 24•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=fe63872d22ce Orange in R12 is an new one after apply this patch.
Assignee | ||
Comment 25•9 years ago
|
||
(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•9 years ago
|
||
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.
Attachment #8442002 -
Flags: review?(dbaron) → review+
Comment 27•9 years ago
|
||
(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?
Assignee | ||
Comment 28•9 years ago
|
||
(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");
Assignee | ||
Comment 29•9 years ago
|
||
carry r+
Attachment #8441998 -
Attachment is obsolete: true
Attachment #8442002 -
Attachment is obsolete: true
Attachment #8442612 -
Flags: review+
Assignee | ||
Comment 30•9 years ago
|
||
Assignee | ||
Comment 31•9 years ago
|
||
Assignee | ||
Comment 32•9 years ago
|
||
Attachment #8442616 -
Attachment is patch: true
Assignee | ||
Comment 33•9 years ago
|
||
Assignee | ||
Comment 34•9 years ago
|
||
(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•9 years ago
|
||
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.
Assignee | ||
Comment 36•9 years ago
|
||
Correct merge conflict
Attachment #8442618 -
Attachment is obsolete: true
Assignee | ||
Comment 37•9 years ago
|
||
(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
Assignee | ||
Comment 38•9 years ago
|
||
Attachment #8442691 -
Attachment is obsolete: true
Assignee | ||
Comment 39•9 years ago
|
||
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
Assignee | ||
Comment 40•9 years ago
|
||
Attachment #8442615 -
Attachment is obsolete: true
Assignee | ||
Comment 41•9 years ago
|
||
Attachment #8442725 -
Attachment is obsolete: true
Attachment #8442750 -
Attachment is patch: true
Assignee | ||
Comment 42•9 years ago
|
||
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
Assignee | ||
Comment 43•9 years ago
|
||
(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.
Flags: needinfo?(ahalberstadt)
Assignee | ||
Comment 44•9 years ago
|
||
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•9 years ago
|
||
(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).
Flags: needinfo?(ahalberstadt)
Assignee | ||
Comment 47•9 years ago
|
||
(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.
Assignee | ||
Comment 48•9 years ago
|
||
Attachment #8442752 -
Attachment is obsolete: true
Assignee | ||
Comment 49•9 years ago
|
||
Attachment #8443229 -
Attachment is obsolete: true
Assignee | ||
Comment 50•9 years ago
|
||
Attachment #8443240 -
Attachment is obsolete: true
Attachment #8443312 -
Attachment is patch: true
Assignee | ||
Comment 51•9 years ago
|
||
Disable more xul test case
Attachment #8443312 -
Attachment is obsolete: true
Assignee | ||
Comment 52•9 years ago
|
||
Try result: https://tbpl.mozilla.org/?tree=Try&rev=6122be44353e
Attachment #8443384 -
Attachment is obsolete: true
Attachment #8442616 -
Flags: review?(ahalberstadt)
Attachment #8442617 -
Flags: review?(ahalberstadt)
Attachment #8442750 -
Flags: review?(ahalberstadt)
Assignee | ||
Comment 53•9 years ago
|
||
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.
Attachment #8443390 -
Flags: review?(ahalberstadt)
Assignee | ||
Comment 54•9 years ago
|
||
Full try https://tbpl.mozilla.org/?tree=Try&rev=3bc428b79534
Assignee | ||
Comment 55•9 years ago
|
||
Oops...more xul found :(
Attachment #8443390 -
Attachment is obsolete: true
Attachment #8443390 -
Flags: review?(ahalberstadt)
Attachment #8443428 -
Flags: review?(ahalberstadt)
Comment 56•9 years ago
|
||
C.J, could you fold all the backout patches into one? It would make it a lot easier to review.
Flags: needinfo?(cku)
Assignee | ||
Comment 58•9 years ago
|
||
Attachment #8442616 -
Attachment is obsolete: true
Attachment #8442617 -
Attachment is obsolete: true
Attachment #8442750 -
Attachment is obsolete: true
Attachment #8443428 -
Attachment is obsolete: true
Attachment #8442616 -
Flags: review?(ahalberstadt)
Attachment #8442617 -
Flags: review?(ahalberstadt)
Attachment #8442750 -
Flags: review?(ahalberstadt)
Attachment #8443428 -
Flags: review?(ahalberstadt)
Assignee | ||
Comment 59•9 years ago
|
||
remove trail space
Attachment #8443491 -
Attachment is obsolete: true
Assignee | ||
Comment 60•9 years ago
|
||
Comment on attachment 8443491 [details] [diff] [review] Part 2: rollback disable test in bug 981477 Combined patch
Attachment #8443491 -
Flags: review?(ahalberstadt)
Attachment #8443503 -
Flags: review?(ahalberstadt)
Assignee | ||
Comment 61•9 years ago
|
||
add email and reviewer in patch
Attachment #8443503 -
Attachment is obsolete: true
Attachment #8443503 -
Flags: review?(ahalberstadt)
Attachment #8443514 -
Flags: review?(ahalberstadt)
Attachment #8441556 -
Attachment description: 2 pixels border → Diagram: 2 pixels border
Attachment #8441570 -
Attachment description: invalid_area_and_capture_area.png → Diagram: Diff of invalid area and captured area
Comment 62•9 years ago
|
||
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?
Attachment #8443514 -
Flags: review?(ahalberstadt) → review+
Assignee | ||
Comment 63•9 years ago
|
||
(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
Assignee | ||
Comment 64•9 years ago
|
||
(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.
Flags: needinfo?(ahalberstadt)
Comment 65•9 years ago
|
||
Oh right, good catch! Here's a new one: https://tbpl.mozilla.org/?tree=Try&rev=7490caf3ad02
Flags: needinfo?(ahalberstadt)
Assignee | ||
Comment 66•9 years ago
|
||
(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
Assignee | ||
Comment 67•9 years ago
|
||
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
Assignee | ||
Comment 68•9 years ago
|
||
All pass in the 1st try Trigger the 2nd try https://tbpl.mozilla.org/?tree=Try&rev=bdc5c7079566
Comment 69•9 years ago
|
||
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•9 years ago
|
||
That looks really green! I think we are good to land.
Attachment #8443881 -
Attachment description: Part 2: fix failure in try 7490caf3ad02 → Part 2: rollback disable test in bug 981477
Attachment #8443881 -
Flags: review+
Attachment #8443514 -
Attachment is obsolete: true
Keywords: checkin-needed
Comment 71•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e56739ee253b https://hg.mozilla.org/integration/mozilla-inbound/rev/5f0396c873f3
Keywords: checkin-needed
Updated•9 years ago
|
Attachment #8443491 -
Flags: review?(ahalberstadt)
Comment 72•9 years ago
|
||
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
Assignee | ||
Comment 73•9 years ago
|
||
<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.
Assignee | ||
Comment 74•9 years ago
|
||
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?
Flags: needinfo?(ahalberstadt)
Comment 75•9 years ago
|
||
That sounds good to me. Better to land this before more stuff breaks on us.
Flags: needinfo?(ahalberstadt)
Assignee | ||
Comment 76•9 years ago
|
||
Change outer-svg-border-and-padding-01.svg back to random-if
Attachment #8445284 -
Flags: review+
Attachment #8443881 -
Attachment is obsolete: true
Attachment #8445284 -
Attachment description: Part 2': rollback disable test in bug 981477 → Part 2: rollback disable test in bug 981477
Assignee | ||
Comment 77•9 years ago
|
||
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
Assignee | ||
Comment 78•9 years ago
|
||
Try result https://tbpl.mozilla.org/?tree=Try&rev=0768befa7ea5
Keywords: checkin-needed
Comment 79•9 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/489c599d5c40 https://hg.mozilla.org/integration/b2g-inbound/rev/861fbef8e95c
Keywords: checkin-needed
Comment 80•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/489c599d5c40 https://hg.mozilla.org/mozilla-central/rev/861fbef8e95c
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Comment 81•9 years ago
|
||
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•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/7130f4a8e2bb https://hg.mozilla.org/releases/mozilla-aurora/rev/019279c9a206
status-b2g-v1.4:
--- → unaffected
status-b2g-v2.0:
--- → fixed
status-b2g-v2.1:
--- → fixed
status-firefox31:
--- → unaffected
status-firefox32:
--- → fixed
status-firefox33:
--- → fixed
Comment 83•9 years ago
|
||
Thanks for getting this fixed and landed.
You need to log in
before you can comment on or make changes to this bug.
Description
•