860213 - Saving a canvas to disk makes firefox hang

Status: VERIFIED FIXED

(Toolkit :: Downloads API, defect)

Description

[Ophir LOJKINE]

Attachment: 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

Comment 1

[(mostly gone) XtC4UaLL [:xtc4uall]]

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?

Comment 2

[Ophir LOJKINE]

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.

Comment 3

[Ophir LOJKINE]

Attachment: 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.

Comment 4

[Manuela Muntean [Away]]

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.

Comment 5

[Manuela Muntean [Away]]

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.

Comment 6

[Manuela Muntean [Away]]

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

[Alice0775 White]

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.

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

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.

Comment 9

[Jan Horak [:jhorak]]

Attachment: 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(?).

Comment 10

[Jan Horak [:jhorak]]

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

Comment 11

[Jan Horak [:jhorak]]

Attachment: 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.

Comment 12

[[e-mail]]

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 13

[[e-mail]]

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.

Comment 14

[Jan Horak [:jhorak]]

Attachment: Use async version of set metadata v2

Thanks for feedback, attaching fixed patch.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8ec71524c28c

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8ec71524c28c

Comment 17

[Ioana (away)]

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.