Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(203)

Issue 2907223002: Deprecate NonThreadSafe in components/drive in favor of SequenceChecker. (Closed)

Created:
3 years, 6 months ago by gab
Modified:
3 years, 6 months ago
Reviewers:
hashimoto
CC:
chromium-reviews, yamaguchi+watch_chromium.org, oka+watch_chromium.org, fukino+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Deprecate NonThreadSafe in components/drive in favor of SequenceChecker. Note to crash team: This CL is a refactor and has no intended behavior change. This change was scripted by https://crbug.com/676387#c8. Note-worthy for the reviewer: * SequenceChecker enforces thread-safety but not thread-affinity! If the classes that were updated are thread-affine (use thread local storage or a third-party API that does) they should be migrated to ThreadChecker instead. * ~NonThreadSafe() used to implcitly check in its destructor ~Sequence/ThreadChecker() doesn't by design. To keep this CL a no-op, an explicit check was added to the destructor of migrated classes. * NonThreadSafe used to provide access to subclasses, as such the |sequence_checker_| member was made protected rather than private where necessary. BUG=676387 This CL was uploaded by git cl split. TBR=hashimoto@chromium.org Review-Url: https://codereview.chromium.org/2907223002 Cr-Commit-Position: refs/heads/master@{#476292} Committed: https://chromium.googlesource.com/chromium/src/+/68602138f8e1f6be21fcd2c2e0e84bedded99901

Patch Set 1 #

Patch Set 2 : rebase off dependency #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -5 lines) Patch
M components/drive/service/drive_api_service.h View 1 3 chunks +4 lines, -2 lines 0 comments Download
M components/drive/service/drive_api_service.cc View 4 chunks +4 lines, -3 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 20 (13 generated)
gab
Hi, this is an automated change made by https://crbug.com/676387#c8 and sharded across directories for OWNERS. ...
3 years, 6 months ago (2017-05-29 19:12:47 UTC) #2
hashimoto
Using SequenceChecker looks good to me, but it seems try bots are failing because of ...
3 years, 6 months ago (2017-05-30 04:42:03 UTC) #7
gab
On 2017/05/30 04:42:03, hashimoto wrote: > Using SequenceChecker looks good to me, but it seems ...
3 years, 6 months ago (2017-05-30 14:29:41 UTC) #8
gab
On 2017/05/30 14:29:41, gab wrote: > On 2017/05/30 04:42:03, hashimoto wrote: > > Using SequenceChecker ...
3 years, 6 months ago (2017-06-01 14:02:49 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2907223002/20001
3 years, 6 months ago (2017-06-01 14:03:21 UTC) #16
commit-bot: I haz the power
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/68602138f8e1f6be21fcd2c2e0e84bedded99901
3 years, 6 months ago (2017-06-01 15:04:19 UTC) #19
hashimoto
3 years, 6 months ago (2017-06-02 03:52:23 UTC) #20
Message was sent while issue was closed.
lgtm
Sorry for being late to respond.

Powered by Google App Engine
This is Rietveld 408576698