Closed
Bug 1374892
Opened 6 years ago
Closed 6 years ago
Consider removing the nsISHistoryListener exception throwing machinery that shows up in Speedometer
Categories
(Core :: DOM: Navigation, defect)
Core
DOM: Navigation
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
After applying attachment 8879779 [details] [diff] [review] and running Speedometer you will see the following invisible exceptions logged when running Speedometer: Exception: JavaScript component does not have a method named: "OnLengthChanged"'JavaScript component does not have a method named: "OnLengthChanged"' when calling method: [nsISHistoryListener::OnLengthChanged], Exception: JavaScript component does not have a method named: "OnIndexChanged"'JavaScript component does not have a method named: "OnIndexChanged"' when calling method: [nsISHistoryListener::OnIndexChanged], Michael, what are these? Do you know? They are neither implemented in <https://searchfox.org/mozilla-central/rev/714606a8145636d93b116943d3a65a6a49d2acf8/browser/components/sessionstore/content/content-sessionStore.js#253> nor in <https://searchfox.org/mozilla-central/source/browser/components/sessionstore/ContentRestore.jsm#334>.
Flags: needinfo?(michael)
Comment 1•6 years ago
|
||
(In reply to :Ehsan Akhgari (needinfo please, extremely long backlog) from comment #0) > Michael, what are these? Do you know? Yes, these are extra methods on the nsISHistoryListener which were added by freesamael for bug 1310761. They are used in order to listen for other changes to the local SHistory which the parent process might need to be notified about. In SessionStore, we don't need to listen to them, as they are extra. We might want to implement them anyway however, if exception handling is slower, or we might want to remove them if they won't be required by the future plans for GroupedSHistory (I'm not sure what the plans are for that - ni?-ing freesamael for that).
Flags: needinfo?(michael) → needinfo?(sawang)
Comment 2•6 years ago
|
||
I don't think we have any plans for GroupSHistory... but could we just add the 2 functions to session store, and probably also mobile/android/chrome/content/browser.js, to prevent exceptions?
Flags: needinfo?(sawang)
Assignee | ||
Comment 3•6 years ago
|
||
OK, I'll add some dummy methods to make us not throw exceptions here. For some reason I didn't see the C++ consumer of this interface before!
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → ehsan
Assignee | ||
Comment 4•6 years ago
|
||
Attachment #8880231 -
Flags: review?(sawang)
Updated•6 years ago
|
Attachment #8880231 -
Flags: review?(sawang) → review+
Pushed by eakhgari@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d7420c18c12e Implement nsISHistoryListener::OnLengthChanged/OnIndexChanged on the JS implementations to avoid generating silent exceptions when Gecko calls these functions; r=freesamael
Comment 6•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d7420c18c12e
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•