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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 338283004: DevTools: Use TargetsToolbar instead of ThreadToolbar (Closed)

Created:
6 years, 6 months ago by sergeyv
Modified:
6 years, 6 months ago
Reviewers:
vsevik
CC:
blink-reviews, 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
Project:
blink
Visibility:
Public.

Description

DevTools: Use TargetsToolbar instead of ThreadToolbar BUG= Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176508

Patch Set 1 #

Total comments: 4

Patch Set 2 : Address vsevik's comments #

Total comments: 13

Patch Set 3 : Address vsevik's comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+167 lines, -134 lines) Patch
M Source/devtools/devtools.gypi View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/devtools/front_end/components/ExecutionContextSelector.js View 1 2 3 chunks +46 lines, -2 lines 0 comments Download
M Source/devtools/front_end/console/ConsoleView.js View 1 2 3 chunks +19 lines, -0 lines 0 comments Download
M Source/devtools/front_end/main/Main.js View 1 2 chunks +2 lines, -2 lines 0 comments Download
M Source/devtools/front_end/sdk/Target.js View 3 chunks +15 lines, -3 lines 0 comments Download
M Source/devtools/front_end/sdk/WorkerTargetManager.js View 1 chunk +1 line, -1 line 0 comments Download
M Source/devtools/front_end/sources/SourcesPanel.js View 1 3 chunks +3 lines, -3 lines 0 comments Download
A Source/devtools/front_end/sources/TargetsToolbar.js View 1 2 1 chunk +78 lines, -0 lines 0 comments Download
D Source/devtools/front_end/sources/ThreadsToolbar.js View 1 chunk +0 lines, -120 lines 0 comments Download
M Source/devtools/front_end/sourcesPanel.css View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/devtools/scripts/frontend_modules.json View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 8 (0 generated)
sergeyv
6 years, 6 months ago (2014-06-17 09:53:41 UTC) #1
vsevik
https://chromiumcodereview.appspot.com/338283004/diff/1/Source/devtools/front_end/console/ConsoleView.js File Source/devtools/front_end/console/ConsoleView.js (right): https://chromiumcodereview.appspot.com/338283004/diff/1/Source/devtools/front_end/console/ConsoleView.js#newcode143 Source/devtools/front_end/console/ConsoleView.js:143: WebInspector.executionContextSelector.addExecutionContextChangeListener(this._executionContextExternallyChanged, this); Let's make all components use WebInspector.context https://chromiumcodereview.appspot.com/338283004/diff/1/Source/devtools/front_end/sources/TargetsToolbar.js ...
6 years, 6 months ago (2014-06-18 12:41:32 UTC) #2
sergeyv
https://codereview.chromium.org/338283004/diff/1/Source/devtools/front_end/console/ConsoleView.js File Source/devtools/front_end/console/ConsoleView.js (right): https://codereview.chromium.org/338283004/diff/1/Source/devtools/front_end/console/ConsoleView.js#newcode143 Source/devtools/front_end/console/ConsoleView.js:143: WebInspector.executionContextSelector.addExecutionContextChangeListener(this._executionContextExternallyChanged, this); On 2014/06/18 12:41:32, vsevik wrote: > Let's ...
6 years, 6 months ago (2014-06-18 16:50:08 UTC) #3
vsevik
lgtm https://codereview.chromium.org/338283004/diff/20001/Source/devtools/front_end/components/ExecutionContextSelector.js File Source/devtools/front_end/components/ExecutionContextSelector.js (right): https://codereview.chromium.org/338283004/diff/20001/Source/devtools/front_end/components/ExecutionContextSelector.js#newcode64 Source/devtools/front_end/components/ExecutionContextSelector.js:64: if (!newTarget || currentContext && currentContext.target() === newTarget) ...
6 years, 6 months ago (2014-06-19 07:52:42 UTC) #4
sergeyv
https://codereview.chromium.org/338283004/diff/20001/Source/devtools/front_end/components/ExecutionContextSelector.js File Source/devtools/front_end/components/ExecutionContextSelector.js (right): https://codereview.chromium.org/338283004/diff/20001/Source/devtools/front_end/components/ExecutionContextSelector.js#newcode64 Source/devtools/front_end/components/ExecutionContextSelector.js:64: if (!newTarget || currentContext && currentContext.target() === newTarget) On ...
6 years, 6 months ago (2014-06-19 10:54:44 UTC) #5
sergeyv
The CQ bit was checked by sergeyv@chromium.org
6 years, 6 months ago (2014-06-19 10:55:11 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergeyv@chromium.org/338283004/40001
6 years, 6 months ago (2014-06-19 10:57:07 UTC) #7
commit-bot: I haz the power
6 years, 6 months ago (2014-06-19 12:56:48 UTC) #8
Message was sent while issue was closed. 
Change committed as 176508

Powered by Google App Engine
This is Rietveld 408576698