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

Issue 19240007: Split TLS implementation into its own translation unit. (Closed)

Created:
7 years, 5 months ago by bungeman-skia
Modified:
7 years, 5 months ago
Reviewers:
robertphillips
CC:
skia-review_googlegroups.com
Visibility:
Public.

Description

Split TLS implementation into its own translation unit. SkTLS has it's own header separate from SkThread, and having SkThread own the platform implementation of SkTLS is problematic with Chromium. The simplest way to clean this up is to put the implementation in its own set of files, where it is also more easily found. R=robertphillips@google.com Committed: https://code.google.com/p/skia/source/detail?r=10105

Patch Set 1 #

Total comments: 2

Patch Set 2 : Explicitly set static initializers used before explicit assignment. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+134 lines, -117 lines) Patch
M gyp/ports.gyp View 7 chunks +11 lines, -8 lines 0 comments Download
A src/ports/SkTLS_none.cpp View 1 1 chunk +18 lines, -0 lines 0 comments Download
A src/ports/SkTLS_pthread.cpp View 1 chunk +30 lines, -0 lines 0 comments Download
A src/ports/SkTLS_win.cpp View 1 1 chunk +75 lines, -0 lines 0 comments Download
M src/ports/SkThread_none.cpp View 3 chunks +0 lines, -15 lines 0 comments Download
M src/ports/SkThread_pthread.cpp View 2 chunks +0 lines, -23 lines 0 comments Download
M src/ports/SkThread_win.cpp View 3 chunks +0 lines, -71 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
bungeman-skia
From looking into crbug.com/258499 it appeared that <chromium>/src/skia/ext/SkThread_chrome.cc should implement sk_atomic_load_memory_order_relaxed or some such thing ...
7 years, 5 months ago (2013-07-15 21:29:09 UTC) #1
robertphillips
lgtm + 2 questions/suggestions https://codereview.chromium.org/19240007/diff/1/src/ports/SkTLS_none.cpp File src/ports/SkTLS_none.cpp (right): https://codereview.chromium.org/19240007/diff/1/src/ports/SkTLS_none.cpp#newcode9 src/ports/SkTLS_none.cpp:9: initialize to NULL? https://codereview.chromium.org/19240007/diff/1/src/ports/SkTLS_win.cpp File ...
7 years, 5 months ago (2013-07-16 14:44:12 UTC) #2
bungeman-skia
7 years, 5 months ago (2013-07-16 14:59:28 UTC) #3
Message was sent while issue was closed.
Committed patchset #2 manually as r10105 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698