Closed Bug 114092 Opened 20 years ago Closed 12 years ago

implement nsChromeRegistry::UninstallPackage

Categories

(Core Graveyard :: RDF, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED INCOMPLETE
mozilla1.0.1

People

(Reporter: tingley, Assigned: tingley)

Details

Attachments

(2 obsolete files)

Tragically, this function is stubbed.

http://lxr.mozilla.org/seamonkey/source/rdf/chrome/src/nsChromeRegistry.cpp#2393

I have code to un-stub it rotting in my tree.  I'd sort of like to land bug
70966 first, but it's not essential.
Status: NEW → ASSIGNED
Attached patch basic implementation (obsolete) — Splinter Review
It's possible that this doesn't do all the things it needs to do, but it's
sufficient to clean chrome.rdf and any overlays.
Targeting aggressively -- I'd really like to get this in 0.9.7 because of its
use to 3rd-party developers.
Keywords: patch, review
Target Milestone: --- → mozilla0.9.7
who was I kidding?  -> 0.9.8
Target Milestone: mozilla0.9.7 → mozilla0.9.8
This bug is targetted for 0.9.8, and should be moved.
Any chance of getting r= / sr= on the proposed patch ?
retargeting.
Target Milestone: mozilla0.9.8 → mozilla1.0
Keywords: mozilla1.0
I have no experince with this code, but the patch looks fine to me :)

Should we add helpwanted and nsbeta1 to this ? I'm sure NS wants this functionality.

Anyone care to review the patch ?
I have no idea if this patch still applies cleanly (I'll check).

My previous efforts to find reviews for this code were completely unsuccessful.
 Once I check the patch, I'll make another attempt.  I'm not overly optimistic.
Keywords: mozilla1.0, patch, review
Target Milestone: mozilla1.0 → mozilla1.0.1
Perhaps brendan can do a catch-all review on this, if waterson (or some RDF
expert) sr=s.  Let's get some traction on this patch.
Comment on attachment 61092 [details] [diff] [review]
basic implementation

>Index: nsChromeRegistry.cpp
>+  nsCOMPtr<nsIRDFDataSource> dataSource;
>+  rv = nsComponentManager::CreateInstance(kRDFXMLDataSourceCID,
>+                                          nsnull,
>+                                          NS_GET_IID(nsIRDFDataSource),
>+                                          (void**) getter_AddRefs(dataSource));

Use do_CreateInstance instead?

>+  if (NS_FAILED(rv)) return rv;

Verify dataSource != 0 instead?

>+  nsCOMPtr<nsIRDFRemoteDataSource> remote = do_QueryInterface(dataSource, &rv);
>+  if (NS_FAILED(rv)) return rv;

Verify remove != 0 instead?

>+  // Synchronously load contents.rdf.
>+  // XXX Ignore old-style manifest.rdf.  Packages that are old enough to still
>+  // be using it shouldn't be calling this anyway (or at least shouldn't be
>+  // expecting it to work).
>+  nsCAutoString pkgName; pkgName.AssignWithConversion(aPackageName);
>+  nsCAutoString key(NS_LITERAL_CSTRING("chrome://") + pkgName +
>+                    NS_LITERAL_CSTRING("/content/contents.rdf"));
>+  remote->Init(key.get());
>+  remote->Refresh(PR_TRUE);
>+
>+  // Uninstall dynamic overlays
>+  rv = UpdateDynamicDataSources(dataSource, PR_TRUE, aUseProfile, PR_TRUE);
>+  if (NS_FAILED(rv)) return rv;
>+
>+  // Remove from all-packages.rdf
>+  rv = UninstallProvider(nsCAutoString("package"), aPackageName,
>+                         aUseProfile);
>+  if (NS_FAILED(rv)) return rv;
>+
>+  return NS_OK;

I'm assuming that we write the datasource back out at this point because there
are no live references left to it? Might be worth a comment if that's the
case...
Attachment #61092 - Flags: superreview+
Joy, this couldn't be coming at a better time.
Attached patch revised per waterson (obsolete) — Splinter Review
mozdev is acting freaky so I have not tested this yet to verify that it still
works, but I'm posting this anyway to strike a blow against my own horrid
laziness.

The CreateInstance() call was to match the local dialect -- shall I go ahead
and convert the file over to do_CreateInstance?  (There's a lot of stuff in the
chrome code that's not quite cutting edge stylistically.)
Attachment #61092 - Attachment is obsolete: true
Attachment #89481 - Attachment is obsolete: true
tever is not RDF QA anymore
QA Contact: tever → nobody
QA Contact: nobody → rdf
Apparently this code has ceased to exist.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → INCOMPLETE
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.