Closed Bug 1307456 Opened 7 years ago Closed 7 years ago

Remove WebPackagedApp code

Categories

(Core :: Networking, defect)

Product:
Core
Component:
Networking
Version:
47 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla52
Tracking Flags:
Tracking Status
firefox52 --- fixed

People

(Reporter: valentin, Assigned: valentin)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-next])

Attachments

(3 files)

Bug 1307456 - Remove dom/newapps
58 bytes, text/x-review-board-request
baku
: review+
Details
Bug 1307456 - Remove PackagedAppService/Verifier
58 bytes, text/x-review-board-request
bagder
: review+
Details
Bug 1307456 - Remove packaged app related changes to nsMultiMixedConv
58 bytes, text/x-review-board-request
bagder
: review+
Details 
      No description provided.
iirc, this was not just for packaged apps, but also intended as a bundling mechanism for web resources (like "everything you need for this slide deck"). At least I'm pretty sure that was a motivation for Jonas.
That is true, however, a lot of the code was specifically dependent on assumptions that were made for b2g - especially the signature verifying bits.
And while I think this is a pretty interesting format/feature, I don't see any traction for it to become a web standard anytime soon. The package format and addressing scheme are still a very good idea.
We can add the code back in at a later point if there's someone willing to use it.
Valentin, can you please also remove dom/newapps when you get to this?  It seems like that directory is only used by PackagedAppDownloader::InstallSignedPackagedApp().  Thank you!
Comment on attachment 8803107 [details]
Bug 1307456 - Remove PackagedAppService/Verifier

https://reviewboard.mozilla.org/r/87322/#review86918
Attachment #8803107 - Flags: review?(daniel) → review+
Comment on attachment 8803108 [details]
Bug 1307456 - Remove packaged app related changes to nsMultiMixedConv

https://reviewboard.mozilla.org/r/87324/#review86920

Less is more! Just a little question but still r+

::: netwerk/streamconv/converters/nsMultiMixedConv.cpp:760
(Diff revision 2)
>  NS_IMETHODIMP
>  nsMultiMixedConv::OnStopRequest(nsIRequest *request, nsISupports *ctxt,
> -                                nsresult aStatus) {
> -
> -    nsresult rv = NS_OK;
> -
> +                                nsresult aStatus)
> +{
> +    if (mToken.IsEmpty()) { // no token, no love.
> +        return NS_ERROR_FAILURE;

This seems simpler and slightly more widely covering than the code did before, even in the non-packagedapp case. Just wanted to make sure this is intended.
Attachment #8803108 - Flags: review?(daniel) → review+
Comment on attachment 8803106 [details]
Bug 1307456 - Remove dom/newapps

https://reviewboard.mozilla.org/r/87320/#review88532
Attachment #8803106 - Flags: review?(amarchesini) → review+
Comment on attachment 8803108 [details]
Bug 1307456 - Remove packaged app related changes to nsMultiMixedConv

https://reviewboard.mozilla.org/r/87324/#review86920

> This seems simpler and slightly more widely covering than the code did before, even in the non-packagedapp case. Just wanted to make sure this is intended.

This is how the code used to look like before we made it more complex with the packaged app use cases.
Pushed by valentin.gosu@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/75890b9806e8
Remove dom/newapps r=baku
https://hg.mozilla.org/integration/autoland/rev/e51be7bba9f5
Remove PackagedAppService/Verifier r=bagder
https://hg.mozilla.org/integration/autoland/rev/0f50ce32748d
Remove packaged app related changes to nsMultiMixedConv r=bagder
No longer blocks: 1369194
You need to log in before you can comment on or make changes to this bug.