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

Issue 1369933004: Revert of Implement a simple ThreadChecker for Mojo and add to |internal::SharedData| and |internal::Router|. (Closed)

Created:
5 years, 2 months ago by Elliot Glaysher
Modified:
5 years, 2 months ago
CC:
chromium-reviews, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review), ben+mojo_chromium.org, chrome-apps-syd-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Implement a simple ThreadChecker for Mojo and add to |internal::SharedData| and |internal::Router|. (patchset #4 id:60001 of https://codereview.chromium.org/1380043002/ ) Reason for revert: Mandoline now crashes immediately on startup. Reverting this change locally fixes it. (mandoline is probably doing something unsafe, but it worked previously to this patch.) mandoline: ../../third_party/mojo/src/mojo/public/cpp/bindings/lib/shared_data.h:64: void mojo::internal::SharedData<mojo::internal::SharedPtr<mojo::Callback<void ()>::Runnable>::Impl>::Holder::Release() [T = mojo::internal::SharedPtr<mojo::Callback<void ()>::Runnable>::Impl]: Assertion `thread_checker_.CalledOnValidThread()' failed. Original issue's description: > Implement a simple ThreadChecker for Mojo and add to |internal::SharedData| and |internal::Router|. > > |internal::SharedData| is used in several places, in particular, > callbacks, which are very easy to misuse by copying around between > threads. |internal::Router| is used commonly between the various > bindings classes, making it an ideal place to catch misuse of bindings. > > BUG=537911 > > Committed: https://crrev.com/7cff2feae7fcccc416827013df112396c84b5017 > Cr-Commit-Position: refs/heads/master@{#352508} TBR=rockot@chromium.org,yzshen@chromium.org,amistry@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=537911 Committed: https://crrev.com/f5b0bb898c06c6c40067d88390ccf79b675cf1d6 Cr-Commit-Position: refs/heads/master@{#352648}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -136 lines) Patch
M third_party/mojo/mojo_edk_tests.gyp View 2 chunks +0 lines, -2 lines 0 comments Download
M third_party/mojo/mojo_public.gyp View 1 chunk +0 lines, -3 lines 0 comments Download
M third_party/mojo/src/mojo/public/cpp/bindings/BUILD.gn View 1 chunk +0 lines, -3 lines 0 comments Download
M third_party/mojo/src/mojo/public/cpp/bindings/lib/router.h View 5 chunks +4 lines, -23 lines 0 comments Download
M third_party/mojo/src/mojo/public/cpp/bindings/lib/router.cc View 2 chunks +0 lines, -4 lines 0 comments Download
M third_party/mojo/src/mojo/public/cpp/bindings/lib/shared_data.h View 3 chunks +1 line, -9 lines 0 comments Download
D third_party/mojo/src/mojo/public/cpp/bindings/lib/thread_checker.h View 1 chunk +0 lines, -37 lines 0 comments Download
D third_party/mojo/src/mojo/public/cpp/bindings/lib/thread_checker_posix.h View 1 chunk +0 lines, -31 lines 0 comments Download
D third_party/mojo/src/mojo/public/cpp/bindings/lib/thread_checker_posix.cc View 1 chunk +0 lines, -24 lines 0 comments Download

Messages

Total messages: 1 (0 generated)
Elliot Glaysher
5 years, 2 months ago (2015-10-06 18:26:53 UTC) #1
Created Revert of Implement a simple ThreadChecker for Mojo and add to
|internal::SharedData| and |internal::Router|.

Powered by Google App Engine
This is Rietveld 408576698