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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 294903014: [DevTools] Add toolbox web contents to show in undocked mode. (Closed)

Created:
6 years, 7 months ago by dgozman
Modified:
6 years, 6 months ago
CC:
chromium-reviews, vsevik, tfarina, yurys, paulirish+reviews_chromium.org, devtools-reviews_chromium.org, aandrey+blink_chromium.org
Visibility:
Public.

Description

[DevTools] Add toolbox web contents to show in undocked mode. When docking is possible, but main window is undocked, we show toolbox web contents. This means that undocked DevTools has two web contents at the same time: one in a separate window, and one near the inspected tab. BUG=327641 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273802

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : mac fixes #

Total comments: 4

Patch Set 5 : Comments addressed #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+217 lines, -181 lines) Patch
M chrome/browser/devtools/devtools_sanity_browsertest.cc View 1 14 chunks +29 lines, -27 lines 0 comments Download
M chrome/browser/devtools/devtools_window.h View 1 2 3 4 4 chunks +18 lines, -18 lines 0 comments Download
M chrome/browser/devtools/devtools_window.cc View 1 2 3 4 24 chunks +117 lines, -65 lines 0 comments Download
M chrome/browser/policy/policy_browsertest.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/task_manager/tab_contents_information.cc View 1 2 3 4 1 chunk +4 lines, -4 lines 0 comments Download
M chrome/browser/ui/cocoa/browser_window_controller.mm View 1 2 3 4 2 chunks +8 lines, -10 lines 2 comments Download
M chrome/browser/ui/cocoa/browser_window_controller_private.mm View 1 2 3 4 1 chunk +3 lines, -6 lines 0 comments Download
M chrome/browser/ui/cocoa/dev_tools_controller.h View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/cocoa/dev_tools_controller.mm View 1 2 3 4 2 chunks +10 lines, -11 lines 0 comments Download
M chrome/browser/ui/cocoa/dev_tools_controller_browsertest.mm View 1 2 3 4 1 chunk +5 lines, -5 lines 0 comments Download
M chrome/browser/ui/cocoa/tab_contents/tab_contents_controller.mm View 1 2 3 4 1 chunk +4 lines, -5 lines 0 comments Download
M chrome/browser/ui/views/frame/browser_view.h View 1 2 3 4 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/ui/views/frame/browser_view.cc View 1 2 3 4 4 chunks +15 lines, -23 lines 0 comments Download

Messages

Total messages: 11 (0 generated)
dgozman
Take a look please.
6 years, 7 months ago (2014-05-27 13:28:01 UTC) #1
pfeldman
https://codereview.chromium.org/294903014/diff/60001/chrome/browser/devtools/devtools_window.cc File chrome/browser/devtools/devtools_window.cc (right): https://codereview.chromium.org/294903014/diff/60001/chrome/browser/devtools/devtools_window.cc#newcode778 chrome/browser/devtools/devtools_window.cc:778: new DevToolsToolboxDelegate(toolbox_web_contents_)); When is the delegate deleted? https://codereview.chromium.org/294903014/diff/60001/chrome/browser/devtools/devtools_window.h File ...
6 years, 7 months ago (2014-05-27 15:26:19 UTC) #2
dgozman
https://codereview.chromium.org/294903014/diff/60001/chrome/browser/devtools/devtools_window.cc File chrome/browser/devtools/devtools_window.cc (right): https://codereview.chromium.org/294903014/diff/60001/chrome/browser/devtools/devtools_window.cc#newcode778 chrome/browser/devtools/devtools_window.cc:778: new DevToolsToolboxDelegate(toolbox_web_contents_)); On 2014/05/27 15:26:19, pfeldman wrote: > When ...
6 years, 6 months ago (2014-05-29 11:22:23 UTC) #3
pfeldman
lgtm
6 years, 6 months ago (2014-05-29 11:24:37 UTC) #4
dgozman
Could you please take a look? avi@ at mac stuff. sky@ at views part and ...
6 years, 6 months ago (2014-05-29 15:12:13 UTC) #5
Avi (use Gerrit)
Mac stuff LGTM https://codereview.chromium.org/294903014/diff/80001/chrome/browser/ui/cocoa/browser_window_controller.mm File chrome/browser/ui/cocoa/browser_window_controller.mm (right): https://codereview.chromium.org/294903014/diff/80001/chrome/browser/ui/cocoa/browser_window_controller.mm#newcode655 chrome/browser/ui/cocoa/browser_window_controller.mm:655: contents, NULL); Can you fit this ...
6 years, 6 months ago (2014-05-29 15:18:06 UTC) #6
sky
LGTM
6 years, 6 months ago (2014-05-29 19:25:12 UTC) #7
dgozman
Thank you for review. https://codereview.chromium.org/294903014/diff/80001/chrome/browser/ui/cocoa/browser_window_controller.mm File chrome/browser/ui/cocoa/browser_window_controller.mm (right): https://codereview.chromium.org/294903014/diff/80001/chrome/browser/ui/cocoa/browser_window_controller.mm#newcode655 chrome/browser/ui/cocoa/browser_window_controller.mm:655: contents, NULL); On 2014/05/29 15:18:07, ...
6 years, 6 months ago (2014-05-30 09:59:48 UTC) #8
dgozman
The CQ bit was checked by dgozman@chromium.org
6 years, 6 months ago (2014-05-30 09:59:56 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dgozman@chromium.org/294903014/80001
6 years, 6 months ago (2014-05-30 10:01:56 UTC) #10
commit-bot: I haz the power
6 years, 6 months ago (2014-05-30 10:55:59 UTC) #11
Message was sent while issue was closed. 
Change committed as 273802

Powered by Google App Engine
This is Rietveld 408576698