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

Issue 1004113004: [DevTools] Do not instrument with InspectorInspectorAgent. (Closed)

Created:
5 years, 9 months ago by dgozman
Modified:
5 years, 9 months ago
Reviewers:
pfeldman, yurys
CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, malch+blink_chromium.org, vivekg_samsung, vivekg, yurys+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, Inactive, devtools-reviews_chromium.org, arv+blink, sergeyv+blink_chromium.org, aandrey+blink_chromium.org, kozyatinskiy+blink_chromium.org
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

[DevTools] Do not instrument with InspectorInspectorAgent. BUG=none Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=192619

Patch Set 1 #

Total comments: 2

Patch Set 2 : no instrumentation #

Total comments: 2

Patch Set 3 : ASSERT(isMainThread) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -22 lines) Patch
M Source/core/inspector/InspectorInspectorAgent.h View 1 1 chunk +2 lines, -4 lines 0 comments Download
M Source/core/inspector/InspectorInspectorAgent.cpp View 1 5 chunks +9 lines, -11 lines 0 comments Download
M Source/core/inspector/InspectorInstrumentation.cpp View 1 2 5 chunks +7 lines, -4 lines 0 comments Download
M Source/core/inspector/InspectorInstrumentation.idl View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/web/WebDevToolsAgentImpl.cpp View 1 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 14 (4 generated)
dgozman
Take a look please.
5 years, 9 months ago (2015-03-23 14:00:15 UTC) #2
pfeldman
https://codereview.chromium.org/1004113004/diff/1/Source/core/inspector/InspectorInspectorAgent.cpp File Source/core/inspector/InspectorInspectorAgent.cpp (right): https://codereview.chromium.org/1004113004/diff/1/Source/core/inspector/InspectorInspectorAgent.cpp#newcode73 Source/core/inspector/InspectorInspectorAgent.cpp:73: m_instrumentingAgents->setInspectorInspectorAgent(this); I don't see why this domain should instrument ...
5 years, 9 months ago (2015-03-24 13:18:54 UTC) #3
dgozman
https://codereview.chromium.org/1004113004/diff/1/Source/core/inspector/InspectorInspectorAgent.cpp File Source/core/inspector/InspectorInspectorAgent.cpp (right): https://codereview.chromium.org/1004113004/diff/1/Source/core/inspector/InspectorInspectorAgent.cpp#newcode73 Source/core/inspector/InspectorInspectorAgent.cpp:73: m_instrumentingAgents->setInspectorInspectorAgent(this); On 2015/03/24 13:18:54, pfeldman wrote: > I don't ...
5 years, 9 months ago (2015-03-24 13:19:40 UTC) #4
dgozman
Take a look please.
5 years, 9 months ago (2015-03-25 11:29:06 UTC) #6
dgozman
On 2015/03/25 11:29:06, dgozman wrote: > Take a look please. Friendly ping.
5 years, 9 months ago (2015-03-26 11:08:52 UTC) #7
yurys
lgtm
5 years, 9 months ago (2015-03-26 12:53:36 UTC) #8
yurys
https://codereview.chromium.org/1004113004/diff/20001/Source/core/inspector/InspectorInstrumentation.cpp File Source/core/inspector/InspectorInstrumentation.cpp (right): https://codereview.chromium.org/1004113004/diff/20001/Source/core/inspector/InspectorInstrumentation.cpp#newcode121 Source/core/inspector/InspectorInstrumentation.cpp:121: if (!instrumentingAgentsSet) Can you add an assert that instrumentingAgentsSet ...
5 years, 9 months ago (2015-03-26 12:55:33 UTC) #9
dgozman
https://codereview.chromium.org/1004113004/diff/20001/Source/core/inspector/InspectorInstrumentation.cpp File Source/core/inspector/InspectorInstrumentation.cpp (right): https://codereview.chromium.org/1004113004/diff/20001/Source/core/inspector/InspectorInstrumentation.cpp#newcode121 Source/core/inspector/InspectorInstrumentation.cpp:121: if (!instrumentingAgentsSet) On 2015/03/26 12:55:33, yurys wrote: > Can ...
5 years, 9 months ago (2015-03-26 15:53:00 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1004113004/40001
5 years, 9 months ago (2015-03-26 15:53:21 UTC) #13
commit-bot: I haz the power
5 years, 9 months ago (2015-03-26 18:14:49 UTC) #14
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=192619

Powered by Google App Engine
This is Rietveld 408576698