Closed Bug 796217 Opened 7 years ago Closed 7 years ago

Refactor Webapps.jsm and related files

Categories

(Core Graveyard :: DOM: Apps, defect)

Product:
Core Graveyard
Component:
DOM: Apps
Version:
Other Branch
Type:
defect
Priority:
Not set

Tracking

(firefox18 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla18
Tracking Flags:
Tracking Status
firefox18 --- fixed

People

(Reporter: fabrice, Assigned: fabrice)

Details

Attachments

(2 files, 2 obsolete files)

Part 1: move DOMApplicationManifest outside of Webapps.jsm
26.79 KB, patch
gwagner
: review+
Details | Diff | Splinter Review
Part 2: refactor PermissionTable.jsm
19.78 KB, patch
ddahl
: review+
Details | Diff | Splinter Review 
Webapps.jsm has grown a bit too much... Let's trim down some fat.
In this part, we move the DOMApplicationManifest into AppsUtils.jsm and rename it ManifestHelper.
Renaming PermissionTable.jsm to PermissionsManager.jsm and exporting just what we need in Webapps.jsm which is the only user so far.

Try run at https://tbpl.mozilla.org/?tree=Try&rev=7a90d8c8c23e with both parts.
OS: Linux → All
Hardware: x86_64 → All
Fixed a bug in the way we call uninstall() from installPermissions.

New try run at https://tbpl.mozilla.org/?tree=Try&rev=e2e5a37830ab
Attachment #666780 - Attachment is obsolete: true
Passes tests locally, I made a new push to try to be sure:
https://tbpl.mozilla.org/?tree=Try&rev=9bdd0f3bfa2c
Attachment #666788 - Attachment is obsolete: true
Attachment #667091 - Flags: review?(ddahl)
Attachment #666777 - Flags: review?(anygregor)
Comment on attachment 667091 [details] [diff] [review]
Part 2: refactor PermissionTable.jsm

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

The only thing I would caution against is calling the exported object "PermissionsManager", I think there will be some confusion with nsIPermsisionManager. I would name the jsm and the exported object "PermissionsInstaller" for clarity.
Attachment #667091 - Flags: review?(ddahl) → review+
Attachment #666777 - Flags: review?(anygregor) → review+
https://hg.mozilla.org/mozilla-central/rev/2cc0da49ccb5
https://hg.mozilla.org/mozilla-central/rev/eb6641924764
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.