Convert gfx::internal::ImageStorage RefCountedThreadSafe

Convert gfx::internal::ImageStorage RefCountedThreadSafe

gfx::internal::ImageStorage has non-thread-safe ref count, but the ref
count is accessed from multiple threads.

It is held by multiple gfx::Image instances, gfx::Image is held by
gfx::ImageFamily, and web_app::ShortcutInfo has gfx::ImageFamily.
Where, web_app::ShortcutInfo is created on UI thread, and destroyed
on FILE thread. Implies, the ref count of gfx::internal::ImageStorage
is touched from both FILE thread and UI thread, and causes a data
race.

This CL converts gfx::internal::ImageStorage from RefCounted to
RefCountedThreadSafe to avoid the race.

BUG=688072

[tzik, 2017-02-17 12:20:30]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-17 12:20:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2017-02-17 13:03:37]

Description was changed from

==========
Convert gfx::internal::ImageStorage RefCountedThreadSafe
==========

to

==========
Convert gfx::internal::ImageStorage RefCountedThreadSafe

gfx::internal::ImageStorage has non-thread-safe ref count, but the ref
count is accessed from multiple threads.

It is held by multiple gfx::Image instances, gfx::Image is held by
gfx::ImageFamily, and web_app::ShortcutInfo has gfx::ImageFamily.
Where, web_app::ShortcutInfo is created on UI thread, and destroyed
on FILE thread. Implies, the ref count of gfx::internal::ImageStorage
is touched from both FILE thread and UI thread, and causes a data
race.

This CL converts gfx::internal::ImageStorage from RefCounted to
RefCountedThreadSafe to avoid the race.
==========

[tzik, 2017-02-17 13:05:13]

Description was changed from

==========
Convert gfx::internal::ImageStorage RefCountedThreadSafe

gfx::internal::ImageStorage has non-thread-safe ref count, but the ref
count is accessed from multiple threads.

It is held by multiple gfx::Image instances, gfx::Image is held by
gfx::ImageFamily, and web_app::ShortcutInfo has gfx::ImageFamily.
Where, web_app::ShortcutInfo is created on UI thread, and destroyed
on FILE thread. Implies, the ref count of gfx::internal::ImageStorage
is touched from both FILE thread and UI thread, and causes a data
race.

This CL converts gfx::internal::ImageStorage from RefCounted to
RefCountedThreadSafe to avoid the race.
==========

to

==========
Convert gfx::internal::ImageStorage RefCountedThreadSafe

gfx::internal::ImageStorage has non-thread-safe ref count, but the ref
count is accessed from multiple threads.

It is held by multiple gfx::Image instances, gfx::Image is held by
gfx::ImageFamily, and web_app::ShortcutInfo has gfx::ImageFamily.
Where, web_app::ShortcutInfo is created on UI thread, and destroyed
on FILE thread. Implies, the ref count of gfx::internal::ImageStorage
is touched from both FILE thread and UI thread, and causes a data
race.

This CL converts gfx::internal::ImageStorage from RefCounted to
RefCountedThreadSafe to avoid the race.

BUG=688072
==========

[tzik, 2017-02-17 13:05:13]

tzik@chromium.org changed reviewers:
+ rsesek@chromium.org

[tzik, 2017-02-17 13:05:36]

PTAL

[commit-bot: I haz the power, 2017-02-17 13:52:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-17 13:52:53]

Dry run: This issue passed the CQ dry run.

[Robert Sesek, 2017-02-17 20:44:56]

I don't think it's this simple, unfortunately. The basic issue is that
gfx::Image is not threadsafe and shouldn't be treated as such. Fixing the
refcounting issue does not fix the actual thread safety problem. There's quite a
bit of history here:

https://groups.google.com/a/chromium.org/d/msg/chromium-dev/8LqVoXQ_2bo/cW0tH...
https://bugs.chromium.org/p/chromium/issues/detail?id=468010

[tzik, 2017-02-21 06:51:11]

On 2017/02/17 20:44:56, Robert Sesek wrote:
> I don't think it's this simple, unfortunately. The basic issue is that
> gfx::Image is not threadsafe and shouldn't be treated as such. Fixing the
> refcounting issue does not fix the actual thread safety problem. There's quite
a
> bit of history here:
> 
>
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/8LqVoXQ_2bo/cW0tH...
> https://bugs.chromium.org/p/chromium/issues/detail?id=468010

Ah.. OK, make sense. Let me change the approach to fix the users of gfx::Image
rather than the ref count itself.