Closed Bug 597811 Opened 14 years ago Closed 14 years ago

[SeaMonkey] mochitests-1/5: "WARNING: NS_ENSURE_SUCCESS(rv, rv) ... nsHTMLInputElement.cpp, line 436" + "test_bug592802.html | Test timed out.", caused by "test_bug548193.html / nsContentPrefService.js"

Categories

(Core :: JavaScript Engine, defect)

x86
All
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla2.0b8
Tracking Status
blocking2.0 --- beta8+

People

(Reporter: sgautherie, Assigned: cdleary)

References

(Blocks 1 open bug, )

Details

(Keywords: intermittent-failure, Whiteboard: fixed-in-tracemonkey)

Attachments

(2 files)

This is a new test.

Example:
{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1284874914.1284877169.25897.gz
Linux comm-central-trunk debug test mochitests-1/5 on 2010/09/18 22:41:54

...
49763 INFO TEST-INFO | /tests/content/html/content/test/test_bug592802.html | maybe run tests <load:true, focus:true> -- loaded: complete active window: ([object ChromeWindow @ 0x9448e78 (native @ 0x997b498)]) chrome://navigator/content/navigator.xul focused window: ([object Window @ 0x9fb69a0 (native @ 0xa255348)]) http://mochi.test:8888/tests/content/html/content/test/test_bug592802.html desired window: ([object Window @ 0x9fb69a0 (native @ 0xa255348)]) http://mochi.test:8888/tests/content/html/content/test/test_bug592802.html child window: ([object Window @ 0x9fb69a0 (native @ 0xa255348)]) http://mochi.test:8888/tests/content/html/content/test/test_bug592802.html docshell visible: true
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file /builds/slave/comm-central-trunk-linux-debug/build/mozilla/content/html/content/src/nsHTMLInputElement.cpp, line 436
49764 ERROR TEST-UNEXPECTED-FAIL | /tests/content/html/content/test/test_bug592802.html | Test timed out.
...
SCREENSHOT: data:image/png;base64,...
}
Code is
{
267 AsyncClickHandler::Run()
...
433       // Store the last used directory using the content pref service
434       rv = nsHTMLInputElement::gUploadLastDir->StoreLastUsedDirectory(doc->GetDocumentURI(),
435                                                                       localFile);
436       NS_ENSURE_SUCCESS(rv, rv);
}
TopFails does not list this failure :-(
Depends on: 565307
Aqualon had tried to reproduce this locally and could not with just the TEST_PATH of this test, but was able to get it to time out by running all of mochi-1

Which indicates its not volkmar's tests issue.
Whiteboard: [orange]
(In reply to comment #3)
> Aqualon had tried to reproduce this locally and could not with just the
> TEST_PATH of this test, but was able to get it to time out by running all of
> mochi-1
> 
> Which indicates its not volkmar's tests issue.

Don't conclude too quickly, it could one other tests I wrote ;)
(In reply to comment #3)
> Aqualon had tried to reproduce this locally and could not with just the
> TEST_PATH of this test, but was able to get it to time out by running all of
> mochi-1

I confirm this on my local Win2K:
*All of content/html/ pass.
*Chunk 1/5 times out on this test.
 Ftr, Error Console lists multiple errors (from previous tests).

I'll try to narrow down which test triggers this failure,
(and file bugs w.r.t. the reported errors.)
When I run the tests on my linux box (only a P4 660), it does not error, so perhaps some sort of timing issue, or something related to how tests are done locally and on the test servers?

Also was just wondering why mockObject.js (http://mxr.mozilla.org/comm-central/source/mozilla/content/html/content/test/test_bug592802.html?force=1#39) was not loaded the same way as in the test_bug500885.html (http://mxr.mozilla.org/comm-central/source/mozilla/content/html/content/test/test_bug500885.html?force=1#11)
(In reply to comment #5)
> I'll try to narrow down which test triggers this failure,

Running test_bug548193.html (and this test) does:
{
Warning: CSP: Directive "allow http://mochi.test:8888" violated by http://example.org/tests/content/base/test/file_CSP.sjs?testid=img_bad&type=img/png

Error: missing ; before statement
Source File: file:///E:/Dvlp/Mozilla/SM21/seamonkey/components/nsContentPrefService.js
Line: 993, Column: 10
Source Code:
      let index = this._dbSchema.indices[name];
}

http://mxr.mozilla.org/mozilla-central/source/content/base/test/test_bug548193.html?force=1
{
89   // correct blocked-uri
90   is(cspReport["blocked-uri"],
91      "http://example.org/tests/content/base/test/file_CSP.sjs?testid=img_bad&type=img/png",
92      "Incorrect blocked uri");
}
Iiuc, the warning is expected.

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/contentprefs/src/nsContentPrefService.js
All 'let' in this file are errorring out :-<
As a workaround, replacing them with 'var' fixes the timeout (and the 2nd test passes):
I'm not sure whether this is what is wanted or if there is some "js version declaration" issue somewhere...

NB: I can't check the NS_ENSURE_SUCCESS() as I can run an Opt build only.
Blocks: 548193
Summary: [SeaMonkey] mochitests-1/5: "WARNING: NS_ENSURE_SUCCESS(rv, rv) ... nsHTMLInputElement.cpp, line 436" + "test_bug592802.html | Test timed out." → [SeaMonkey] mochitests-1/5: "WARNING: NS_ENSURE_SUCCESS(rv, rv) ... nsHTMLInputElement.cpp, line 436" + "test_bug592802.html | Test timed out.", caused by "test_bug548193.html / nsContentPrefService.js"
Comment on attachment 476738 [details] [diff] [review]
(Av1) Simplify test_bug592802.html mockObjects.js load
[Checked in: Comment 10]

Ftr, the file is copied at
http://mxr.mozilla.org/comm-central/source/mozilla/testing/mochitest/tests/SimpleTest/Makefile.in#57
Comment on attachment 476738 [details] [diff] [review]
(Av1) Simplify test_bug592802.html mockObjects.js load
[Checked in: Comment 10]

http://hg.mozilla.org/mozilla-central/rev/461202e21221
Attachment #476738 - Attachment description: (Av1) Simplify test_bug592802.html mockObjects.js load → (Av1) Simplify test_bug592802.html mockObjects.js load [Checked in: Comment 10]
Whiteboard: [orange] → [ToDo: comment 7] [orange]
(In reply to comment #7)
> (In reply to comment #5)
> > I'll try to narrow down which test triggers this failure,
> 
> Running test_bug548193.html (and this test)
Serge, could you give some steps how to run only those two tests together?
(In reply to comment #11)
> Serge, could you give some steps how to run only those two tests together?

It's manual: just remove the other test files before running mochitests-1 :-|
Ok, I have a slight handle on this failure.

In Short: We're getting "not available" for the content pref service.

In long:

we execute the test without failing stuff:

http://mxr.mozilla.org/comm-central/source/mozilla/content/html/content/test/test_bug592802.html?force=1 (including the click on element 'b')

The test then waits on the change event (registered at line # 186

The event would (normally) be fired by: http://mxr.mozilla.org/comm-central/source/mozilla/content/html/content/src/nsHTMLInputElement.cpp?mark=437-446#430

But instead we are getting a fail on the rv = nsHTMLInputElement::gUploadLastDir->StoreLastUsedDirectory(doc->GetDocumentURI(), localFile); section RIGHT ABOVE that (line #430)

and dropping out because we are checking the rv.

The error we are getting is equal to NS_ERROR_NOT_AVAILABLE, which that function only returns two ways, one of which is null and void for us (private browsing mode) the other is if we can't get the content pref service.

-----------

Solution: Possibly don't bother with the rv of this function? I'm not sure it makes that much sense to abort the change event if this fails.

volkmar?
By the way...

(In reply to comment #7)
> http://mxr.mozilla.org/mozilla-central/source/toolkit/components/contentprefs/src/nsContentPrefService.js
> All 'let' in this file are errorring out :-<
> As a workaround, replacing them with 'var' fixes the timeout (and the 2nd test
> passes):
> I'm not sure whether this is what is wanted or if there is some "js version
> declaration" issue somewhere...

Is the underlying problem...

I've been pinging around on IRC to find out _how_ that is possible [for us but not firefox]... but haven't reached anyone who seems to care.
It sounds wrong indeed to fail when we can't set the last used directory. For example, the return value of FetchLastUsedDirectory is just ignored. We should fix that (and also fix the cause of the failure).
(Just a thought: could that be somehow related to bug 533290?)
Trying to repro, what was the procedure for getting it down to just those two tests? (Also, did we see it happening on Linux/64? That's what I'm trying to repro on.)
(In reply to comment #17)

> what was the procedure for getting it down to just those two tests?

See comment 12.
Maybe another test than test_bug548193.html can trigger this failure too:
at least that one does, that's all I looked for (iirc).

> Also, did we see it happening on Linux/64?

SeaMonkey has no Linux/64 tests box.
But I would expect the failure to happen there too.
(In reply to comment #18)
> SeaMonkey has no Linux/64 tests box.

But you can find both build and tests at
/pub/seamonkey/tinderbox-builds/comm-central-trunk-linux64/
Okay, saw in on my box: 51419 ERROR TEST-UNEXPECTED-FAIL | /tests/content/html/content/test/test_bug592802.html | Test timed out.

If anybody has an idea about how to isolate it further, I'd appreciate the help!
Assignee: nobody → cdleary
Status: NEW → ASSIGNED
(In reply to comment #7)
> I'm not sure whether this is what is wanted or if there is some "js version
> declaration" issue somewhere...

(In reply to comment #20)
> If anybody has an idea about how to isolate it further, I'd appreciate the
> help!

Fwiw, Callek told me earlier today he wanted to check whether bug 595691 might fix this (regression?).
Tried this out to force the component loader to use the latest JS version but I'm still seeing what look like the same failures.

For now I'm hoping that the other version cleanup I'm working on will be in trunk soon and magically resolve this issue -- if we can get a reduced test case I'll take a harder look.
Comment on attachment 482662 [details] [diff] [review]
Force JSVERSION_LATEST in the component loader
[Checked in: Comment 32 & 34+36+37]

Ok, on windows, tested this (along with Bug 596580 applied) and it fixes this bug for me.

Mochi-1 did not exhibit this. (before it was _always_). I'll later do a patch for the bandaid I suggested to volkmar.  My *one* concern is I'm still not sure what triggered this; but I'm happy with the deeper fix.
Attachment #482662 - Flags: feedback+
Depends on: 596580
Depends on: 603919
I've open bug 603919 to still send the 'changed' event if StoreLastUsedDirectory fails.
Comment on attachment 482662 [details] [diff] [review]
Force JSVERSION_LATEST in the component loader
[Checked in: Comment 32 & 34+36+37]

can we please get someone to review this and it checked in, it removes all instances of this problem for me. [yes cdleary can't review his own patch, but he'll know who to request review from]
Attachment #482662 - Flags: review?(cdleary)
Comment on attachment 482662 [details] [diff] [review]
Force JSVERSION_LATEST in the component loader
[Checked in: Comment 32 & 34+36+37]

sayrer will know if this is okay to force on JS components. If so, I'll give a last lap through a moz-central try before pushing.
Attachment #482662 - Flags: review?(cdleary) → review?(sayrer)
Attachment #482662 - Flags: review?(sayrer) → review+
(In reply to comment #26)
> Comment on attachment 482662 [details] [diff] [review]
> Force JSVERSION_LATEST in the component loader.
> 
> sayrer will know if this is okay to force on JS components. If so, I'll give a
> last lap through a moz-central try before pushing.

Hmm this is now passing on windows with currently in-tree code, perhaps "something else" fixed this test. But I still think we should take this patch anyway.
(In reply to comment #27)

Indeed!

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1287491837.1287497101.22438.gz
Linux comm-central-trunk debug test mochitests-1/5 on 2010/10/19 05:37:17
rev:92544eb8a854
moz:c9df0c5cbf8c
nsHTMLInputElement.cpp assertion only
+
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1287480721.1287483158.7801.gz
OS X 10.5 comm-central-trunk debug test mochitests-1/5 on 2010/10/19 02:32:01
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1287484097.1287487241.1314.gz
WINNT 5.2 comm-central-trunk debug test mochitests-1/5 on 2010/10/19 03:28:17
rev:92544eb8a854
moz:cb8754333dd5
{
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file e:/builds/slave/comm-central-trunk-win32-debug/build/mozilla/content/html/content/src/nsHTMLInputElement.cpp, line 432

WARNING: NS_ENSURE_TRUE(aListener) failed: file e:/builds/slave/comm-central-trunk-win32-debug/build/mozilla/editor/libeditor/base/nsEditor.cpp, line 1759
}

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1287499482.1287502502.12740.gz
Linux comm-central-trunk debug test mochitests-1/5 on 2010/10/19 07:44:42
rev:92544eb8a854
moz:84abb3a647d9
(.!?.)
+
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1287488841.1287490965.23130.gz
OS X 10.5 comm-central-trunk debug test mochitests-1/5 on 2010/10/19 04:47:21
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1287500056.1287505662.26221.gz
WINNT 5.2 comm-central-trunk debug test mochitests-1/5 on 2010/10/19 07:54:16
rev:92544eb8a854
moz:84abb3a647d9
{
WARNING: NS_ENSURE_SUCCESS(result, result) failed with result 0x80004005: file e:/builds/slave/comm-central-trunk-win32-debug/build/mozilla/editor/libeditor/base/nsEditor.cpp, line 3895
WARNING: NS_ENSURE_SUCCESS(res, res) failed with result 0x80004005: file e:/builds/slave/comm-central-trunk-win32-debug/build/mozilla/editor/libeditor/text/nsTextEditRules.cpp, line 497
}
twice.

Fix timeframe:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c9df0c5cbf8c&tochange=84abb3a647d9
"Bug 570387 - PlacesDBUtils should register itself in the idle-daily category."

Go figure...
Fwiw, see later bug 605503 too.
(In reply to comment #28)
> Fwiw, see later bug 605503 too.

And, with that changeset, this bug is back:

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1287537643.1287542036.15922.gz
Linux comm-central-trunk debug test mochitests-1/5 on 2010/10/19 18:20:43
{
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file /builds/slave/comm-central-trunk-linux-debug/build/mozilla/content/html/content/src/nsHTMLInputElement.cpp, line 432

WARNING: An event was posted to a thread that will never run it (rejected): file /builds/slave/comm-central-trunk-linux-debug/build/mozilla/xpcom/threads/nsThread.cpp, line 371
WARNING: leaking reference to nsTimerImpl: file /builds/slave/comm-central-trunk-linux-debug/build/mozilla/xpcom/threads/nsTimerImpl.cpp, line 491
WARNING: 1 sort operation has occurred for the SQL statement '0xbe52128'.  See https://developer.mozilla.org/En/Storage/Warnings details.: file /builds/slave/comm-central-trunk-linux-debug/build/mozilla/storage/src/mozStoragePrivateHelpers.cpp, line 138
}

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1287537985.1287540400.8724.gz
OS X 10.5 comm-central-trunk debug test mochitests-1/5 on 2010/10/19 18:26:25
{
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file /builds/slave/comm-central-trunk-macosx-debug/build/mozilla/content/html/content/src/nsHTMLInputElement.cpp, line 432
}

Fwiw...
Comment on attachment 482662 [details] [diff] [review]
Force JSVERSION_LATEST in the component loader
[Checked in: Comment 32 & 34+36+37]

Hmm shortly after this was written Chris told me he would land it on tm; but I see now indication that has happend... I'm hoping it can land either there ASAP or a2.0 can be granted so it can land in trunk.
Attachment #482662 - Flags: approval2.0?
"blocking2.0=?":
Ensure components use JSVERSION_LATEST to accept |let|, etc, no risk.
blocking2.0: --- → ?
Component: DOM → JavaScript Engine
QA Contact: general → general
Looks spiffy on try, pushed to TM.

http://hg.mozilla.org/tracemonkey/rev/b71e965f4093
Whiteboard: [ToDo: comment 7] [orange] → [ToDo: comment 7] [orange] fixed-in-tracemonkey
blocking2.0: ? → beta8+
Closing this out, reopen if it it's still an issue.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Attachment #482662 - Flags: approval2.0?
(In reply to comment #32)
> http://hg.mozilla.org/tracemonkey/rev/b71e965f4093

http://hg.mozilla.org/mozilla-central/rev/b71e965f4093

V.Fixed, per 3-platform SeaMonkey t-b.
Status: RESOLVED → VERIFIED
Whiteboard: [ToDo: comment 7] [orange] fixed-in-tracemonkey → [orange] fixed-in-tracemonkey
Target Milestone: --- → mozilla2.0b8
Comment on attachment 482662 [details] [diff] [review]
Force JSVERSION_LATEST in the component loader
[Checked in: Comment 32 & 34+36+37]

On OS/2 we don't have mmap and we break after this checkin
../../../../dist/include/jsapi.h:2469: error: too many arguments to function 'JSScript* JS_CompileFileHandleForPrincipalsVersion(JSContext*, JSObject*, const char*, FILE*, JSPrincipals*)'
I:/comm-central.bak/mozilla/js/src/xpconnect/loader/mozJSComponentLoader.cpp:1154: error: at this point in file

Isn't there "JSVersion version" missing as the last argument of JS_CompileFileHandleForPrincipalsVersion in jsapi.h ?

>diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp
>--- a/js/src/jsapi.cpp
>+++ b/js/src/jsapi.cpp
>@@ -4493,16 +4504,24 @@ JS_CompileFileHandleForPrincipals(JSCont
>+JS_CompileFileHandleForPrincipalsVersion(JSContext *cx, JSObject *obj, const char *filename,
>+                                         FILE *file, JSPrincipals *principals, JSVersion version)
>diff --git a/js/src/jsapi.h b/js/src/jsapi.h
>--- a/js/src/jsapi.h
>+++ b/js/src/jsapi.h
>@@ -2422,16 +2429,21 @@ extern JS_PUBLIC_API(JSScript *)
>+extern JS_PUBLIC_API(JSScript *)
>+JS_CompileFileHandleForPrincipalsVersion(JSContext *cx, JSObject *obj,
>+                                         const char *filename, FILE *fh,
>+                                         JSPrincipals *principals);
>+
>diff --git a/js/src/xpconnect/loader/mozJSComponentLoader.cpp b/js/src/xpconnect/loader/mozJSComponentLoader.cpp
>--- a/js/src/xpconnect/loader/mozJSComponentLoader.cpp
>+++ b/js/src/xpconnect/loader/mozJSComponentLoader.cpp
>@@ -1113,39 +1113,38 @@ mozJSComponentLoader::GlobalForLocation(
> #else  /* HAVE_PR_MEMMAP */
>-            script = JS_CompileFileHandleForPrincipals(cx, global,
>-                                                       nativePath.get(),
>-                                                       fileHandle, jsPrincipals);
>+            script = JS_CompileFileHandleForPrincipalsVersion(
>+              cx, global, nativePath.get(), fileHandle, jsPrincipals, JSVERSION_LATEST);
> 
>             /* JS will close the filehandle after compilation is complete. */
> #endif /* HAVE_PR_MEMMAP */
Comment on attachment 482662 [details] [diff] [review]
Force JSVERSION_LATEST in the component loader
[Checked in: Comment 32 & 34+36+37]

(In reply to comment #35)
> Isn't there "JSVersion version" missing as the last argument of
> JS_CompileFileHandleForPrincipalsVersion in jsapi.h ?

Good catch!

http://hg.mozilla.org/mozilla-central/rev/5fc1f9723b56
Attachment #482662 - Attachment description: Force JSVERSION_LATEST in the component loader. → Force JSVERSION_LATEST in the component loader [Checked in: Comment 32 & 34+36]
Comment on attachment 482662 [details] [diff] [review]
Force JSVERSION_LATEST in the component loader
[Checked in: Comment 32 & 34+36+37]

http://hg.mozilla.org/mozilla-central/rev/9892fb9d7141
(Dv1) Revert extraneous change in patch Cv1

Sorry about that.
Attachment #482662 - Attachment description: Force JSVERSION_LATEST in the component loader [Checked in: Comment 32 & 34+36] → Force JSVERSION_LATEST in the component loader [Checked in: Comment 32 & 34+36+37]
(In reply to comment #36)
> http://hg.mozilla.org/mozilla-central/rev/5fc1f9723b56

For future reference, this was not a valid use of "DONTBUILD". Bustage fixes should get builds.
(In reply to comment #38)
> Bustage fixes should get builds.

In this case, as only OS/2 seemed affected, I thought DONTBUILD would be fine...
Blocks: 619713
No longer blocks: 619713
Blocks: 619713
Whiteboard: [orange] fixed-in-tracemonkey → fixed-in-tracemonkey
You need to log in before you can comment on or make changes to this bug.