Last Comment Bug 603228 - "Exception / NS_ERROR_FAILURE / nsIWebProgress.addProgressListener / notification.xml :: addProgressListener :: line 66" caused by some part of bug 595810
: "Exception / NS_ERROR_FAILURE / nsIWebProgress.addProgressListener / notifica...
Status: VERIFIED FIXED
: regression
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1b2
Assigned To: neil@parkwaycc.co.uk
:
:
Mentors:
http://mxr.mozilla.org/comm-central/s...
Depends on:
Blocks: 595810
  Show dependency treegraph
 
Reported: 2010-10-10 16:41 PDT by Serge Gautherie (:sgautherie)
Modified: 2010-10-13 13:47 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proposed patch (3.13 KB, patch)
2010-10-11 03:42 PDT, neil@parkwaycc.co.uk
iann_bugzilla: review+
bugspam.Callek: feedback+
Details | Diff | Splinter Review
Alternate patch (2.08 KB, patch)
2010-10-11 04:07 PDT, neil@parkwaycc.co.uk
no flags Details | Diff | Splinter Review

Description Serge Gautherie (:sgautherie) 2010-10-10 16:41:58 PDT
On
http://tinderbox.mozilla.org/showbuilds.cgi?tree=SeaMonkey&maxdate=1286709647&hours=24&legend=0&norules=1

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1286666344.1286667820.29875.gz&fulltext=1
OS X 10.5 comm-central-trunk debug test mochitest-other on 2010/10/09 16:19:04
rev:7661b2db37db
moz:904a556a15f2
Fine.

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1286671376.1286672895.15141.gz&fulltext=1
OS X 10.5 comm-central-trunk debug test mochitest-other on 2010/10/09 17:42:56
rev:bfd1320b39ac
moz:904a556a15f2
Lots of tests trigger
{
JavaScript error: , line 0: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIWebProgress.addProgressListener]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://communicator/content/bindings/notification.xml :: addProgressListener :: line 66"  data: no]
}

Regression timeframe:
http://hg.mozilla.org/comm-central/pushloghtml?fromchange=7661b2db37db&tochange=bfd1320b39ac
Obvious culprit: bug 595810.

Code is
{
60       <method name="addProgressListener">
61         <body>
62           <![CDATA[
63             if (this.activeBrowser && !this._addedProgressListener) {
64               this.activeBrowser.webProgress
65                   .addProgressListener(this, Components.interfaces.nsIWebProgress.NOTIFY_LOCATION |
66                                              Components.interfaces.nsIWebProgress.NOTIFY_REFRESH);
67               this._addedProgressListener = true;
68             }
69           ]]>
70         </body>
71       </method>
}
Comment 1 Justin Wood (:Callek) 2010-10-10 20:06:48 PDT
To just be thorough, from Bug 595810 copying here what I pasted.

(In reply to comment #40)
> (In reply to comment #39)
> > http://hg.mozilla.org/comm-central/rev/317093217f10
> > Part 4: reland geolocation doorhanger implementation
> 
> Causing test fails in
> http://mxr.mozilla.org/comm-central/source/mozilla/content/html/document/test/test_bug369370.html?force=1
> 
> I don't see anything at a glance in the patchset to cause this, but helpful:
> 
> Error: uncaught exception: [Exception... "Component returned failure code:
> 0x80004005 (NS_ERROR_FAILURE) [nsIWebProgress.addProgressListener]"  nsresult:
> "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame ::
> chrome://communicator/content/bindings/notification.xml :: addProgressListener
> :: line 67"  data: no]
> 
> Appears twice when the test runs (once for each launched SeaMonkey window)
> 
> JS frame :: chrome://communicator/content/bindings/notification.xml ::
> addProgressListener :: line 71
> Source File: chrome://communicator/content/bindings/notification.xml
> Line: 73
> 
> JS frame :: chrome://communicator/content/bindings/notification.xml ::  :: line
> 491
> Source File: chrome://communicator/content/bindings/notification.xml
> Line: 73
> 
> From manual frame inspection
> 
> I'm backing this out due to the test failures, [sorry].
> 
> Neil, can we (please) split out remaining work to new bugs so we can properly
> track this landing.
> 
> Serge filed a bug for this error, not knowing it was causing test failures at
> the time though, lets track this actual fix [and relanding of this patch] in
> Bug 603228 because of that, and file new bugs for additional work here...
Comment 2 Justin Wood (:Callek) 2010-10-10 20:10:24 PDT
(In reply to comment #1)
> > I'm backing this out due to the test failures, [sorry].

Done: http://hg.mozilla.org/comm-central/rev/8b5776bb0c0e
Comment 3 Serge Gautherie (:sgautherie) 2010-10-10 22:21:57 PDT
(In reply to comment #1)
> > Causing test fails in [test_bug369370.html]

Ftr/Fwiw, this test is run in mochitest-plain-1, and failed on Linux and Windows but not MacOSX :-|
Comment 4 Serge Gautherie (:sgautherie) 2010-10-10 22:25:03 PDT
(In reply to comment #1)
> > Causing test fails in [test_bug369370.html]

And the failure was:
{
53429 ERROR TEST-UNEXPECTED-FAIL | /tests/content/html/document/test/test_bug369370.html | Checking scrollLeft - got 424, expected 408
53430 ERROR TEST-UNEXPECTED-FAIL | /tests/content/html/document/test/test_bug369370.html | Checking scrollTop - got 332, expected 316
}
This was Windows, Linux reported 423 and 331.
Comment 5 neil@parkwaycc.co.uk 2010-10-11 01:59:35 PDT
This exception, although unexpected, is unrelated to the test failure.
Comment 6 neil@parkwaycc.co.uk 2010-10-11 03:42:50 PDT
Created attachment 482212 [details] [diff] [review]
Proposed patch

This fixes the progress listener exception by remembering that the listener was adding using a JS property because the XBL field gets reset.

In order to demonstrate that this fixes test_bug369370.html you of course need to reapply attachment 481165 [details] [diff] [review].
Comment 7 neil@parkwaycc.co.uk 2010-10-11 04:07:51 PDT
Created attachment 482219 [details] [diff] [review]
Alternate patch

In the previous patch I moved the code to make the reuse of the xw variable clearer. This version of the patch is much smaller so you may prefer it.
Comment 8 Bruno 'Aqualon' Escherl 2010-10-11 08:00:53 PDT
(In reply to comment #6)
> Created attachment 482212 [details] [diff] [review]
> Proposed patch
This fixes the test issue and the error in the console for me (Linux x86_64 debug build).
Comment 9 Ian Neal 2010-10-11 16:53:49 PDT
Comment on attachment 482212 [details] [diff] [review]
Proposed patch

r=me for this patch and relanding the backed out patch
Comment 10 neil@parkwaycc.co.uk 2010-10-13 04:42:55 PDT
Pushed changeset d3d16f77aebb to comm-central.
Comment 11 Serge Gautherie (:sgautherie) 2010-10-13 13:09:31 PDT
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1286987577.1286989950.15836.gz&fulltext=1
Linux comm-central-trunk debug test mochitest-other on 2010/10/13 09:32:57
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1286993085.1286994605.6815.gz&fulltext=1
OS X 10.5 comm-central-trunk debug test mochitest-other on 2010/10/13 11:04:45
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1286983082.1286986372.31690.gz&fulltext=1
WINNT 5.2 comm-central-trunk debug test mochitest-other on 2010/10/13 08:18:02

No more exception.

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1286973999.1286976575.20231.gz
Linux comm-central-trunk debug test mochitests-1/5 on 2010/10/13 05:46:39
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1286973253.1286975693.15863.gz
OS X 10.5 comm-central-trunk debug test mochitests-1/5 on 2010/10/13 05:34:13
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1286980344.1286983009.15259.gz
WINNT 5.2 comm-central-trunk debug test mochitests-1/5 on 2010/10/13 07:32:24

No more test failure.

V.Fixed
Comment 12 Justin Wood (:Callek) 2010-10-13 13:47:16 PDT
Comment on attachment 482212 [details] [diff] [review]
Proposed patch

Tests not failing, feedback+

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