1374892 - Consider removing the nsISHistoryListener exception throwing machinery that shows up in Speedometer

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[:ehsan akhgari]

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>.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

(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).

Comment 2

[Samael Wang [:freesamael] (away for now)]

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?

Comment 3

[:ehsan akhgari]

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!

Comment 4

[:ehsan akhgari]

Attachment: Implement nsISHistoryListener::OnLengthChanged/OnIndexChanged on the JS implementations to avoid generating silent exceptions when Gecko calls these functions

Comment 5

[Pulsebot]

Pushed by [e-mail]:
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

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d7420c18c12e