949666 - [e10s] Refactor file picker code to work better across processes

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I'm filing this bug to cover a few things:

1. The file picker dialog should return file handles, not just paths. That's the most reasonable way to get it working with sandboxing.

2. The code to determine the last directory where you used the file picker runs in the content process, but it makes much more sense for it to run in the parent.

3. The current proxied implementation of the file picker uses ::Show, which is model. We should switch to an API that uses ::Open.

Comment 1

[Tom S [:evilpie]]

I think we are done here.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Well, we still have problem #2. We could deal with that in some other way though. Maybe we just need to make the content pref service work in the child. Or we could move that code to the parent.

Comment 3

[Tom S [:evilpie]]

I think most stuff here is fixed, but problem #2 is still quite annoying, especially when using bugzilla. Renominating.

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Blake implemented the content pref service a while ago, so we just need to remove the conditionals around that code. Want to take this Tom?

Comment 5

[Tom S [:evilpie]]

Attachment: Pass displayDirectory to parent

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8519443 [details] [diff] [review]
Pass displayDirectory to parent

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

Looks great. Thanks!

::: dom/html/HTMLInputElement.cpp
@@ +708,2 @@
>        mInput->OwnerDoc(), lastUsedDir);
> +    printf("result: %u\n", uint32_t(rv));

Looks like you left this in by accident.

Comment 7

[Tom S [:evilpie]]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=e8c639228134
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a2c37d55e5e

Comment 8

[Carsten Book [:Tomcat]]

sorry had to back this out for test failures like https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3721157&repo=mozilla-inbound (seems this was also in the try run but not sure how this was caused by this push)

Comment 9

[Carsten Book [:Tomcat]]

also seems https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3721153&repo=mozilla-inbound is related

Comment 10

[Tom S [:evilpie]]

I guess we should put back the content process check, but just for b2g.

Comment 11

[Tom S [:evilpie]]

Attachment: filepicker

I put back the ContentProcess check for b2g. Looks like this fixes the issue on try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ad1d0c7b82bb

Comment 12

[Tom S [:evilpie]]

I made an other try push. It looks really clean, and not a single b2g mochitest failure https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=b4cbcf12b411
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9e1814d648d

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c9e1814d648d