If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Provide a download-start event to Gaia

NEW
Assigned to

Status

Firefox OS
General
4 years ago
4 years ago

People

(Reporter: fabrice, Assigned: fabrice)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [systemsfe])

Attachments

(2 attachments, 1 obsolete attachment)

29.06 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: feedback+
Details | Diff | Splinter Review
8.13 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: feedback+
Details | Diff | Splinter Review
(Assignee)

Description

4 years ago
This mozChromeEvent will be triggered in the following situations:
- Downloading content from an unsupported mime type.
- When the "download" attribute exists on a <a> tag.
- When the content-disposition header is set.

The event sends the url of the resource to download, along with the manifest URL of the application that started the download, and a boolean that let gaia know if this comes from a browser frame.

Gaia must then use the manifest URL and the browser flag as parameters to a XHR constructor to make sure that we use the appropriate data jar for this request.
(Assignee)

Comment 1

4 years ago
Created attachment 809522 [details] [diff] [review]
wip

Ben, as we discussed on irc, please check how this will fail in the worker xhr and advise a solution (I just need to get the appId from the manifest url using the xpcom AppsService).
Attachment #809522 - Flags: feedback?(bent.mozilla)

Updated

4 years ago
Whiteboard: [systemsfe]
Comment on attachment 809522 [details] [diff] [review]
wip

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

::: dom/workers/XMLHttpRequest.cpp
@@ +1489,5 @@
> +    // Get the appId from the manifestURL, if any.
> +    uint32_t appId = nsIScriptSecurityManager::NO_APP_ID;
> +    if (!aParams.mMozManifestURL.IsEmpty()) {
> +      nsCOMPtr<nsIAppsService>
> +        appsService = do_GetService(APPS_SERVICE_CONTRACTID);

This won't work properly since this service is implemented in JS. You'll need to proxy to the main thread for this information.

I think basically you can just store the manifestURL that is passed here and then pass it along to the proxy. Then use the appservice once you've bounced to the main thread.
Attachment #809522 - Flags: feedback?(bent.mozilla) → feedback-
(Assignee)

Comment 3

4 years ago
Created attachment 816894 [details] [diff] [review]
wip 2
Attachment #809522 - Attachment is obsolete: true
Attachment #816894 - Flags: feedback?(bent.mozilla)
(Assignee)

Comment 4

4 years ago
Created attachment 816896 [details] [diff] [review]
tests

First try at writing tests. They don't pass and I'm not sure where the issue is yet. We set a load context on the xhr channel and I can see the GetAppId() being called and returning the expected ID, but we still don't get the previously set cookie back.
Attachment #816896 - Flags: feedback?(bent.mozilla)
Comment on attachment 816894 [details] [diff] [review]
wip 2

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

This looks right from the worker side, I think. Maybe I found your bug:

::: dom/workers/XMLHttpRequest.cpp
@@ +137,3 @@
>    : mWorkerPrivate(nullptr), mXMLHttpRequestPrivate(aXHRPrivate),
>      mMozAnon(aMozAnon), mMozSystem(aMozSystem),
> +    mMozManifestURL(EmptyString()),

You probably want to use the manifest url that you passed in, right?

@@ +1444,5 @@
>    mWorkerPrivate(aWorkerPrivate),
>    mResponseType(XMLHttpRequestResponseType::Text), mTimeout(0),
>    mJSObjectRooted(false), mBackgroundRequest(false),
> +  mWithCredentials(false), mCanceled(false), mMozAnon(false), mMozSystem(false),
> +  mMozManifestURL(EmptyString()), mMozIsInBrowserElement(false)

No need to initialize mMozManifestURL, it will be empty automatically.
Attachment #816894 - Flags: feedback?(bent.mozilla) → feedback+
Comment on attachment 816896 [details] [diff] [review]
tests

This test looks ok to me. You'll want to split the actual test code into a js file so that you can run it in a worker, I guess.
Attachment #816896 - Flags: feedback?(bent.mozilla) → feedback+
You need to log in before you can comment on or make changes to this bug.