Actually, maybe unifying all methods with callbacks to have the same behavior isn't worth the effort. But documenting what threads callbacks get called on (and making sure the any particular method always calls its callback on a predictable thread) definitely seems worth it.

Documented at least the current behavior (after fixing the two methods that sometimes call their callback on the IO thread and sometimes on the UI thread) in https://codereview.chromium.org/1757023003

I still think getting some more consistency would be nice, but at least with comments people don't have to dig through the implementation to figure out on which thread a method can be called and on which thread they should expect their callback to be called...

Yes, this has come up previously: https://codereview.chromium.org/893793005 and https://codereview.chromium.org/761923004/ and https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/uWGNk-hVlGc/M92wu3izhGEJ

I think SWContextWrapper was originally intended to be threadsafe but agree it's gotten inconsistent and confusing the threads each function is entered on and calls back on. I'd support requiring everything on the IO thread as you propose above.

SWContextWrapper was originally for two things

- implement the content public ServiceWorkerContext api, provide the entry points for those methods (some of which were intended to be only UI thread invocable).

- bridge the gap between the UI thread StoragePartition object and its indirect ownership of the IO thread SwContextCore object.

Over time, methods got added "for convenience". Might be cleaner to remove the convenience methods and provide a public context_core() getter that's only callable from the IO thread.

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

commit e51e01d8b2c430f2f327692e2b98006b8dc0763d
Author: mek <mek@chromium.org>
Date: Fri Mar 04 00:05:45 2016

Document confusing threading requirements in ServiceWorkerContext(Wrapper).

This adds comments to all public methods in ServiceWorkerContext and
ServiceWorkerContextWrapper explaining on which thread the method can be
called, and on which thread its callback will be called.

Also fixes some inconsistencies where for some methods a callback could
be sometimes called on different threads.

BUG=591427

Review URL: https://codereview.chromium.org/1757023003

Cr-Commit-Position: refs/heads/master@{#379145}

[modify] https://crrev.com/e51e01d8b2c430f2f327692e2b98006b8dc0763d/content/browser/service_worker/service_worker_context_wrapper.cc
[modify] https://crrev.com/e51e01d8b2c430f2f327692e2b98006b8dc0763d/content/browser/service_worker/service_worker_context_wrapper.h
[modify] https://crrev.com/e51e01d8b2c430f2f327692e2b98006b8dc0763d/content/public/browser/service_worker_context.h

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

commit 347362dc1067fdee8a0b512bf24659ae5b8dced6
Author: droger <droger@chromium.org>
Date: Fri Dec 09 14:16:06 2016

CheckHasServiceWorkerCallback always called on the UI thread

This updates the code to match the documentation in
service_worker_context.h.

BUG=591427

Review-Url: https://codereview.chromium.org/2566623004
Cr-Commit-Position: refs/heads/master@{#437531}

[modify] https://crrev.com/347362dc1067fdee8a0b512bf24659ae5b8dced6/content/browser/service_worker/service_worker_context_wrapper.cc

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This is timely given https://chromium-review.googlesource.com/c/chromium/src/+/1344136.

Putting back on the shelf but it'd be nice to bring consistency to this API, probably by requiring functions to be called on a specific thread.