jarPrefix is currently a concatenation of appId and inMozBrowser as cookieJar was removed in Bug 1182347. Bug 1179985 seeks to replace this with origin/originSuffix. Once the remaining consumers (Bug 1165219, Bug 1165277, and Bug 1165217) are migrated to originSuffix, jarPrefix can be removed.

Sample patch. Will do additional testing once dependent bugs are fixed.

Yoshi, would you mind to let me know if I missed any dependent bugs?

Yeah, I think the bugs you listed are enough.

Bug 1165217 is just about fixing nsIUsageCallback. Quota manager uses jarPrefix + originNoSuffix to find storage directories (for example: <profile>/storage/persistent/1007+f+app+++system.gaiamobile.org). So we need to fix it/convert it somehow before jarPrefix can be removed.

Thanks Jan, added.

I rebased and built, try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3a2927ae61782be1ec1bae55dd56aa841271051c

the WIP patch covered everything except the ActorParent.cpp GetJarPrefix function.

rebased, fixed build issues, try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f883ae8a26b966dd75d3367a21f902d3cbc204db

this is the updated WIP rebased and fixed. it completely removes the GetJarPrefix code. The only thing that is of concern to me are the changes I made CreateDirectoryMetadata to remove the calls to GetJarPrefix. I assume that the suffix created from the origin attributes should be sufficient since it includes the origin attributes that were being fed into GetJarPrefix. I'm just not sure what changing the metadata string will do to stored state.

(In reply to Dave Huseby [:huseby] from comment #9) > Created attachment 8799544 [details] [diff] [review] > WIP patch > > this is the updated WIP rebased and fixed. it completely removes the > GetJarPrefix code. The only thing that is of concern to me are the changes > I made CreateDirectoryMetadata to remove the calls to GetJarPrefix. I > assume that the suffix created from the origin attributes should be > sufficient since it includes the origin attributes that were being fed into > GetJarPrefix. I'm just not sure what changing the metadata string will do > to stored state. Please don't remove it from ActorsParent.cpp The code there handles reading/upgrading of .metadata (version 1) files in user profile which were created in an older FF. I forked the GetJarPrefix() method because I knew it will be removed at some point.

Jan, Do you have a plan for removing it from ActorsParent.cpp?

updated patch to revert the removal of the function from ActorsParent.cpp. Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a5fa6014db2b145456d4a30c32c4c19653d33d26

updated the patch

(In reply to Dave Huseby [:huseby] from comment #11) > Jan, > > Do you have a plan for removing it from ActorsParent.cpp? Why would I want to remove it ? Those .metadata files were created in an older FF and we need to read/upgrade them in newer FF. The old format didn't use origin attributes, it used origin and group prefixed with a jar prefix. The code is self contained, it's not exported anywhere, I don't see why we need a plan to remove it.

Comment on attachment 8799578 [details] [diff] [review] Removes the jar prefix code that is no longer needed. Review of attachment 8799578 [details] [diff] [review]: ----------------------------------------------------------------- I'm a reviewer for dom/quota, that's why I commented on ActorsParent.cpp changes, but I'm not a reviewer for dom/caps. You need to find some one else, sorry about that.

Comment on attachment 8799578 [details] [diff] [review] Removes the jar prefix code that is no longer needed. Hi Dan, According to the modules list, you're a peer for DOM/Security/Caps which this patch is a part of. Would you mind reviewing it for me? Thanks, --dave

Comment on attachment 8799578 [details] [diff] [review] Removes the jar prefix code that is no longer needed. Review of attachment 8799578 [details] [diff] [review]: ----------------------------------------------------------------- r=dveditz I worry about changing the binary compatibility of nsIPrincipal and nsIScriptSecurityManager because in the past external apps hooked into those and we caused crashes. Maybe that doesn't happen as much anymore, but we should keep an eye out for hints of it when we hit Beta.

(In reply to Daniel Veditz [:dveditz] from comment #17) > we should keep an eye out for hints of it when we hit Beta. Can you elaborate on what that looks like? I'd like to better understand what you mean by that.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bca19125cf303bc9982019c3bbee8f03597460f7

(In reply to Dave Huseby [:huseby] from comment #18) > (In reply to Daniel Veditz [:dveditz] from comment #17) > > we should keep an eye out for hints of it when we hit Beta. > > Can you elaborate on what that looks like? I'd like to better understand > what you mean by that. New crashes, possibly in 3rd party .dll's, possibly in principal or ScriptSecurityManager code. It /should/ be OK.

The try push looks good.

already r+, rebased. try push looks good.

Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f19085b4d55b Eliminate nsIPrincipal::jarPrefix. r=dveditz