Open Bug 1573469 Opened 8 months ago Updated 5 months ago

The downloaded file when opened from downloads page launched in a desktop app should be opened in a separate process.

Categories

(Firefox :: File Handling, defect, P3)

Desktop
Windows 10
defect

Tracking

()

People

(Reporter: raghvendra.kumar, Unassigned)

References

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/75.0.3770.142 Safari/537.36

Steps to reproduce:

What did you do? (steps to reproduce)

Make sure MS office apps is not already running(not previously launched)

Open Mozilla Firefox downloads page and open any previously downloaded document associated with MS office(PowerPoint etc)

PowerPoint process gets launched as a child process of Firefox (the process ownership is retained by Firefox can be viewed by process explorer)

Actual results:

What happened? (actual results)

The process ownership is retained by Firefox can be viewed by process explorer

Expected results:

What should have happened? (expected results)

Launched app should be opened in a separate process.

This is a deviation from the Fire Fox process model

Hi Raghvendra,

Thanks for taking the time of opening this bug. I tried to reproduce this issue on Windows 10 with the latest version of Nightly 70.0a1 (2019-08-20) with a new profile but the powerpoint file is opened by the program Powerpoint. Could you send me a video featuring this issue? I use LICEcap, if it helps. What version of firefox are you using?

On the other hand, does this issue occur with a fresh profile? you can find the steps here: https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles?redirectlocale=en-US&redirectslug=Managing-profiles#w_starting-the-profile-manager

Thanks in advance, Flor.

Flags: needinfo?(raghvendra.kumar)
Attached image issue.gif

Hi Florencia,

Thanks for the reply.

Powerpoint file is opened by the program Powerpoint which is fine.

Case 1 : Powerpoint program was NOT already running on the system

           In this case opening the powerpoint file from firefox launches it as a child process under the firefox main process.

Case 2: Powerpoint program already running on the system

         In this case opening the powerpoint file from firefox launches it as a child process under the Powerpoint program main process.

The powerpoint file should not be launched under firefox main process in case 1.

attaching the gif showing the launch and process explorer

Attached image PPT.gif

Hi,

Thanks for the video, I now understand your issue better. As you can see in my gif, I didn't have PPT running before, I downloaded a video and opened it and it opened on a different process.

Could you please try with a fresh profile? you can find the steps here: https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles?redirectlocale=en-US&redirectslug=Managing-profiles#w_starting-the-profile-manager

Thanks in advance, Florencia.

PS: If I did something wrong, please point it out!

Flags: needinfo?(raghvendra.kumar)

Tried with a new Mozilla profile and I can still see the process explorer showing the process under Mozilla parent process.

Are you sure the Powerpoint was not running prior to opening it from Mozilla ?

Hi Raghav,

I'm sure it wasn't running. You can even see on my video that there´s no PPT process running prior to the download from Mozilla. Nonetheless, I've chosen a component for this bug so that the developers may look at it. We'll await their answer. If you consider that there's another component that's more proper for this case you may change it.

Best regards, Flor.

Component: Untriaged → Downloads API
Flags: needinfo?(raghvendra.kumar)
OS: Unspecified → Windows 10
Product: Firefox → Toolkit
Hardware: Unspecified → Desktop

(In reply to Florencia Marina Di Ciocco from comment #4)

Thanks for the video, I now understand your issue better. As you can see in my gif, I didn't have PPT running before, I downloaded a video and opened it and it opened on a different process.

The difference is the reporter is using process explorer ( https://docs.microsoft.com/en-us/sysinternals/downloads/process-explorer ) rather than the builtin task manager that you used.

I don't have powerpoint, but I can reproduce the new process showing up as a child of the firefox parent process when using notepad and a txt file. Tested with 70.0b3 on win10.

Aaron, I know you've worked on issues related to how we start external processes recently. Is this expected (and should we close invalid/wontfix) ?

Note also that I see Chrome doing exactly the same thing (for .txt files, anyway).

Component: Downloads API → File Handling
Flags: needinfo?(aklotz)
Product: Toolkit → Firefox
Blocks: 1570601

(In reply to :Gijs (he/him) from comment #7)

Aaron, I know you've worked on issues related to how we start external processes recently. Is this expected (and should we close invalid/wontfix) ?

I would say that it depends on whether there are any adverse affects being encountered as a consequence of this. From what I have read in this bug, that is unclear.

  1. If quitting Firefox also quits the external process, then we have a problem.
  2. If the external process is behaving unexpectedly (perhaps as a result of inheriting some of Firefox's process attributes), then we have a problem.

OTOH, if the issue is just the placement of the child external process in the process tree, then it is not really much of an issue.

I am a little bit concerned about item (2) in the above list, even if it is not currently reported as an issue, as we could possibly end up encountering the same problem opening a downloaded file as we experienced when opening URLs with custom schemes in bug 1567614. It certainly wouldn't hurt to apply a similar patch to this use case.

Flags: needinfo?(aklotz)

I think it is not just the placement of the child external process in the process tree but the external process which we are launching using ShellExecuteExW from a our native Firefox extension does get killed after quitting Firefox so as you pointed out it appears that unexpected behaviour is result of inheriting some of Firefox's process attributes and it works fine in chrome though.

(In reply to Raghav from comment #9)

the external process which we are launching using ShellExecuteExW from a our native Firefox extension does get killed after quitting Firefox

Uh, can you elaborate a bit on this? There's no mention of an extension in comment #0 in the steps to reproduce, and we don't normally load C++ inside the Firefox process from extensions, and don't directly expose ShellExecuteExW.

Flags: needinfo?(raghvendra.kumar)

Needinfo'ing the right account...

Flags: needinfo?(raghvendra.kumar) → needinfo?(write.to.raghav)

(In reply to Aaron Klotz [:aklotz] from comment #8)

  1. If quitting Firefox also quits the external process, then we have a problem.

FWIW, with the steps from comment #0 and using notepad, notepad does not quit when quitting Firefox.

I'm not sure if/how the steps in comment #9 are different. I don't have powerpoint to check; I installed "powerpoint mobile" from the windows store, and that does not appear "under" Firefox in the process explorer process tree when launched, presumably because it is one of the newfangled Windows 10 / WinRT apps.

There's no mention of an extension in comment #0 in the steps to reproduce

It's actually mentioned in the another bug linked to it.

we don't normally load C++ inside the Firefox process from extensions, and don't directly expose ShellExecuteExW

We have a native window app which is launched via Firefox extension and we read the following from Mozilla's documentation:

Mozilla kills the subprocesses if they do not break away
https://wiki.mozilla.org/Security/Sandbox/Process_model#Privileged_Content_Process
On Windows, the browser puts the native application's process into a Job object, and kills the job. If the native application launches any additional processes and wants them to remain open after the native application itself is killed, then the native application must launch the additional process with the CREATE_BREAKAWAY_FROM_JOB flag.

We are using ShellExecuteExW instead in the native app and suspect that it is inheriting some of Firefox's process attributes.

Flags: needinfo?(write.to.raghav)

(In reply to Raghav from comment #13)

There's no mention of an extension in comment #0 in the steps to reproduce

It's actually mentioned in the another bug linked to it.

Alright, so let's discuss it in that bug. I've moved that to webextensions seeing as that's what you're doing. The path from opening a file from the downloads window and the one that nativeMessaging uses to create processes is very different so they need a different discussion and different fix.

No longer blocks: 1570601
See Also: → 1570601

(In reply to :Gijs (he/him) from comment #7)

The difference is the reporter is using process explorer ( https://docs.microsoft.com/en-us/sysinternals/downloads/process-explorer ) rather than the builtin task manager that you used.

Thanks, that made the difference! I was able to reproduce the bug on the following versions:

release 69.0 (64-bit), nightly 71.0a1 (2019-09-06) (64-bit) and Beta 70.0b3 (64-bit).

Best regards, Flor.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: 68 Branch → Trunk

To fix this we'd need to fix:

https://searchfox.org/mozilla-central/rev/4218cb868d8deed13e902718ba2595d85e12b86b/uriloader/exthandler/win/nsMIMEInfoWin.cpp#101
https://searchfox.org/mozilla-central/rev/4218cb868d8deed13e902718ba2595d85e12b86b/xpcom/io/nsLocalFileWin.cpp#3065

and potentially

https://searchfox.org/mozilla-central/rev/4218cb868d8deed13e902718ba2595d85e12b86b/xpcom/threads/nsProcessCommon.cpp#468

(called via nsMIMEInfoWin delegating to https://searchfox.org/mozilla-central/rev/4218cb868d8deed13e902718ba2595d85e12b86b/uriloader/exthandler/nsMIMEInfoImpl.cpp#336 which uses nsProcess to launch a thing)

This has been working this way since ~forever, so I don't think it makes sense to track as a regression or on a per-release basis. Nobody on the Firefox frontend side has cycles to address this, and I think it's unclear that this issue as filed (as opposed to the webext issue) has any user-visible impact, so I'm putting this in the backlog - Aaron, if you think from a sandboxing "better safe than sorry" perspective, we should prioritize this higher, please feel free to adjust the bug accordingly.

Flags: needinfo?(aklotz)
Priority: -- → P3

P3 is fine for now.

Flags: needinfo?(aklotz)

This will be a dup of Bug 1588975.

(In reply to :Gijs (he/him) from comment #16)

To fix this we'd need to fix:

https://searchfox.org/mozilla-central/rev/4218cb868d8deed13e902718ba2595d85e12b86b/uriloader/exthandler/win/nsMIMEInfoWin.cpp#101
https://searchfox.org/mozilla-central/rev/4218cb868d8deed13e902718ba2595d85e12b86b/xpcom/io/nsLocalFileWin.cpp#3065

and potentially

https://searchfox.org/mozilla-central/rev/4218cb868d8deed13e902718ba2595d85e12b86b/xpcom/threads/nsProcessCommon.cpp#468

My patch changes nsLocalFile::Launch in nsLocalFileWin.cpp. I leave ShellExecuteExW in nsMIMEInfoWin::LaunchWithFile because it's only to run rundll32.exe for a given DLL.

Changing nsProcess::RunProcess in nsProcessCommon.cpp looks risky because nsProcess launches a process synchronously and tracks a launched process handle, that is not possible with IShellDispatch2.ShellExecute. Alternatively, I replace LaunchWithIProcess in nsMIMEInfoWin::LaunchWithFile.

You need to log in before you can comment on or make changes to this bug.