Saving a canvas to disk makes firefox hang

VERIFIED FIXED in Firefox 23

Status

()

Toolkit
Downloads API
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: pere.jobs, Assigned: jhorak)

Tracking

({hang, regression})

19 Branch
mozilla23
All
Linux
hang, regression
Points:
---

Firefox Tracking Flags

(firefox20- affected, firefox21- affected, firefox22- affected, firefox23- verified, firefox-esr17 unaffected)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
Created attachment 735674 [details]
File with a canvas

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:20.0) Gecko/20100101 Firefox/20.0
Build ID: 20130329030832

Steps to reproduce:

1. Loaded http://ophir.lojkine.free.fr/interference/interference.html
2. Clicked "Sauvagarder la pemière imge", or right-click on the second and chose "Save to disk"


Actual results:

1. Firefox asks where I want to save the image.
2. I choose a location
3. Firefox saves the image, I can open it with another application
4. Firefox freezes


Expected results:

Firefox shouldn't have freezed
Attachment #735674 - Attachment mime type: text/plain → text/html
FWIW, WFM using Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0 ID:20130409194949 CSet: 5bb6421fee94

Does toggling "browser.download.manager.scanWhenDone" in about:config make any Difference?
Is your Download Dir a common local Path?
Flags: needinfo?(pere.jobs)
(Reporter)

Comment 2

5 years ago
browser.download.manager.scanWhenDone was set to true, I set it to false, but the issue didn't disappear.

One interesting thing is that while firefox is freezed, it doesn't use 100% CPU.
Flags: needinfo?(pere.jobs)
(Reporter)

Comment 3

5 years ago
Created attachment 736925 [details]
backtrace of the firefox process during the freeze

I attached gdb to firefox during the freeze, and ran bt. I don't know if it can help, but here is the result.
I don't use a debug build, so most function names are missing.
I can also reproduce this issue, with the latest Nightly, build ID: 20130415030812, on an Ubuntu 12.10 32-bit machine, with both "browser.download.manager.scanWhenDone" set to true (default value) and false.
Status: UNCONFIRMED → NEW
Component: Untriaged → File Handling
Ever confirmed: true
Reproducible on the latest Aurora - build ID: 20130415004014
Reproducible on the latest Beta - build ID: 20130408165307

Note:

I will investigate if this issue is a regression. With Firefox 4, "Sauvagarder la pemière imge" is not displayed as a link, so I can't save the image.
This issue is also reproducible with Firefox 20.0.1 and 19.0.2, but it doesn't reproduce on Firefox 18.0.1, so the regression is recent, between version 18 and 19.

Comment 7

5 years ago
Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/aa5e3b445810
Mozilla/5.0 (X11; Linux i686; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121009030547
Bad:
http://hg.mozilla.org/mozilla-central/rev/1301a72b1c39
Mozilla/5.0 (X11; Linux i686; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121012030610
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=aa5e3b445810&tochange=1301a72b1c39

Inlocal build,
Last Good: 4210c1f677e5
First Bad: 0008531f1429

Triggered by: 
	0008531f1429	Jan Horak — Bug 797349 - Add support for metadata::download-uri for later display in file manager by GIO. r=Neil


I guess landing of Bug 797349 triggers https://bugzilla.gnome.org/show_bug.cgi?id=637095 .

Workaround: In a terminal run the following 2 commands:
  rm -rf ~/.local/share/gvfs-metadata
  pkill gvfsd-metadata

Then, the Firefox will be back to normal.
Blocks: 797349
Component: File Handling → Download Manager
Keywords: hang, regression
Product: Firefox → Toolkit
Version: 20 Branch → 19 Branch

Updated

5 years ago
status-firefox-esr17: --- → unaffected
tracking-firefox20: --- → ?
tracking-firefox21: --- → ?
tracking-firefox22: --- → ?
tracking-firefox23: --- → ?
We've already shipped this several times, without more data on significant user impact there's no need to track this but a low risk fix would be considered for uplift.
status-firefox20: --- → affected
status-firefox21: --- → affected
status-firefox22: --- → affected
status-firefox23: --- → affected
tracking-firefox20: ? → -
tracking-firefox21: ? → -
tracking-firefox22: ? → -
tracking-firefox23: ? → -
(Assignee)

Comment 9

5 years ago
Created attachment 740203 [details]
backtrace with symbols

Yes, looks like g_daemon_vfs_local_file_set_attributes is blocking ui when calling gvfs_metadata_call_set_sync. Looks like bug in GIO(?).
Attachment #736925 - Attachment is obsolete: true
(Assignee)

Comment 10

5 years ago
Looks like mSource->GetSpec() doesn't return valid URI but: image/jpeg;base64,/9j/4AAQSkZJRgABAQAAA
QABAAD/2wBDAAMCAgICAgM...

This might confuse GIO. So we can check whenever uri is valid (any hint for appropriate function?) and/or use async version of g_file_set_attribute, ie. g_file_set_attributes_async which will run blocking operation in new thread.

Affected code: http://dxr.mozilla.org/mozilla-central/toolkit/components/downloads/nsDownloadManager.cpp#l2672
(Assignee)

Comment 11

5 years ago
Created attachment 740843 [details] [diff] [review]
Use async version of set metadata v1

Attached patch use async version of g_file_set_attribute. This avoids UI lock when DBUS call reaches timeout.
Attachment #740843 - Flags: review?(neil)
Comment on attachment 740843 [details] [diff] [review]
Use async version of set metadata v1

[Sorry for the delay.]

>+    nsCString msg("Set file metadata failed: ");
>+    msg.Append(err->message);
>+    NS_WARNING(msg.get());
Just skimmed the patch, but this probably needs to be in an #ifdef debug patch to avoid a useless variable. (You could call NS_DebugBreak directly to avoid the extra variable, but that doesn't avoid the #ifdef, since NS_DebugBreak is available in release too.)
Comment on attachment 740843 [details] [diff] [review]
Use async version of set metadata v1

One other nit:

>+void gio_set_metadata_done(GObject *source_obj, GAsyncResult *res, gpointer user_data)
This should be static void.
Attachment #740843 - Flags: review?(neil) → feedback+
(Assignee)

Comment 14

5 years ago
Created attachment 745121 [details] [diff] [review]
Use async version of set metadata v2

Thanks for feedback, attaching fixed patch.
Attachment #740843 - Attachment is obsolete: true
Attachment #745121 - Flags: review?(neil)

Updated

5 years ago
Attachment #745121 - Flags: review?(neil) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ec71524c28c
Assignee: nobody → jhorak
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8ec71524c28c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-firefox23: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla23

Comment 17

5 years ago
Verified as fixed on:
Mozilla/5.0 (X11; Linux i686; rv:23.0) Gecko/20100101 Firefox/23.0 (20130703181823)
Mozilla/5.0 (X11; Linux i686; rv:25.0) Gecko/20130703 Firefox/25.0 (20130703031323)

The images are saved immediately and with no hangs, freezes, non-responsiveness.
Status: RESOLVED → VERIFIED
status-firefox23: fixed → verified
Hardware: x86_64 → All

Updated

5 years ago
QA Contact: ioana.budnar
You need to log in before you can comment on or make changes to this bug.