114092 - implement nsChromeRegistry::UninstallPackage

Status: RESOLVED INCOMPLETE

(Core Graveyard :: RDF, defect)

Description

[Chase Tingley]

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.

Comment 1

[Chase Tingley]

Attachment: basic implementation

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.

Comment 2

[Chase Tingley]

Targeting aggressively -- I'd really like to get this in 0.9.7 because of its
use to 3rd-party developers.

Comment 3

[Chase Tingley]

who was I kidding?  -> 0.9.8

Comment 4

[Vidar Haarr (not reading bugmail)]

This bug is targetted for 0.9.8, and should be moved.
Any chance of getting r= / sr= on the proposed patch ?

Comment 5

[Chase Tingley]

retargeting.

Comment 6

[Vidar Haarr (not reading bugmail)]

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 ?

Comment 7

[Chase Tingley]

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.

Comment 8

[Håkan Waara]

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 9

[Chris Waterson]

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

Comment 10

[Syd Logan]

Joy, this couldn't be coming at a better time.

Comment 11

[Chase Tingley]

Attachment: revised per waterson

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

Comment 12

[benc]

tever is not RDF QA anymore

Comment 13

[(not currently active) Ted Mielczarek]

Apparently this code has ceased to exist.