Closed
Bug 1179058
Opened 9 years ago
Closed 9 years ago
~18,000 instances of 'WARNING: NS_ENSURE_SUCCESS(rv, true) failed with result 0x80570030' emitted from docshell/base/nsDocShell.cpp during linux64 debug testing
Categories
(Core :: DOM: Navigation, defect)
Core
DOM: Navigation
Tracking
()
RESOLVED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: erahm, Assigned: nhnt11)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
5.91 KB,
patch
|
adw
:
review+
|
Details | Diff | Splinter Review |
> 17724 [NNNNN] WARNING: NS_ENSURE_SUCCESS(rv, true) failed with result 0x80570030: file docshell/base/nsDocShell.cpp, line 11758 This is the #1 most verbose warning [1] during linux64 debug testing. It seems to have appeared in the last week. 0x80570030 is NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED. It shows up primarily during e10s tests: > mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-4-bm122-tests1-linux64-build6.txt:3997 > mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-3-bm115-tests1-linux64-build6.txt:3643 > mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-5-bm122-tests1-linux64-build10.txt:3497 > mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-1-bm115-tests1-linux64-build2.txt:2757 > mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-2-bm123-tests1-linux64-build1.txt:2674 > mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-browser-chrome-3-bm53-tests1-linux64-build6.txt:562 > mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-browser-chrome-1-bm52-tests1-linux64-build5.txt:384 > mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-browser-chrome-2-bm122-tests1-linux64-build13.txt:209 > mozilla-central_ubuntu64_vm-debug_test-mochitest-other-bm115-tests1-linux64-build5.txt:1 This warning was introduced in bug 1118285 on 2015-06-29. My best guess is that the JS object implementing nsIXULBrowserWindow referenced in |nsContentTreeOwner::ShouldAddToSessionHistory| [2] has not defined a |shouldAddToSessionHistory| function. [1] https://hg.mozilla.org/mozilla-central/annotate/291614a686f1/docshell/base/nsDocShell.cpp#l11758 [2] https://hg.mozilla.org/mozilla-central/annotate/291614a686f1/xpfe/appshell/nsContentTreeOwner.cpp#l472
Assignee | ||
Comment 1•9 years ago
|
||
This patch fixes the warnings on my local debug build. Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c77f1156434
Comment 2•9 years ago
|
||
Hmm, this won't work as expected since the content process's "copy" of NewTabURL at runtime will be different from chrome's. An overridden URL in one won't be seen in the other. (I verified by dump()ing NewTabURL.get() in tab-content.js after overriding the URL using the (chrome) browser console. You can use the content toolbox too.) We broke this in bug 1118285 but didn't realize it. Before that, docshell checked browser.newtab.url itself, right? Prefs storage is shared across the two processes, so it worked. I think we're going to have to have NewTabURL send a message to content whenever the URL is overridden just so that this WebBrowserChrome.shouldAddToSessionHistory() can use it. If necessary, in the meantime we can land an empty implementation of shouldAddToSessionHistory just to make the warnings go away. needinfo'ing Florian for his thoughts.
Flags: needinfo?(florian)
Updated•9 years ago
|
Attachment #8628487 -
Flags: review?(adw)
Reporter | ||
Comment 3•9 years ago
|
||
(In reply to Drew Willcoxon :adw from comment #2) > Hmm, this won't work as expected since the content process's "copy" of > NewTabURL at runtime will be different from chrome's. An overridden URL in > one won't be seen in the other. (I verified by dump()ing NewTabURL.get() in > tab-content.js after overriding the URL using the (chrome) browser console. > You can use the content toolbox too.) > > We broke this in bug 1118285 but didn't realize it. Before that, docshell > checked browser.newtab.url itself, right? Prefs storage is shared across > the two processes, so it worked. > > I think we're going to have to have NewTabURL send a message to content > whenever the URL is overridden just so that this > WebBrowserChrome.shouldAddToSessionHistory() can use it. If necessary, in > the meantime we can land an empty implementation of > shouldAddToSessionHistory just to make the warnings go away. > > needinfo'ing Florian for his thoughts. So this is the rare instance where a warning is actually useful. I don't think we should silence it, perhaps we should backout bug 1118285 instead if a fix is going to take some time.
Comment 4•9 years ago
|
||
(In reply to Drew Willcoxon :adw from comment #2) > I think we're going to have to have NewTabURL send a message to content > whenever the URL is overridden just so that this > WebBrowserChrome.shouldAddToSessionHistory() can use it. If necessary, in > the meantime we can land an empty implementation of > shouldAddToSessionHistory just to make the warnings go away. Assuming the session history is handled by the content process and the warning isn't just noise, I'm afraid we'll have to to this, yes.
Flags: needinfo?(florian)
Assignee | ||
Comment 5•9 years ago
|
||
This reverts the interface changes in bug 1118285 and makes nsDocShell check specifically for about:newtab. This is a short term fix. For the long term we likely want NewTabURL to pass messages to content when the URL changes.
Attachment #8628487 -
Attachment is obsolete: true
Attachment #8628974 -
Flags: review?(adw)
Updated•9 years ago
|
Attachment #8628974 -
Flags: review?(adw) → review+
Comment 6•9 years ago
|
||
(In reply to Drew Willcoxon :adw from comment #2) > Hmm, this won't work as expected since the content process's "copy" of > NewTabURL at runtime will be different from chrome's. Good catch. Probably worth filing a bug to somehow make NewTabURL.jsm throw an exception or warning when used from a content process.
Assignee | ||
Comment 7•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b312442bfdd
Keywords: checkin-needed
Comment 9•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ea4f4e3b0090
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in
before you can comment on or make changes to this bug.
Description
•