I have been looking at this again.

Firstly, it's apparently now supposed to be a SequenceChecker, not a ThreadChecker.

Secondly, it appears that scoped_refptr now has a SequenceChecker embedded in it, which automatically catches non-thread-safe use of the reference count across threads. I'm not sure why this isn't catching cases where gfx::Image is used on two thread at the same time.

I got it.

> Secondly, it appears that scoped_refptr now has a SequenceChecker embedded in it, which automatically catches non-thread-safe use
> of the reference count across threads. I'm not sure why this isn't catching cases where gfx::Image is used on two thread at the
> same time.

So base::RefCounted only catches non-thread-safe AddRef / Release (i.e., when scoped_refptrs are copied or destroyed). The Image code I'm looking at in web_app::CreatePlatformShortcuts does not ever copy an Image so the reference count doesn't change, and the automatic sequence checking in RefCounted doesn't trigger.

After much thought and experimentation, I realised that we needed a SequenceChecker in gfx::Image to *automatically detach itself* when the refcount becomes 1 (otherwise you need explicit manual handoff whenever you take a ref-count-1 image and std::move it to another thread, which is always safe). Then I realised that base::RefCounted does precisely this, so we can leverage RefCounted's internal SequenceChecker to get this for free. Then we won't need any manual handoff code.

I filed  Issue 746792  for adding a way for subclasses to access RefCounted sequence checking capabilities. Then we can use it in the ImageStorage methods.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ba1643624e7d103cdc9f9dc86d94210e5387435a

commit ba1643624e7d103cdc9f9dc86d94210e5387435a
Author: Matt Giuca <mgiuca@chromium.org>
Date: Mon Jul 24 07:56:10 2017

Expose RefCounted sequence checker to subclass and use in gfx::Image.

This adds a Debug-only method base::RefCounted::IsOnValidSequence
(protected) for subclasses to DCHECK whenever they want, to ensure they
are being accessed in a thread-safe manner. This has the advantage over
creating their own SequenceChecker of avoiding false positives when the
object has been safely handed off to another thread (by std::moving a
single reference object).

gfx::Image's internal storage class now DCHECKs this on all method
calls, to catch illegal use of images shared by multiple threads (e.g.,
 https://crbug.com/596348 ).

Bug:  600229 ,  746792 
Change-Id: I1de5e59d403b3b5abe1abd5ebc3a9d09db23949c
Reviewed-on: https://chromium-review.googlesource.com/578750
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488929}
[modify] https://crrev.com/ba1643624e7d103cdc9f9dc86d94210e5387435a/base/memory/ref_counted.h
[modify] https://crrev.com/ba1643624e7d103cdc9f9dc86d94210e5387435a/chrome/browser/web_applications/update_shortcut_worker_win.cc
[modify] https://crrev.com/ba1643624e7d103cdc9f9dc86d94210e5387435a/ui/gfx/image/image.cc
[modify] https://crrev.com/ba1643624e7d103cdc9f9dc86d94210e5387435a/ui/gfx/image/image.h
[modify] https://crrev.com/ba1643624e7d103cdc9f9dc86d94210e5387435a/ui/gfx/image/image_family.cc
[modify] https://crrev.com/ba1643624e7d103cdc9f9dc86d94210e5387435a/ui/gfx/image/image_family.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/79a3044c7b7ac74a57c0e8013c776e9a1c334cd7

commit 79a3044c7b7ac74a57c0e8013c776e9a1c334cd7
Author: Matt Giuca <mgiuca@chromium.org>
Date: Wed Aug 16 02:53:37 2017

gfx::Image: Removed ability to disable thread checking.

Removes gfx::Image::DisableThreadChecking and corresponding method in
gfx::ImageFamily. There are no more known cases where thread checking
fails (after r493672 fixed a thread-safety issue in UpdateShortcutWorker
and removed the last call to the DisableThreadChecking method).

Bug:  600229 
Change-Id: I81b3365a7c0380997bc16f4d0d93e3393d86cfb0
Reviewed-on: https://chromium-review.googlesource.com/611863
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494678}
[modify] https://crrev.com/79a3044c7b7ac74a57c0e8013c776e9a1c334cd7/ui/gfx/image/image.cc
[modify] https://crrev.com/79a3044c7b7ac74a57c0e8013c776e9a1c334cd7/ui/gfx/image/image.h
[modify] https://crrev.com/79a3044c7b7ac74a57c0e8013c776e9a1c334cd7/ui/gfx/image/image_family.cc
[modify] https://crrev.com/79a3044c7b7ac74a57c0e8013c776e9a1c334cd7/ui/gfx/image/image_family.h