Closed Bug 1167224 Opened 5 years ago Closed 5 years ago

remove getMostRecentBrowserWindow() from nsIBrowserGlue and use RecentWindow in nsBrowserGlue.js

Categories

(Firefox :: General, defect)

defect
Not set
Points:
2

Tracking

()

RESOLVED FIXED
Firefox 41
Tracking Status
firefox41 --- fixed

People

(Reporter: mak, Assigned: abdelrahman, Mentored)

References

Details

(Whiteboard: [good first bug][lang=js])

Attachments

(1 file)

getMostRecentBrowserWindow() can be removed from the interface, I couldn't find any external consumers for it, in add-ons or in our code.

In any case, the scope here is to audit these consumers:
http://mxr.mozilla.org/mozilla-central/search?string=getMostRecentBrowserWindow%28%29
And ensure they are using either the addon-sdk version of this method, or the RecentWindow.jsm version.
No consumer should use the nsIBrowserGlue version. Should be easy to cross check by also looking for nsIBrowserGlue instantiations: http://mxr.mozilla.org/mozilla-central/search?string=nsIBrowserGlue.

Once verified the only consumers are internal to nsBrowserGlue.js, should convert those internal consumers to use RecentWindow.getMostRecentBrowserWindow().

Then remove getMostRecentBrowserWindow() method from nsBrowserGlue.js and remove its definition in nsIBrowserGlue.idl (remember to change the uuid)
Blocks: 1148475
Flags: firefox-backlog+
Flags: qe-verify-
Summary: remove getMostRecentWindow() from nsIBrowserGlue and use RecentWindow in nsBrowserGlue.js → remove getMostRecentBrowserWindow() from nsIBrowserGlue and use RecentWindow in nsBrowserGlue.js
I want to know more about changing the UUID.
I read this (https://developer.mozilla.org/en-US/docs/Generating_GUIDs) and I think we just notify that we have changed nsIBrowserGlue.idl, right?
Assignee: nobody → a.ahmed1026
Status: NEW → ASSIGNED
Attachment #8609857 - Flags: review?(mak77)
yes, every time you change an idl, you are basically changing the interface, you need to bump the uuid of the interface otherwise components using a different version of it might end up using a broken virtual table, and crash (if they are cpp) or just malfunction.

There is an additional way to generate an UUID that is not documented on MDN, that is ./mach uuid
Comment on attachment 8609857 [details] [diff] [review]
rev 1 - using RecentWindow in nsBrowserGlue.js

Review of attachment 8609857 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good, thank you! I will push this to Try server to run some tests. Since the change is trivial I will just trigger OSX.

Btw, one unimportant thing, for marking the reviewer in the commit message we usually use the IRC nickname (most have it in the bugzilla name as [:nickname]) if available, in my case it would be r=mak. That way I would get a ping on irc when the patch lands. It's not very important though, so no worries.
Attachment #8609857 - Flags: review?(mak77) → review+
https://hg.mozilla.org/mozilla-central/rev/79f4cf7c37a8
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
You need to log in before you can comment on or make changes to this bug.