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

RESOLVED FIXED in Firefox 56

Status

()

Core
Document Navigation
RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

(Blocks: 1 bug)

unspecified
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

8 months ago
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)
(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)
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

8 months 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

8 months ago
Assignee: nobody → ehsan
(Assignee)

Comment 4

8 months ago
Created attachment 8880231 [details] [diff] [review]
Implement nsISHistoryListener::OnLengthChanged/OnIndexChanged on the JS implementations to avoid generating silent exceptions when Gecko calls these functions
Attachment #8880231 - Flags: review?(sawang)
Attachment #8880231 - Flags: review?(sawang) → review+

Comment 5

8 months ago
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

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d7420c18c12e
Status: NEW → RESOLVED
Last Resolved: 8 months 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.