Closed Bug 603228 Opened 14 years ago Closed 14 years ago

"Exception / NS_ERROR_FAILURE / nsIWebProgress.addProgressListener / notification.xml :: addProgressListener :: line 66" caused by some part of bug 595810

Categories

(SeaMonkey :: UI Design, defect)

Product:
SeaMonkey
Component:
UI Design
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
seamonkey2.1b2

People

(Reporter: sgautherie, Assigned: neil)

References

(
URL
)

Details

(Keywords: regression)

Attachments

(2 files)

Proposed patch
3.13 KB, patch
iannbugzilla
: review+
Callek
: feedback+
Details | Diff | Splinter Review
Alternate patch
2.08 KB, patch
Details | Diff | Splinter Review 
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>
}
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...
(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
Assignee: nobody → neil
(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 :-|
(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.
This exception, although unexpected, is unrelated to the test failure.
Attached patch Proposed patch — — Splinter Review 
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].
Attachment #482212 - Flags: review?(iann_bugzilla)
Attachment #482212 - Flags: review?(bugspam.Callek)
Attached patch Alternate patch — — Splinter Review 
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.
(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 on attachment 482212 [details] [diff] [review]
Proposed patch

r=me for this patch and relanding the backed out patch
Attachment #482212 - Flags: review?(iann_bugzilla) → review+
Attachment #482212 - Flags: review?(bugspam.Callek) → feedback?(bugspam.Callek)
Pushed changeset d3d16f77aebb to comm-central.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → VERIFIED
Target Milestone: --- → seamonkey2.1b2
Comment on attachment 482212 [details] [diff] [review]
Proposed patch

Tests not failing, feedback+
Attachment #482212 - Flags: feedback?(bugspam.Callek) → feedback+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: