248044 - JAR protocol handler should be moved out of necko and into libjar

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Darin Fisher]

JAR protocol handler should be moved out of necko and into libjar.

Justification:

The JAR protocol handler requires CAPS to get the channel owner from
the script security context in form of an nsIPrincipal.

I've always wondered why the JAR protocol even lives in Necko.  It seems
like it could be moved into libjar.  Then libjar could depend on necko, etc.

Necko doesn't have much use for the JAR protocol internally.  It's on par
with the CHROME protocol, which is not included in Necko either.

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

-> me and targeting optimistically

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

I'll attach a patch showing the changes I made to the moved files separately

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: changes to moved files

Comment 4

[Darin Fisher]

Comment on attachment 151829 [details]
changes to moved files

>+    nsCOMPtr<nsIStandardURL> stdURL(do_CreateInstance("@mozilla.org/network/standard-url;1"));

use NS_STANDARDURL_CONTRACTID instead?	it makes life easier if we ever
wish to change consumers over to another nsIStandardURL implementation
while maintaining the old one.

Comment 5

[Darin Fisher]

Comment on attachment 151828 [details] [diff] [review]
patch

one comment: make sure that libjar still builds correctly in standalone mode. 
we probably do not want standalone libjar to include the jar protocol handler.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

standalone libjar just uses the files listed at
http://lxr.mozilla.org/seamonkey/source/modules/libjar/objs.mk#38 afaict... so
it should continue to work. any idea how I can build it?

Comment 7

[Darin Fisher]

cc'ing dveditz

dan: can you confirm that this is not going to cause problems for the standalone
version of libjar?  thanks!

Comment 8

[Daniel Veditz [:dveditz]]

The "standalone" version for the installer is really just the basic nsZipArchive
code, should be fine to move all this stuff into the Mozilla component.

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

urg, this patch is incomplete, it doesn't remove the dirs from allmakefiles.sh,
and the necko_jar.xpt file from various installer file lists

I'll make a new patch sometime during the next few days, feel free to review the
rest of the patch in the meantime :)

Comment 10

[Benjamin Smedberg]

Comment on attachment 151828 [details] [diff] [review]
patch

With allmakefiles.sh changes, packaging xpt files, and darin's comments, r=me

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: changes to existing files

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

Moved files:
netwerk/protocol/jar/src/nsJARProtocolHandler.cpp
netwerk/protocol/jar/src/nsJARURI.cpp
netwerk/protocol/jar/src/nsJARURI.h
netwerk/protocol/jar/src/nsJARChannel.h
netwerk/protocol/jar/src/nsJARProtocolHandler.h
netwerk/protocol/jar/src/nsJARChannel.cpp
netwerk/protocol/jar/public/nsIJARURI.idl
netwerk/protocol/jar/public/nsIJARProtocolHandler.idl
netwerk/protocol/jar/public/nsIJARChannel.idl

All with an identical name to modules/libjar.

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: changes to moved files, v2

updated per darin's comment

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 158774 [details] [diff] [review]
changes to moved files, v2

In addition to the two patches here, I'll remove all of netwerk/modules/jar,
and get someone to copy the files as described above.

Comment 15

[Darin Fisher]

Comment on attachment 158758 [details] [diff] [review]
changes to existing files

looks like you also need to remove the "#undef NECKO_PROTOCOL_jar" line from
necko-config.h.in, right?

Comment 16

[Brendan Eich [:brendan]]

Repository copy per comment 12 is done now.

/be

Comment 17

[Christian :Biesinger (don't email me, ping me on IRC)]

patch checked in.

leaving open as a reminder to remove the old files, once tinderboxes are green

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

tboxes are green, old files removed.

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This seems to have broken Camino.

Comment 20

[Christian :Biesinger (don't email me, ping me on IRC)]

sorry about the camino breakage :( should be fixed now, thanks to pinkerton.