Neil and Benjamin may know something about this. At least it Neil did the patch in bug 281988, and Benjamin reviewed it. XPFE chrome registry bugs are not likely to get good traction in the RDF component, is there a better place to put those?
Created attachment 252893 [details] [diff] [review] Proposed patch * Broke out the xpcNativeWrapper test into a separate method * Call it after programmatically installing chrome * Call it after loading the profile * Call it after checking for new chrome This also fixes the case of starting Profile Manager with a new installation.
Isn't it wasteful to call FlagXPCNativeWrappers from InstallProvider where we know exactly which packages have been added? If we install a couple of extensions at once we might end up going through the entire list of packages a dozen times. However, this case is probably rare enough so that it doesn't need optimizing...
(In reply to comment #4) >Isn't it wasteful to call FlagXPCNativeWrappers from InstallProvider where we >know exactly which packages have been added? Actually I don't think we do. In fact, I'm not convinced that the provider type is accurate.
Comment on attachment 252893 [details] [diff] [review] Proposed patch This fixes the problem, looks good otherwise as well. What I would prefer you to do however: add a flag to disable FlagXPCNativeWrappers calls from InstallProvider. You could then set this flag when CheckForNewChrome executes during the initialization - there will be an FlagXPCNativeWrappers call after it anyways.
(In reply to comment #6) >add a flag to disable FlagXPCNativeWrappers calls from InstallProvider. I used the existing mBatchInstallFlushes flag, which is set by ProcessNewChromeBuffer, which is called from CheckForNewChrome.
I see, I missed that part. Please ignore comment 4 then. However, you are assuming that a CheckForNewChrome call will always be followed by FlagXPCNativeWrappers - you should add a comment on this, just in case somebody decides to call CheckForNewChrome from a different place..
Speaking of mBatchInstallFlushes - there are quite a few return statements between the set and unset of this flag. Under some conditions we might end up with mBatchInstallFlushes being permanently set which will both prevent this patch from working and make chrome changes disappear after a restart. I guess this needs a separate bug.
Comment on attachment 252893 [details] [diff] [review] Proposed patch sr=jag. Feel free to move the mBatchInstallFlushes toggling out of |ProcessNewChromeBuffer()|
Created attachment 253309 [details] [diff] [review] Patch for checkin [jag saw this version of the patch] I guess this is needed on branches too, right?
Fix checked in to the trunk.
Comment on attachment 253309 [details] [diff] [review] Patch for checkin a=me for 1.1.1 and 1.0.8
Fix checked in to the branches.