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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 688623002: [Thread-safe] Remove TracedValue::setArray (Closed)

Created:
6 years, 1 month ago by hiroshige
Modified:
6 years, 1 month ago
Reviewers:
caseq, pdr., yurys
CC:
caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, malch+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, vsevik+blink_chromium.org, pfeldman+blink_chromium.org, paulirish+reviews_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, sergeyv+blink_chromium.org, aandrey+blink_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

[Thread-safe] Remove TracedValue::setArray JSONArray can contain thread-unsafe Strings and thus isn't safe to be contained in TracedValue. However, TracedValue::setArray takes JSONArray as input. This CL removes setArray and adds ScriptCallStack::toTracedValue() that uses other thread-safe methods in TracedValue instead. BUG=427893 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185034

Patch Set 1 #

Patch Set 2 : #

Total comments: 4

Patch Set 3 : Rebase. / Rename to ScriptCallStack::toTracedValue. #

Patch Set 4 : Rename to ScriptCallFrame::toTracedValue. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+32 lines, -9 lines) Patch
M Source/core/inspector/InspectorTraceEvents.cpp View 1 2 3 chunks +3 lines, -3 lines 0 comments Download
M Source/core/inspector/ScriptCallFrame.h View 1 2 3 2 chunks +3 lines, -0 lines 0 comments Download
M Source/core/inspector/ScriptCallFrame.cpp View 1 2 3 3 chunks +15 lines, -0 lines 0 comments Download
M Source/core/inspector/ScriptCallStack.h View 1 2 2 chunks +2 lines, -0 lines 0 comments Download
M Source/core/inspector/ScriptCallStack.cpp View 1 2 3 2 chunks +9 lines, -0 lines 0 comments Download
M Source/platform/TracedValue.h View 1 chunk +0 lines, -1 line 0 comments Download
M Source/platform/TracedValue.cpp View 1 chunk +0 lines, -5 lines 0 comments Download

Messages

Total messages: 16 (5 generated)
hiroshige
PTAL. Follow-up from https://codereview.chromium.org/683103003/.
6 years, 1 month ago (2014-11-06 11:01:36 UTC) #2
pdr.
On 2014/11/06 at 11:01:36, hiroshige wrote: > PTAL. Follow-up from https://codereview.chromium.org/683103003/. LGTM. Thank you for ...
6 years, 1 month ago (2014-11-07 03:05:15 UTC) #3
yurys
lgtm https://codereview.chromium.org/688623002/diff/20001/Source/core/inspector/ScriptCallStack.h File Source/core/inspector/ScriptCallStack.h (right): https://codereview.chromium.org/688623002/diff/20001/Source/core/inspector/ScriptCallStack.h#newcode60 Source/core/inspector/ScriptCallStack.h:60: void setInspectorArray(TracedValue*, const char* name) const; nit: can ...
6 years, 1 month ago (2014-11-07 13:15:27 UTC) #5
hiroshige
https://codereview.chromium.org/688623002/diff/20001/Source/core/inspector/ScriptCallStack.h File Source/core/inspector/ScriptCallStack.h (right): https://codereview.chromium.org/688623002/diff/20001/Source/core/inspector/ScriptCallStack.h#newcode60 Source/core/inspector/ScriptCallStack.h:60: void setInspectorArray(TracedValue*, const char* name) const; Done. Should I ...
6 years, 1 month ago (2014-11-10 08:02:59 UTC) #6
yurys
https://codereview.chromium.org/688623002/diff/20001/Source/core/inspector/ScriptCallStack.h File Source/core/inspector/ScriptCallStack.h (right): https://codereview.chromium.org/688623002/diff/20001/Source/core/inspector/ScriptCallStack.h#newcode60 Source/core/inspector/ScriptCallStack.h:60: void setInspectorArray(TracedValue*, const char* name) const; On 2014/11/10 08:02:59, ...
6 years, 1 month ago (2014-11-10 08:08:17 UTC) #7
hiroshige
https://codereview.chromium.org/688623002/diff/20001/Source/core/inspector/ScriptCallStack.h File Source/core/inspector/ScriptCallStack.h (right): https://codereview.chromium.org/688623002/diff/20001/Source/core/inspector/ScriptCallStack.h#newcode60 Source/core/inspector/ScriptCallStack.h:60: void setInspectorArray(TracedValue*, const char* name) const; On 2014/11/10 08:08:17, ...
6 years, 1 month ago (2014-11-10 08:19:09 UTC) #8
yurys
On 2014/11/10 08:19:09, hiroshige wrote: > https://codereview.chromium.org/688623002/diff/20001/Source/core/inspector/ScriptCallStack.h > File Source/core/inspector/ScriptCallStack.h (right): > > https://codereview.chromium.org/688623002/diff/20001/Source/core/inspector/ScriptCallStack.h#newcode60 > ...
6 years, 1 month ago (2014-11-10 08:35:43 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/688623002/60001
6 years, 1 month ago (2014-11-10 10:06:53 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/35573)
6 years, 1 month ago (2014-11-10 12:19:05 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/688623002/60001
6 years, 1 month ago (2014-11-10 13:33:55 UTC) #15
commit-bot: I haz the power
6 years, 1 month ago (2014-11-10 13:34:38 UTC) #16
Message was sent while issue was closed. 
Committed patchset #4 (id:60001) as 185034

Powered by Google App Engine
This is Rietveld 408576698