1187159 - [PackagedAppService] Pass principal and flags from the requesting channel to the packaged app channel

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Valentin Gosu [:valentin] (he/him)]

Right now, when downloading a package, the channel used to download that is the SystemPrincipal. This isn't good, because the cache entry holding the metadata ends up in a different cache storage than the packaged resources.
Also, the flags used by the requesting channel should be applied to the packaged app channel, so if the user presses ctrl-shift-R, the request will bypass the cache, or if the docshell wants LOAD_NO_NETWORK_IO we don't hit the network.

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Attachment: Pass principal and flags from the requesting channel to the packaged app channel

Comment 2

[Valentin Gosu [:valentin] (he/him)]

Honza, do you think you could review this?

Comment 3

[Valentin Gosu [:valentin] (he/him)]

Attachment: Add mochitest for loading packaged apps (iframe+fetch+mozapp)

* * *
try: -b o -p linux64 -u all -t none

Comment 4

[Honza Bambas (:mayhemer)]

Comment on attachment 8641460 [details] [diff] [review]
Pass principal and flags from the requesting channel to the packaged app channel

Review of attachment 8641460 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/base/nsIPackagedAppService.idl
@@ +15,5 @@
>  
>  /**
>   * nsIPackagedAppService
>   */
>  [scriptable, builtinclass, uuid(77f9a34d-d082-43f1-9f83-e852d0173cd5)]

any iid change?

@@ +25,2 @@
>     *   - example: http://test.com/path/to/package!//resource.html
> +   * @aFlags - the load flags for the request

the doxygen works as following:

@param <paramname>
   description..

are the load flags merged or set?

::: netwerk/protocol/http/PackagedAppService.cpp
@@ -651,5 @@
> -  // needs to have a separate entry for anonymous channels.
> -  uint32_t extra_flags = 0;
> -  if (aInfo->IsAnonymous()) {
> -    extra_flags = nsIRequest::LOAD_ANONYMOUS;
> -  }

so, no more support for LOAD_ANONYMOUS unless set by the caller?

::: netwerk/protocol/http/PackagedAppService.h
@@ +22,5 @@
>  // which are either returned from the cache if they exist and are valid,
>  // or downloads the package.
>  // The package format is defined at:
>  //     https://w3ctag.github.io/packaging-on-the-web/#streamable-package-format
> +// Downloading the package is triggered by calling getResource(aURI, aInfo, aCallback)

omit or update the args too

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ca1510a96cc6
https://hg.mozilla.org/integration/mozilla-inbound/rev/78a61d56adae

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out together with bug 1190502: https://hg.mozilla.org/integration/mozilla-inbound/rev/56124e9589e1

23:29:41     INFO -  ==2020==ERROR: LeakSanitizer: detected memory leaks
23:29:41     INFO -  Direct leak of 96 byte(s) in 3 object(s) allocated from:
23:29:41     INFO -      #0 0x472231 in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
23:29:41     INFO -      #1 0x48b79d in moz_xmalloc /builds/slave/m-in-l64-asan-0000000000000000/build/src/memory/mozalloc/mozalloc.cpp:83
23:29:41     INFO -      #2 0x7fc61d0e1dfd in operator new /builds/slave/m-in-l64-asan-0000000000000000/build/src/obj-firefox/netwerk/base/../../dist/include/mozilla/mozalloc.h:186
23:29:41     INFO -      #3 0x7fc61d0e1dfd in mozilla::net::GetLoadContextInfo(nsIChannel*) /builds/slave/m-in-l64-asan-0000000000000000/build/src/netwerk/base/LoadContextInfo.cpp:68
23:29:41     INFO -      #4 0x7fc61d5833a6 in mozilla::net::nsHttpChannel::BeginConnect() /builds/slave/m-in-l64-asan-0000000000000000/build/src/netwerk/protocol/http/nsHttpChannel.cpp:5215
23:29:41     INFO -      #5 0x7fc61d58599f in mozilla::net::nsHttpChannel::OnProxyAvailable(nsICancelable*, nsIChannel*, nsIProxyInfo*, nsresult) /builds/slave/m-in-l64-asan-0000000000000000/build/src/netwerk/protocol/http/nsHttpChannel.cpp:5441
23:29:41     INFO -  -----------------------------------------------------
23:29:41     INFO -  Suppressions used:
23:29:41     INFO -    count      bytes template
23:29:41     INFO -       40        986 libc.so
23:29:41     INFO -      832      26576 nsComponentManagerImpl
23:29:41     INFO -       69       9384 mozJSComponentLoader::LoadModule
23:29:41     INFO -        1        384 pixman_implementation_lookup_composite
23:29:41     INFO -     7862     372704 libfontconfig.so
23:29:41     INFO -        2         48 PORT_Strdup_Util
23:29:41     INFO -        2         88 _PR_Getfd
23:29:41     INFO -      134      56872 libcairo.so
23:29:41     INFO -        1         32 libdl.so
23:29:41     INFO -     2313      41164 libglib-2.0.so
23:29:41     INFO -        2        144 libpulse.so
23:29:41     INFO -        1         40 libpulsecommon-1.1.so
23:29:41     INFO -        2         56 libresolv.so
23:29:41     INFO -  -----------------------------------------------------
23:29:41     INFO -  SUMMARY: AddressSanitizer: 96 byte(s) leaked in 3 allocation(s).
23:29:41     INFO -  TEST-INFO | Main app process: exit 0
23:29:41  WARNING -  TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at mozilla::net::GetLoadContextInfo, mozilla::net::nsHttpChannel::BeginConnect, mozilla::net::nsHttpChannel::OnProxyAvailable

Comment 7

[(no longer active)]

FWIW comment 6 only backed out this one bug.

Comment 8

[(no longer active)]

Backed out the tests in https://hg.mozilla.org/integration/mozilla-inbound/rev/7a7febd0dcbe.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/392ada5a94bb
https://hg.mozilla.org/integration/mozilla-inbound/rev/7916e6a2ad85

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/392ada5a94bb
https://hg.mozilla.org/mozilla-central/rev/7916e6a2ad85