Last Comment Bug 771354 - Loading remote xul crashes gecko
: Loading remote xul crashes gecko
Status: RESOLVED FIXED
[qa?]
: crash, regression
Product: Core
Classification: Components
Component: Security: CAPS (show other bugs)
: Trunk
: All All
: -- critical (vote)
: mozilla18
Assigned To: Bobby Holley (:bholley) (busy with Stylo)
: Ioana (away)
Mentors:
Depends on: 791605 774607
Blocks: 754202 771224
  Show dependency treegraph
 
Reported: 2012-07-05 15:08 PDT by Malini Das [:mdas] - Away, not checking bugmail
Modified: 2012-12-17 14:00 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
fixed
fixed
unaffected


Attachments
mochitest chrome test (1.07 KB, application/vnd.mozilla.xul+xml)
2012-07-05 15:08 PDT, Malini Das [:mdas] - Away, not checking bugmail
no flags Details
'remote' xul to be loaded (796 bytes, application/vnd.mozilla.xul+xml)
2012-07-05 15:08 PDT, Malini Das [:mdas] - Away, not checking bugmail
no flags Details
backtrace (9.44 KB, text/plain)
2012-07-06 07:46 PDT, Malini Das [:mdas] - Away, not checking bugmail
no flags Details
backtrace for all threads (79.22 KB, text/plain)
2012-07-06 07:47 PDT, Malini Das [:mdas] - Away, not checking bugmail
no flags Details
Don't special-case principal assignment for chrome windows in nsGlobalWindow.cpp. v1 (3.93 KB, patch)
2012-07-26 10:50 PDT, Bobby Holley (:bholley) (busy with Stylo)
bzbarsky: review+
Details | Diff | Splinter Review

Description Malini Das [:mdas] - Away, not checking bugmail 2012-07-05 15:08:04 PDT
Created attachment 639482 [details]
mochitest chrome test

We load some remote xul for Marionette unit testing purposes by setting the profile's "dom.allow_XUL_XBL_for_file" preference to true, and loading some locally hosted xul. 

The .xul file used is attached (test.xul). It hold a dialog with some elements in it. Sometime after last week we've been seeing this crash:

WARNING: Not same origin error!: file /Users/mdas/code/mozilla-central/dom/base/nsJSEnvironment.cpp, line 350
JavaScript error: chrome://global/content/bindings/dialog.xml, line 196: Permission denied for <file://> to get property XPCComponents.classes


To reproduce this error, copy test.xul somewhere to your file system. Copy  test_file_fail.xul into your (objdir)/_tests/testing/mochitest/chrome directory and replace the YOUR_PATH_HERE text in test_file_fail.xul with the file path to test.xul (for example: file:///Users/mdas/test.xul). Run it with:

python runtests.py --chrome --test-path=test_file_fail.xul --setpref="dom.allow_XUL_XBL_for_file=true"

Once you click and run the test, the gecko process will soon crash, and you will see the Permission Denied message above.

From our tests, it seems the change that triggered this happened sometime between these commits: https://github.com/mozilla/mozilla-central/compare/6b7e00d441baa574c0c62f3cc9ef01482223009a...2be4e380b4fb6ffc71a98c96de2d30cd48eeb8f2
Comment 1 Malini Das [:mdas] - Away, not checking bugmail 2012-07-05 15:08:21 PDT
Created attachment 639483 [details]
'remote' xul to be loaded
Comment 2 Henrik Skupin (:whimboo) 2012-07-05 21:56:33 PDT
Malini, can you run it under gdb and provide a crash stack? Also will you be able to get a smaller regression range? Is this code on github a 1-1 clone from hg.mozilla.org/mozilla-central?
Comment 3 Bobby Holley (:bholley) (busy with Stylo) 2012-07-06 00:39:19 PDT
This sounds more like a regression from bug 754202, but I don't see that in the revision range.
Comment 4 Malini Das [:mdas] - Away, not checking bugmail 2012-07-06 07:46:33 PDT
Created attachment 639665 [details]
backtrace

Here's the backtrace after the crash
Comment 5 Malini Das [:mdas] - Away, not checking bugmail 2012-07-06 07:47:34 PDT
Created attachment 639667 [details]
backtrace for all threads

output of `thread apply all bt full`
Comment 6 Benjamin Smedberg [:bsmedberg] 2012-07-06 07:49:18 PDT
What revision is this stacktrace against, so we can match up line numbers?
Comment 7 Malini Das [:mdas] - Away, not checking bugmail 2012-07-06 07:53:52 PDT
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #6)
> What revision is this stacktrace against, so we can match up line numbers?

https://hg.mozilla.org/mozilla-central/rev/47f814827db6
Comment 8 Malini Das [:mdas] - Away, not checking bugmail 2012-07-06 07:54:18 PDT
(In reply to Henrik Skupin (:whimboo) from comment #2)
> Malini, can you run it under gdb and provide a crash stack? Also will you be
> able to get a smaller regression range? Is this code on github a 1-1 clone
> from hg.mozilla.org/mozilla-central?

I'll try to get a better regression range, and yes, this code is a 1-1 clone.
Comment 9 Benjamin Smedberg [:bsmedberg] 2012-07-06 08:29:52 PDT
Crash is at http://hg.mozilla.org/mozilla-central/annotate/47f814827db6/dom/base/nsJSEnvironment.cpp#l1222

1221 principal->Equals(scopeObjectPrincipal, &equal);
1222 MOZ_ASSERT(equal);
Comment 10 Jonathan Griffin (:jgriffin) 2012-07-06 10:20:02 PDT
(In reply to Bobby Holley (:bholley) from comment #3)
> This sounds more like a regression from bug 754202, but I don't see that in
> the revision range.

Yep, that was my fault, that regression range was for another crashing bug, not this one.  The m-c commit this first appears on is http://hg.mozilla.org/mozilla-central/rev/4a8e0d5fc954.
Comment 11 Jonathan Griffin (:jgriffin) 2012-07-06 10:30:21 PDT
And tracing that back through the mozilla-inbound merge, this problem first appears at http://hg.mozilla.org/integration/mozilla-inbound/rev/cf09f7b139da.
Comment 12 Bobby Holley (:bholley) (busy with Stylo) 2012-07-06 11:31:18 PDT
I'm totally willing to believe that this is happening, but I can't reproduce it with the STR in comment 0. I see the two warnings, but I don't get a crash.
Comment 13 Bobby Holley (:bholley) (busy with Stylo) 2012-07-06 11:34:42 PDT
From the looks of it, we're getting a mismatch in principals between the XUL element and the binding. I'm guessing we should just pass the principal from the bound content to the field installer.
Comment 14 Malini Das [:mdas] - Away, not checking bugmail 2012-07-06 13:51:10 PDT
(In reply to Bobby Holley (:bholley) from comment #12)
> I'm totally willing to believe that this is happening, but I can't reproduce
> it with the STR in comment 0. I see the two warnings, but I don't get a
> crash.

That's odd, I just pulled from m-c today and I can reproduce the error. Can you verify that you passed the `--setpref="dom.allow_XUL_XBL_for_file=true"` parameter to the runtests.py program? If you don't set that pref, then it'll ignore the file and you just get warnings.
Comment 15 Bobby Holley (:bholley) (busy with Stylo) 2012-07-06 14:13:53 PDT
I see the XUL popup window (box, test,test,test, etc), so presumably that part's working. Maybe it's a platform thing? I'm on mac.

Anyway, I understand the problem and it should be straightforward to fix, but I need a way to reproduce it first. Can you try to put together a reduced crashtest? Or is there no way to do remote xul under the reftest (or mochitest) harness?
Comment 16 Malini Das [:mdas] - Away, not checking bugmail 2012-07-06 14:27:21 PDT
(In reply to Bobby Holley (:bholley) from comment #15)
> I see the XUL popup window (box, test,test,test, etc), so presumably that
> part's working. Maybe it's a platform thing? I'm on mac.
> 
> Anyway, I understand the problem and it should be straightforward to fix,
> but I need a way to reproduce it first. Can you try to put together a
> reduced crashtest? Or is there no way to do remote xul under the reftest (or
> mochitest) harness?

I'm not too familiar with the harness, I can see if there's another way though, unless someone here knows of a way.

I'm also running on a mac, 10.6.8
Comment 17 Bobby Holley (:bholley) (busy with Stylo) 2012-07-10 02:25:12 PDT
Sorry I disappeared last night (was dinner time over here in Europe). Were you able to reproduce the crash with a clean build?
Comment 18 Jonathan Griffin (:jgriffin) 2012-07-10 10:28:50 PDT
I reproduced with the latest m-c build:  http://stage.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux-debug/1341933030/firefox-16.0a1.en-US.linux-i686.tar.bz2

To reproduce:  download build, launch it, and add the following prefs:

marionette.defaultPrefs.enabled = true
dom.allow_XUL_XBL_for_file = true

Then restart it.

Next, go to m-c/testing/marionette/client/marionette, and execute the following command:

python runtests.py --address localhost:2828 tests/unit/test_text.py

You may need to install some python packages first, but these are available via pypi:  datazilla, manifestparser, mozhttpd, mozprocess
Comment 19 Malini Das [:mdas] - Away, not checking bugmail 2012-07-10 15:33:46 PDT
Oh, you'll need python2.7 to run the test in Comment 18

If for whatever reason you don't want to download those packages into your python path, we have a virtualenv you can use. You can go to m-c/testing/marionette/client/marionette and then type:

sh venv_test.sh <path to python2.7>

This will create a marionette_venv directory in m-c/testing/marionette/client, and if you cd into that directory and call

. bin/activate

you'll have a python virtual environment with all the packages pre-installed. You can then go back to m-c/testing/marionette/client/marionette and run:

python runtests.py --address localhost:2828 tests/unit/test_text.py

To get out of the virtual environment, just type 'deactivate'
Comment 20 Malini Das [:mdas] - Away, not checking bugmail 2012-07-12 12:24:54 PDT
Hm once bug 771224 lands, we won't be able to reproduce this with the latest builds, you'd have to run them from the previous debug builds (ftp://ftp.mozilla.org/pub/firefox/tinderbox-builds/mozilla-central-macosx64-debug/ for osx 64bit). 

I'm not sure what to do as a test alternative, since the mochitest failed to reproduce on bholley's machine.
Comment 21 Bobby Holley (:bholley) (busy with Stylo) 2012-07-13 07:46:03 PDT
Ok, I managed to reproduce this by rewinding to 5b77d71ed927, building with ENABLE_MARIONETTE=1, enabling the virtual environment, and running the test. I'll take a crack at fixing the bug next week. Thanks for all the hard work helping me reproduce! ;-)
Comment 24 Bobby Holley (:bholley) (busy with Stylo) 2012-07-26 10:50:15 PDT
Created attachment 646210 [details] [diff] [review]
Don't special-case principal assignment for chrome windows in nsGlobalWindow.cpp. v1

This is a simple patch that applies on top of bug 774607, whose correctness in turn depends on bug 774633.
Comment 25 Boris Zbarsky [:bz] 2012-08-23 14:32:05 PDT
Comment on attachment 646210 [details] [diff] [review]
Don't special-case principal assignment for chrome windows in nsGlobalWindow.cpp. v1

r=me
Comment 26 Bobby Holley (:bholley) (busy with Stylo) 2012-08-23 16:46:22 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/670c8a2695b3
Comment 27 Matt Brubeck (:mbrubeck) 2012-08-23 21:41:48 PDT
Backed out along with other patches from the same push, because of possibly causing timeouts in 'make check' on Windows:
https://hg.mozilla.org/integration/mozilla-inbound/rev/deeadcce3f64
https://tbpl.mozilla.org/php/getParsedLog.php?id=14652164&tree=Mozilla-Inbound
Comment 28 Bobby Holley (:bholley) (busy with Stylo) 2012-09-05 11:35:47 PDT
Relanded to m-i with the fix for the hanging windows make check:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=cfc884e6e414
Comment 29 Ryan VanderMeulen [:RyanVM] 2012-09-05 19:38:49 PDT
https://hg.mozilla.org/mozilla-central/rev/cfc884e6e414
Comment 30 Bobby Holley (:bholley) (busy with Stylo) 2012-09-06 10:19:33 PDT
Pushed to m-a:
https://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?changeset=b8ed43695721
Comment 31 Bobby Holley (:bholley) (busy with Stylo) 2012-09-06 14:50:00 PDT
I didn't test this stuff with the marionette environment recently, and I seem to have lost track of it. If someone wants to make verify this, use the steps in comment 18.
Comment 32 Bobby Holley (:bholley) (busy with Stylo) 2012-09-07 11:25:46 PDT
Pushed to m-b:
https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?changeset=6fbaf3aed7a0

Note that the approval here comes from bug 774633. The patches are inextricably tied, and it's just a historical artifact that they're split across separate bugs.
Comment 33 Ioana (away) 2012-10-09 07:35:28 PDT
I have been trying to verify this bug, but I ran into a problem. If anyone here can help me, please do so. Here are the details:

When I try to verify this fix with the steps from comment 18, I get this http://pastebin.mozilla.org/1861886.

I built the client locally, with Marionette enabled. I also set the following preferences, and restarted the client, before trying to run the test:
marionette.defaultPrefs.enabled;true
marionette.defaultPrefs.port;2828
dom.allow_XUL_XBL_for_file;true

I used the mozilla-central repo for the build, since I was told that Marionette is broken on beta.
Comment 34 Jonathan Griffin (:jgriffin) 2012-10-09 11:30:30 PDT
(In reply to Ioana Budnar [QA] from comment #33)
> I have been trying to verify this bug, but I ran into a problem. If anyone
> here can help me, please do so. Here are the details:
> 
> When I try to verify this fix with the steps from comment 18, I get this
> http://pastebin.mozilla.org/1861886.
> 
> I built the client locally, with Marionette enabled. I also set the
> following preferences, and restarted the client, before trying to run the
> test:
> marionette.defaultPrefs.enabled;true
> marionette.defaultPrefs.port;2828
> dom.allow_XUL_XBL_for_file;true
> 
> I used the mozilla-central repo for the build, since I was told that
> Marionette is broken on beta.

That should do the trick. How did you enable Marionette in your build?  In case you didn't know, you need to add

ENABLE_MARIONETTE=1

in your mozconfig before building.
Comment 35 Malini Das [:mdas] - Away, not checking bugmail 2012-10-09 16:13:11 PDT
(In reply to Jonathan Griffin (:jgriffin) from comment #34)
> (In reply to Ioana Budnar [QA] from comment #33)
> > I have been trying (In reply to Ioana Budnar [QA] from comment #33)
> I have been trying to verify this bug, but I ran into a problem. If anyone
> here can help me, please do so. Here are the details:
> 
> When I try to verify this fix with the steps from comment 18, I get this
> http://pastebin.mozilla.org/1861886.
> 
> I built the client locally, with Marionette enabled. I also set the
> following preferences, and restarted the client, before trying to run the
> test:
> marionette.defaultPrefs.enabled;true
> marionette.defaultPrefs.port;2828
> dom.allow_XUL_XBL_for_file;true
> 
> I used the mozilla-central repo for the build, since I was told that
> Marionette is broken on beta.

I just pulled mozilla-central today and built, and had no problems running tests. The following is what my mozconfig has.

mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/../obj-ff
mk_add_options AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213
. $topsrcdir/browser/config/mozconfig
mk_add_options MOZ_MAKE_FLAGS="-j4"
export CC=/usr/local/bin//clang
export CXX=/usr/local/bin//clang++

ac_add_options --enable-debug
ac_add_options --disable-optimize

ENABLE_MARIONETTE=1

The only thing of importance here is "ENABLE_MARIONETTE=1"

As for the profile, my user.js file only contains:
pref("marionette.defaultPrefs.enabled", true);

Hope this helps!
Comment 36 Ioana (away) 2012-10-12 07:58:43 PDT
Jonathan, Malini, I had this in my mozconfig:
ENABLE_MARIONETTE=1
ac_add_options --enable-debug

Malini, are any of the other options in your mozconfig related to Marionette?
Comment 37 Jonathan Griffin (:jgriffin) 2012-10-12 09:21:10 PDT
(In reply to Ioana Budnar [QA] from comment #36)
> Jonathan, Malini, I had this in my mozconfig:
> ENABLE_MARIONETTE=1
> ac_add_options --enable-debug
> 
> Malini, are any of the other options in your mozconfig related to Marionette?

That should do the trick.
Comment 38 Ioana (away) 2012-10-17 07:36:25 PDT
(In reply to Jonathan Griffin (:jgriffin) from comment #37)
> (In reply to Ioana Budnar [QA] from comment #36)
> > Jonathan, Malini, I had this in my mozconfig:
> > ENABLE_MARIONETTE=1
> > ac_add_options --enable-debug
> > 
> > Malini, are any of the other options in your mozconfig related to Marionette?
> 
> That should do the trick.

It's not doing the trick for me yet. Any ideas on what the problem could be?

Note You need to log in before you can comment on or make changes to this bug.