Closed Bug 512759 Opened 15 years ago Closed 15 years ago

mochitest-plain timeouts on Linux/Mac SeaMonkey

Categories

(Core :: Security: PSM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1
Tracking Status
status1.9.2 --- beta1-fixed
status1.9.1 --- .4-fixed

People

(Reporter: kairo, Assigned: mayhemer)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Serge, you were right to propose filing a bug on those, some investigation now made me realize we're actually popping up a download file picker dialog on one of the new security tests from bug 509692 and that's what causes our Linux/Mac mochitest-plain runs to time out and before that report focus problems (no wonder if a modal dialog is open).

Not sure yet which tests makes it show up, why it's only Mac and Linux, why we're different than Firefox (different download manager prefs causing it perhaps?) or how to fix it.
Please see security\manager\ssl\tests\mochitest\mixedcontent\test_bug383369.html and the runTest() function. I set properties for Firefox to NOT OPEN the download manager window and I prevent any "save as..." dialogs by setting download behavior for application/x-auto-download (the file I demand to download has this mime type) to auto. All this is handled in the runTest() function.

What exactly is "download file picker dialog"?
(In reply to comment #0)
> Serge, you were right to propose filing a bug on those, some investigation now

Thanks.

> made me realize we're actually popping up a download file picker dialog on one

That's why I asked you to check our boxes...

> of the new security tests from bug 509692 and that's what causes our Linux/Mac

That's what the Linux regression timeframe hinted, since it was unlikely to be the ssltunnel patch.
The MacOSX regression timeframe had a previous checkin too...

> how to fix it.

(You really pissed me off on that (and the other) one.)
status1.9.1: --- → ?
OS: Linux → All
Hardware: x86 → All
Assignee: kaie → honzab.moz
OK, should I backout the patch or at least disable the test for now, or are we about to find a constructive solution to this issue? 

"different download manager prefs causing it perhaps" - could anyone give me a hint to modify the test for seamonkey's preferences?
Status: NEW → ASSIGNED
(In reply to comment #1)
> What exactly is "download file picker dialog"?

The file picker (in the case on my local machine, the native GTK "safe file" dialog) that shows up when saving a download and not having your prefs set to safe downloads without asking. I don't know by heart what the pref name and value is, but I hope that description tells you what I mean.
"Blocks: 448125" without "Keywords: mlk" is unexpected:
is is leaking? where?
Oops, I think I picked the wrong bug. I wanted dependency on the SeaMonkey-specific test failures.

(In reply to comment #3)
> OK, should I backout the patch or at least disable the test for now, or are we
> about to find a constructive solution to this issue? 

Disabling would at least be a short-term help, but we might be on to something easy here.

> "different download manager prefs causing it perhaps" - could anyone give me a
> hint to modify the test for seamonkey's preferences?

If we just set the download target prefs (sorry, don't know the name/s by heart) to the same value as Firefox uses and reset them after the test, that should do the trick. The issue is probably the we have different defaults - Firefox saves to the download folder without prompting by default, while SeaMonkey prompts for the location to save in.
Blocks: SmTestFail
No longer blocks: SmTestLeak
(In reply to comment #4)
> (In reply to comment #1)
> > What exactly is "download file picker dialog"?
> 
> The file picker (in the case on my local machine, the native GTK "safe file"
> dialog) that shows up when saving a download and not having your prefs set to
> safe downloads without asking. I don't know by heart what the pref name and
> value is, but I hope that description tells you what I mean.

By setting "browser.download.folderList" to 2 and ".dir" to a valid path I prevent the file picker, the pref is read/used here: http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/nsExternalHelperAppService.cpp#404, AFAIK, this is Gecko code, not SeaMonkey specific, isn't it?

Then, I set the download behavior to "auto" for my file: 

    const mimeSvc = Components.classes["@mozilla.org/mime;1"]
      .getService(Components.interfaces.nsIMIMEService);
    var handlerInfo = mimeSvc.getFromTypeAndExtension("application/x-auto-download", "auto");
    handlerInfo.preferredAction = Components.interfaces.nsIHandlerInfo.saveToDisk;
    handlerInfo.alwaysAskBeforeHandling = false;
    handlerInfo.preferredApplicationHandler = null;

This downloads the file on Firefox. However, the related code is executed in uriloader as well, still not specific to SM. Then, what's wrong?
(In reply to comment #7)
> By setting "browser.download.folderList" to 2 and ".dir" to a valid path I
> prevent the file picker

Not unless browser.download.useDownloadDir is true, as far as I can tell. And it's false by default in SeaMonkey.
Using the pref helps.
Attachment #397473 - Flags: review?(kairo)
Comment on attachment 397473 [details] [diff] [review]
v1
[Checkin: See comment 12 & 19 & 22]

>     prefs.setCharPref("dir", "");
>+    prefs.setBoolPref("useDownloadDir", false);
>     prefs.setIntPref("folderList", 0);
>     prefs.setBoolPref("manager.closeWhenDone", false);
>     prefs.setBoolPref("manager.showWhenStarting", true);

For cleaning up, you really should do .clearUserPref() for all those values, as that just sets it back to whatever is the default for the app. See https://developer.mozilla.org/en/nsIPrefBranch#clearUserPref.28.29
For the new line you'd just do
prefs.clearUserPref("useDownloadDir");
and the next test will not know that you used your own values here (given you do that for all of them). :)

With that change, r=me.
Comment on attachment 397473 [details] [diff] [review]
v1
[Checkin: See comment 12 & 19 & 22]

Oops, should also mark the review and not only talk about it :)
Attachment #397473 - Flags: review?(kairo) → review+
I pushed this with my nit fixed as http://hg.mozilla.org/mozilla-central/rev/2b834d317457 and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9827dc366a48 so we can get those tests cleared up.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
SeaMonkey has not cycled yet,
but this seems to have broken Firefox 3.5 :-/
Unless it revealed another issue: see bug 494291!
Blocks: 494291
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Flags: in-testsuite+
I don't see how that changeset can have caused this, but it looks obvious that it must be that, and I don't have a Shiretoko tree around right now to test what's up, so I backed out the clearUserPref() changes from 1.9.1 (trunk is interestingly fine) and hope that part is the cause for FF 3.5, so we have some slight possibility left to fix both.
See http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b764310a9115 for that checkin.
Keywords: checkin-needed
Whiteboard: [c-n: comment 12 to m-1.9.2]
(In reply to comment #14)
> See http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b764310a9115 for that
> checkin.

This didn't help FF 3.5.
May be you need a try+catch around some of the pref lines?
I'm sure it's not anything that can be solved with a try/catch, or we probably would have an error message. I can't debug this right now, so I only can re-break SeaMonkey and back this out again. At least we had a green cycle on SeaMonkey in between.

As I can't debug right now (probably not today any more), backed everything out completely: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d41a885b2a0a
You could "if FF else SM" ftb...
OK, so what bites us is bug 487059, .clearUserPref throws in 1.9.1 if there is no user-set value, which is the case for FF as we just set the pref to the same value as the default. We can work around this by just wrapping every .clearUserPref into a try/catch.

I'll check in a variant with that into 1.9.1 only to fix the SeaMonkey test bustage without breaking FF.
Pushed to 1.9.1 with try/catch on all clearUserPref calls as http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0a86b30c95ce - sorry, I just realized I didn't attribute this checkin to Honza this time :(

BTW, the version that landed on mozilla-central can also land on 1.9.2, as bug 487059 went in way before that branch was cut, so no try/catch needed there.
Status: REOPENED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
Good, looks like it sticks now, Firefox 3.5 unit tests are green on Windows and Mac so far, Linux should be no different.
(In reply to comment #15)
> May be you need a try+catch around some of the pref lines?

(In reply to comment #16)
> I'm sure it's not anything that can be solved with a try/catch

(In reply to comment #18)
> We can work around this by just wrapping every
> .clearUserPref into a try/catch.

No comment...
Comment on attachment 397473 [details] [diff] [review]
v1
[Checkin: See comment 12 & 19 & 22]


http://hg.mozilla.org/releases/mozilla-1.9.2/rev/6a0235149815
Attachment #397473 - Attachment description: v1 → v1 [Checkin: See comment 12 & 19 & 22]
Keywords: checkin-needed
Whiteboard: [c-n: comment 12 to m-1.9.2]
Is there a log of this updated test running anywhere for Linux and Mac Seamonkey to verify against for 1.9.1?
http://tinderbox.mozilla.org/SeaMonkey2.0/ has the logs for our 1.9.1-based unit test boxes.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: