Remove call to IsRunningSequenceOnCurrentThread() in media_file_system_backend.cc

Remove call to IsRunningSequenceOnCurrentThread() in media_file_system_backend.cc

SequencedWorkerPool is being migrated to base/task_scheduler and there
is no equivalent to IsRunningSequenceOnCurrentThread() in
base/task_scheduler.

Developers should use SequenceChecker to verify that tasks run
sequentially. Unlike
SequencedWorkerPool::IsRunningSequenceOnCurrentThread(),
SequenceChecker works everywhere in Chrome (MessageLoop,
SequencedWorkerPool, base/task_scheduler...).

BUG=622400
Committed: https://crrev.com/cec55646e176f883295bb4be37599aa7c0cb0555
Cr-Commit-Position: refs/heads/master@{#417084}

[fdoray, 2016-09-07 13:49:27]

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

[commit-bot: I haz the power, 2016-09-07 13:49:33]

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

[commit-bot: I haz the power, 2016-09-07 14:29:33]

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

[commit-bot: I haz the power, 2016-09-07 14:29:34]

Dry run: This issue passed the CQ dry run.

[fdoray, 2016-09-07 14:37:01]

fdoray@chromium.org changed reviewers:
+ reillyg@chromium.org

[fdoray, 2016-09-07 14:37:02]

PTAL

[fdoray, 2016-09-07 14:49:43]

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

[commit-bot: I haz the power, 2016-09-07 14:50:05]

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

[fdoray, 2016-09-07 14:51:48]

Description was changed from

==========
Remove call to IsRunningSequenceOnCurrentThread() in media_file_system_backend.h

SequencedWorkerPool is being migrated to base/task_scheduler and there
is no equivalent to IsRunningSequenceOnCurrentThread() in
base/task_scheduler.

Developers should use SequenceChecker to verify that tasks run
sequentially. Unlike
SequencedWorkerPool::IsRunningSequenceOnCurrentThread(),
SequenceChecker works everywhere in Chrome (MessageLoop,
SequencedWorkerPool, base/task_scheduler...).

BUG=622400
==========

to

==========
Remove call to IsRunningSequenceOnCurrentThread() in
media_file_system_backend.cc

SequencedWorkerPool is being migrated to base/task_scheduler and there
is no equivalent to IsRunningSequenceOnCurrentThread() in
base/task_scheduler.

Developers should use SequenceChecker to verify that tasks run
sequentially. Unlike
SequencedWorkerPool::IsRunningSequenceOnCurrentThread(),
SequenceChecker works everywhere in Chrome (MessageLoop,
SequencedWorkerPool, base/task_scheduler...).

BUG=622400
==========

[commit-bot: I haz the power, 2016-09-07 15:39:08]

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

[commit-bot: I haz the power, 2016-09-07 15:39:09]

Dry run: This issue passed the CQ dry run.

[Reilly Grant (use Gerrit), 2016-09-07 18:07:05]

https://codereview.chromium.org/2318133002/diff/20001/chrome/browser/media_ga...
File chrome/browser/media_galleries/fileapi/media_file_system_backend.cc
(right):

https://codereview.chromium.org/2318133002/diff/20001/chrome/browser/media_ga...
chrome/browser/media_galleries/fileapi/media_file_system_backend.cc:161:
DCHECK(g_media_sequence_checker.Get().CalledOnValidSequence());
I don't like consolidating the DCHECK into a single place because it means the
line number reported is not as useful.

[fdoray, 2016-09-07 19:21:51]

https://codereview.chromium.org/2318133002/diff/20001/chrome/browser/media_ga...
File chrome/browser/media_galleries/fileapi/media_file_system_backend.cc
(right):

https://codereview.chromium.org/2318133002/diff/20001/chrome/browser/media_ga...
chrome/browser/media_galleries/fileapi/media_file_system_backend.cc:161:
DCHECK(g_media_sequence_checker.Get().CalledOnValidSequence());
On 2016/09/07 18:07:05, Reilly Grant wrote:
> I don't like consolidating the DCHECK into a single place because it means the
> line number reported is not as useful.

I don't want a public CurrentlyOnMediaTaskRunnerThread() method that relies on
SequenceChecker because people could be tempted to use it outside of DCHECKs.
e.g.:
  if (MediaFileSystemBackend::CurrentlyOnMediaTaskRunnerThread())
    ThisIsAlwaysCalledInANonDCHECKBuild();

Since DCHECK prints a stack trace, does this CL really makes it harder to
understand reported errors?

Potential solutions:
1. Keep this CL as-is.
2. Have a CurrentlyOnMediaSequence() method based on SequenceChecker - with a
comment that says that it shouldn't be used outside of DCHECKs since it always
returns true in non-DCHECK builds.
3. Have a GetMediaSequenceChecker() method that returns a SequenceChecker. I
believe that people are familiar with the fact that SequenceChecker is a no-op
in non-DCHECK builds and won't try to do if
(MediaFileSystemBackend::GetMediaSequenceChecker().CalledOnValidSequence()) {
... }

[Reilly Grant (use Gerrit), 2016-09-07 20:58:55]

https://codereview.chromium.org/2318133002/diff/20001/chrome/browser/media_ga...
File chrome/browser/media_galleries/fileapi/media_file_system_backend.cc
(right):

https://codereview.chromium.org/2318133002/diff/20001/chrome/browser/media_ga...
chrome/browser/media_galleries/fileapi/media_file_system_backend.cc:161:
DCHECK(g_media_sequence_checker.Get().CalledOnValidSequence());
On 2016/09/07 at 19:21:51, fdoray wrote:
> On 2016/09/07 18:07:05, Reilly Grant wrote:
> > I don't like consolidating the DCHECK into a single place because it means
the
> > line number reported is not as useful.
> 
> I don't want a public CurrentlyOnMediaTaskRunnerThread() method that relies on
SequenceChecker because people could be tempted to use it outside of DCHECKs.
e.g.:
>   if (MediaFileSystemBackend::CurrentlyOnMediaTaskRunnerThread())
>     ThisIsAlwaysCalledInANonDCHECKBuild();
> 
> Since DCHECK prints a stack trace, does this CL really makes it harder to
understand reported errors?
> 
> Potential solutions:
> 1. Keep this CL as-is.
> 2. Have a CurrentlyOnMediaSequence() method based on SequenceChecker - with a
comment that says that it shouldn't be used outside of DCHECKs since it always
returns true in non-DCHECK builds.
> 3. Have a GetMediaSequenceChecker() method that returns a SequenceChecker. I
believe that people are familiar with the fact that SequenceChecker is a no-op
in non-DCHECK builds and won't try to do if
(MediaFileSystemBackend::GetMediaSequenceChecker().CalledOnValidSequence()) {
... }

I prefer option 2. Stack traces often do not include line numbers.

[Reilly Grant (use Gerrit), 2016-09-07 21:37:37]

lgtm, option 1 is fine too.

[fdoray, 2016-09-07 22:08:06]

The CQ bit was checked by fdoray@chromium.org

[commit-bot: I haz the power, 2016-09-07 22:08:49]

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

[commit-bot: I haz the power, 2016-09-07 22:15:16]

Description was changed from

==========
Remove call to IsRunningSequenceOnCurrentThread() in
media_file_system_backend.cc

SequencedWorkerPool is being migrated to base/task_scheduler and there
is no equivalent to IsRunningSequenceOnCurrentThread() in
base/task_scheduler.

Developers should use SequenceChecker to verify that tasks run
sequentially. Unlike
SequencedWorkerPool::IsRunningSequenceOnCurrentThread(),
SequenceChecker works everywhere in Chrome (MessageLoop,
SequencedWorkerPool, base/task_scheduler...).

BUG=622400
==========

to

==========
Remove call to IsRunningSequenceOnCurrentThread() in
media_file_system_backend.cc

SequencedWorkerPool is being migrated to base/task_scheduler and there
is no equivalent to IsRunningSequenceOnCurrentThread() in
base/task_scheduler.

Developers should use SequenceChecker to verify that tasks run
sequentially. Unlike
SequencedWorkerPool::IsRunningSequenceOnCurrentThread(),
SequenceChecker works everywhere in Chrome (MessageLoop,
SequencedWorkerPool, base/task_scheduler...).

BUG=622400
==========

[commit-bot: I haz the power, 2016-09-07 22:15:17]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-07 22:17:06]

Description was changed from

==========
Remove call to IsRunningSequenceOnCurrentThread() in
media_file_system_backend.cc

SequencedWorkerPool is being migrated to base/task_scheduler and there
is no equivalent to IsRunningSequenceOnCurrentThread() in
base/task_scheduler.

Developers should use SequenceChecker to verify that tasks run
sequentially. Unlike
SequencedWorkerPool::IsRunningSequenceOnCurrentThread(),
SequenceChecker works everywhere in Chrome (MessageLoop,
SequencedWorkerPool, base/task_scheduler...).

BUG=622400
==========

to

==========
Remove call to IsRunningSequenceOnCurrentThread() in
media_file_system_backend.cc

SequencedWorkerPool is being migrated to base/task_scheduler and there
is no equivalent to IsRunningSequenceOnCurrentThread() in
base/task_scheduler.

Developers should use SequenceChecker to verify that tasks run
sequentially. Unlike
SequencedWorkerPool::IsRunningSequenceOnCurrentThread(),
SequenceChecker works everywhere in Chrome (MessageLoop,
SequencedWorkerPool, base/task_scheduler...).

BUG=622400

Committed: https://crrev.com/cec55646e176f883295bb4be37599aa7c0cb0555
Cr-Commit-Position: refs/heads/master@{#417084}
==========

[commit-bot: I haz the power, 2016-09-07 22:17:07]

Patchset 2 (id:??) landed as
https://crrev.com/cec55646e176f883295bb4be37599aa7c0cb0555
Cr-Commit-Position: refs/heads/master@{#417084}