Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Test for bug 113934 fails to complete due to JS error

RESOLVED FIXED in Firefox 7

Status

()

Core
Document Navigation
P1
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: graememcc, Assigned: bz)

Tracking

({regression})

Trunk
mozilla8
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox7- fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
Running mochitest-chrome today, I noticed a couple of windows from the tests for bug 113934 hanging around until the end of the test suite. Handily, I had an older tree lying around to confirm this was new. Same when running the test in isolation.

We're now only running 3 tests out of 13, before hitting an uncaught JS exception - an NS_ERROR_FAILURE bubbling up from the call to compareCanvases in compareSnapshots.

hg bisect points to http://hg.mozilla.org/mozilla-central/rev/39bd0f6314c3 from bug 552605.

This isn't just a local problem, the slaves running m-o on tinderbox are hitting this too (at least linux debug, not checked opt or other platforms). 

Build logs before/after the push:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1309594430.1309598240.28946.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1309600613.1309605277.30375.gz

For the record, compareCanvases bales when checking the image sizes are equal, but it's presumably the change to WindowSnapshot in the cset above that's done the damage
(Assignee)

Comment 1

6 years ago
And of course someone made exceptions non-fatal in chrome tests.  :(

Joe, can you take a look at this please?
Blocks: 552605
tracking-firefox7: --- → ?
(Assignee)

Comment 2

6 years ago
Oh, I see, we used to never actually take the gWindowUtils codepath?

Is there a reason that throws on a size mismatch instead of just returning false?

Graeme, does making that change fix things here?
(Assignee)

Comment 3

6 years ago
Oh, and I requested tracking because we're missing part of our test suite now, unless either this bug is fixed or the change to WindowSnapshot is backed out.
(Reporter)

Comment 4

6 years ago
> Oh, I see, we used to never actually take the gWindowUtils codepath?
Yes, using an older tree, for this test at least, the comparison was only on the data:// URLs.
 
> Is there a reason that throws on a size mismatch instead of just returning
> false?
Looking at bug 387132, compareCanvases returns the number of differing pixels rather than true/false, so differing sizes would make that number meaningless.
(Assignee)

Comment 5

6 years ago
OK.  Does changing WindowSnapshot to check the imagedata sizes itself on that codepath and treat the result as "not equal" fix things?  I can post a patch for that tomorrow if you'd prefer.
(Assignee)

Updated

6 years ago
Duplicate of this bug: 670838
These are the actual values being compared:

(gdb) p (img1.mRawPtr)->GetSize()
$5 = {
  <mozilla::gfx::BaseSize<int,nsIntSize>> = {
    width = 1606389496, 
    height = 32767
  }, <No data fields>}
(gdb) p (img2.mRawPtr)->GetSize()
$6 = {
  <mozilla::gfx::BaseSize<int,nsIntSize>> = {
    width = 1606389496, 
    height = 32767
  }, <No data fields>}
(gdb) p (img1.mRawPtr)->Stride()
$7 = 1200
(gdb) p (img2.mRawPtr)->Stride()
$8 = 120

Are they sensible?
Blocks: 652494
(Assignee)

Comment 8

6 years ago
The 1200 and 120 look correct.

The GetSize results there look moderately bogus to me.
Is there anything I can do for this?
(Assignee)

Comment 10

6 years ago
Created attachment 545370 [details] [diff] [review]
Fix

Sure, review the patch!
Attachment #545370 - Flags: review?(joe)
(Assignee)

Updated

6 years ago
Assignee: nobody → bzbarsky
Priority: -- → P1
Whiteboard: [need review]
Comment on attachment 545370 [details] [diff] [review]
Fix

It feels weird to be calling ok() from this function, but as long as it works I'm happy.
Attachment #545370 - Flags: review?(joe) → review+
(Assignee)

Comment 12

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/7dd3e78e88ed
Flags: in-testsuite?
Whiteboard: [need review]
Target Milestone: --- → mozilla8

Updated

6 years ago
Attachment #545370 - Flags: approval-mozilla-aurora+

Updated

6 years ago
tracking-firefox7: ? → -
(Assignee)

Comment 13

6 years ago
http://hg.mozilla.org/releases/mozilla-aurora/rev/d5b0d70cf3bf
status-firefox7: --- → fixed
http://hg.mozilla.org/mozilla-central/rev/7dd3e78e88ed
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
(Assignee)

Comment 15

6 years ago
Actually, we have no test to make sure this doesn't regress... that's what the "in-testsuite?" is about.
Flags: in-testsuite+ → in-testsuite?
qa- as no QA fix verification necessary
Whiteboard: [qa-]

Comment 17

6 years ago
Bug 652494 covers fixing the test suite. Removing this from the list of bugs that need tests created.
Flags: in-testsuite? → in-testsuite-
You need to log in before you can comment on or make changes to this bug.