Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(784)

Issue 2755143002: Revert of Avoid calling isMainThread() in the common cases when checking ScriptForbiddenScope::isSc… (Closed)

Created:
1 year, 1 month ago by esprehn
Modified:
1 year, 1 month ago
Reviewers:
sof, dglazkov, jbroman
CC:
blink-reviews, chromium-reviews, kinuko+watch
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Avoid calling isMainThread() in the common cases when checking ScriptForbiddenScope::isScriptForbid… (patchset #2 id:20001 of https://codereview.chromium.org/2709763003/ ) Reason for revert: meh. Original issue's description: > Avoid calling isMainThread() in the common cases when checking ScriptForbiddenScope::isScriptForbidden(). > > Usually when s_scriptForbiddenCount is true we're about to crash, so check that > first then check which thread it is. This means a background thread will read > the potentially racy value of the counter, but it'll always return false when > it checks isMainThread() next. > > Review-Url: https://codereview.chromium.org/2709763003 > Cr-Commit-Position: refs/heads/master@{#456284} > Committed: https://chromium.googlesource.com/chromium/src/+/3b4b07ced6315a25960ca5a51dd1b7ba88de53db TBR=dglazkov@chromium.org,jbroman@chromium.org,sigbjornf@opera.com # Not skipping CQ checks because original CL landed more than 1 days ago. Review-Url: https://codereview.chromium.org/2755143002 Cr-Commit-Position: refs/heads/master@{#458085} Committed: https://chromium.googlesource.com/chromium/src/+/36b4ac1bd5e75a1d3a37cfb7188c50cd754a3ca7

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -5 lines) Patch
M third_party/WebKit/Source/platform/ScriptForbiddenScope.h View 1 chunk +1 line, -5 lines 0 comments Download

Messages

Total messages: 9 (5 generated)
esprehn
Created Revert of Avoid calling isMainThread() in the common cases when checking ScriptForbiddenScope::isScriptForbid…
1 year, 1 month ago (2017-03-17 15:29:33 UTC) #2
sof
if needed, lgtm (acquireLoad(..) not palatable? )
1 year, 1 month ago (2017-03-17 18:44:20 UTC) #4
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/2755143002/1
1 year, 1 month ago (2017-03-20 15:05:19 UTC) #6
commit-bot: I haz the power
1 year, 1 month ago (2017-03-20 16:37:17 UTC) #9
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/36b4ac1bd5e75a1d3a37cfb7188c...

Powered by Google App Engine
This is Rietveld 408576698