linux_aura: run IconLoader::ReadIcon on the UI thread.

linux_aura: run IconLoader::ReadIcon on the UI thread.

The implementation goes through GTK's native icon routines which are not
thread-safe.

BUG=350545
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=256391

[davidben, 2014-03-10 16:41:09]

erg: Main review
thakis: OWNERS

This does mean we're blocking the UI thread on file IO, but the alternative is
to implement icon theme resolution ourselves (which we currently do for the GTK
port and get wrong).

[Elliot Glaysher, 2014-03-10 17:22:21]

In general, it's regrettable that it has to be done this way, but GTK was never
designed with thread safety in mind, so I'm not really surprised.

LGTM

[Nico, 2014-03-11 16:26:18]

https://codereview.chromium.org/192883004/diff/1/chrome/browser/icon_loader_l...
File chrome/browser/icon_loader_linux.cc (right):

https://codereview.chromium.org/192883004/diff/1/chrome/browser/icon_loader_l...
chrome/browser/icon_loader_linux.cc:34: return content::BrowserThread::FILE;
Isn't icon_loader_linux gtk too? Why would linux/aura need this but linux/gtk
wouldn't?

[davidben, 2014-03-11 16:37:12]

https://codereview.chromium.org/192883004/diff/1/chrome/browser/icon_loader_l...
File chrome/browser/icon_loader_linux.cc (right):

https://codereview.chromium.org/192883004/diff/1/chrome/browser/icon_loader_l...
chrome/browser/icon_loader_linux.cc:34: return content::BrowserThread::FILE;
On 2014/03/11 16:26:19, Nico wrote:
> Isn't icon_loader_linux gtk too? Why would linux/aura need this but linux/gtk
> wouldn't?

It's a little bizarre since linux/aura is more native than linux/gtk in this
regard.

The GTK port uses our own implementation of this, but it didn't actually work
(http://crbug.com/102211). I fixed it for Aura by having it just use the GTK
native implementation (which was too crashy originally or something?) and didn't
bother duplicating it for the GTK port. They aren't really arranged to easily be
able to share GTK code. That both fixed the problem and lets us remove our
reimplemented icon theme logic when the GTK port goes away...

...except that it introduced this threading problem.

[Nico, 2014-03-11 16:47:14]

lgtm stamp

To me it sounds that blocking the UI thread might be worse than that other bug,
I don't understand all the background here. If Elliot is happy, I guess it's
fine.

[davidben, 2014-03-11 18:07:52]

The CQ bit was checked by davidben@chromium.org

[commit-bot: I haz the power, 2014-03-11 18:16:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davidben@chromium.org/192883004/1

[commit-bot: I haz the power, 2014-03-12 01:48:12]

Change committed as 256391