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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 442303002: DevTools: migrate DevTools APIs to use WebContents instead of RenderViewHost. (Closed)

Created:
6 years, 4 months ago by pfeldman
Modified:
6 years, 4 months ago
CC:
chromium-reviews, tburkard+watch_chromium.org, nasko+codewatch_chromium.org, dcheng, aandrey+blink_chromium.org, miu+watch_chromium.org, extensions-reviews_chromium.org, cbentzel+watch_chromium.org, vsevik, darin-cc_chromium.org, devtools-reviews_chromium.org, chromium-apps-reviews_chromium.org, jennb, creis+watch_chromium.org, jianli, gavinp+prer_chromium.org, paulirish+reviews_chromium.org, jochen+watch_chromium.org, davidben+watch_chromium.org, tfarina, Dmitry Titov, yurys, dominich+watch_chromium.org
Project:
chromium
Visibility:
Public.

Description

DevTools: migrate DevTools APIs to use WebContents instead of RenderViewHost. R=dgozman@chromium.org, jam@chromium.org TBR=extensions) for simple RVH -> WebContents migration call sites., kalman (apps, mnaganov (android_webview) NOTRY=true Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288297

Patch Set 1 #

Total comments: 8

Patch Set 2 : Review comments addressed. #

Total comments: 12

Patch Set 3 : Review comments addressed. #

Total comments: 1

Patch Set 4 : rebaselined #

Patch Set 5 : w/ android fix #

Patch Set 6 : More fixes #

Patch Set 7 : rebaselined #

Patch Set 8 : rebaselined #

Patch Set 9 : for landing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+380 lines, -506 lines) Patch
M android_webview/native/aw_dev_tools_server.cc View 1 2 2 chunks +6 lines, -8 lines 0 comments Download
M apps/app_window_registry.cc View 1 1 chunk +4 lines, -4 lines 0 comments Download
M chrome/browser/android/dev_tools_server.cc View 1 2 3 4 5 5 chunks +13 lines, -20 lines 0 comments Download
M chrome/browser/apps/app_browsertest.cc View 2 chunks +8 lines, -8 lines 0 comments Download
M chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc View 1 2 1 chunk +8 lines, -5 lines 0 comments Download
M chrome/browser/devtools/browser_list_tabcontents_provider.cc View 1 2 3 4 5 6 1 chunk +1 line, -4 lines 0 comments Download
M chrome/browser/devtools/chrome_devtools_manager_delegate.cc View 1 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/devtools/devtools_sanity_browsertest.cc View 1 7 chunks +12 lines, -16 lines 0 comments Download
M chrome/browser/devtools/devtools_target_impl.h View 1 4 chunks +8 lines, -6 lines 0 comments Download
M chrome/browser/devtools/devtools_target_impl.cc View 1 2 7 chunks +44 lines, -48 lines 0 comments Download
M chrome/browser/devtools/devtools_targets_ui.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/devtools/devtools_window.h View 1 2 3 4 5 6 7 4 chunks +8 lines, -7 lines 0 comments Download
M chrome/browser/devtools/devtools_window.cc View 1 2 3 4 5 6 7 8 chunks +25 lines, -27 lines 0 comments Download
M chrome/browser/devtools/devtools_window_testing.h View 1 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/devtools/devtools_window_testing.cc View 1 2 chunks +3 lines, -4 lines 0 comments Download
M chrome/browser/extensions/api/app_window/app_window_api.cc View 1 2 chunks +5 lines, -6 lines 0 comments Download
M chrome/browser/extensions/api/debugger/debugger_api.cc View 1 6 chunks +11 lines, -15 lines 0 comments Download
M chrome/browser/extensions/api/developer_private/developer_private_api.cc View 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/extensions/devtools_util.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_service.cc View 1 2 3 4 5 6 7 8 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/prerender/prerender_browsertest.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/printing/print_dialog_cloud.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/renderer_context_menu/render_view_context_menu.cc View 1 2 3 4 5 6 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/renderer_host/render_process_host_chrome_browsertest.cc View 6 chunks +6 lines, -7 lines 0 comments Download
M chrome/browser/task_manager/renderer_resource.h View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/task_manager/renderer_resource.cc View 1 chunk +0 lines, -8 lines 0 comments Download
M chrome/browser/task_manager/resource_provider.h View 1 chunk +0 lines, -6 lines 0 comments Download
M chrome/browser/task_manager/resource_provider.cc View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/task_manager/task_manager.h View 1 chunk +0 lines, -7 lines 0 comments Download
M chrome/browser/task_manager/task_manager.cc View 1 chunk +0 lines, -9 lines 0 comments Download
M chrome/browser/ui/cocoa/extensions/extension_popup_controller.mm View 1 2 5 chunks +6 lines, -5 lines 0 comments Download
M chrome/browser/ui/panels/panel.cc View 1 1 chunk +4 lines, -6 lines 0 comments Download
M chrome/browser/ui/views/extensions/extension_popup.cc View 1 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/ui/webui/extensions/extension_error_ui_util.cc View 1 2 3 4 5 6 7 8 2 chunks +7 lines, -4 lines 0 comments Download
M chrome/browser/ui/webui/extensions/extension_settings_handler.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M content/browser/devtools/devtools_agent_host_impl.h View 1 1 chunk +3 lines, -3 lines 0 comments Download
M content/browser/devtools/devtools_agent_host_impl.cc View 1 1 chunk +5 lines, -3 lines 0 comments Download
M content/browser/devtools/devtools_manager_unittest.cc View 1 6 chunks +8 lines, -8 lines 0 comments Download
M content/browser/devtools/render_view_devtools_agent_host.h View 1 2 3 chunks +10 lines, -11 lines 0 comments Download
M content/browser/devtools/render_view_devtools_agent_host.cc View 1 2 8 chunks +35 lines, -72 lines 0 comments Download
M content/browser/devtools/renderer_overrides_handler.h View 1 2 chunks +3 lines, -0 lines 0 comments Download
M content/browser/devtools/renderer_overrides_handler.cc View 1 2 3 4 5 6 7 8 17 chunks +78 lines, -92 lines 0 comments Download
M content/browser/devtools/renderer_overrides_handler_browsertest.cc View 1 1 chunk +1 line, -3 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_android.cc View 1 2 3 4 5 6 7 8 1 chunk +3 lines, -3 lines 0 comments Download
M content/browser/web_contents/web_contents_impl.cc View 1 2 3 1 chunk +2 lines, -9 lines 0 comments Download
M content/public/browser/devtools_agent_host.h View 1 4 chunks +9 lines, -14 lines 0 comments Download
M content/shell/browser/shell_devtools_delegate.cc View 1 4 chunks +11 lines, -16 lines 0 comments Download
M content/shell/browser/shell_devtools_frontend.cc View 1 2 3 4 5 6 1 chunk +1 line, -2 lines 0 comments Download
M extensions/browser/process_manager.cc View 1 1 chunk +5 lines, -7 lines 0 comments Download

Messages

Total messages: 15 (0 generated)
pfeldman
6 years, 4 months ago (2014-08-06 12:48:58 UTC) #1
dgozman
https://codereview.chromium.org/442303002/diff/1/chrome/browser/devtools/devtools_window.cc File chrome/browser/devtools/devtools_window.cc (right): https://codereview.chromium.org/442303002/diff/1/chrome/browser/devtools/devtools_window.cc#newcode467 chrome/browser/devtools/devtools_window.cc:467: DevToolsWindow* result = ToggleDevToolsWindow( Unnecessary |result| variable. https://codereview.chromium.org/442303002/diff/1/chrome/browser/extensions/api/app_window/app_window_api.cc File ...
6 years, 4 months ago (2014-08-06 13:06:10 UTC) #2
pfeldman
https://codereview.chromium.org/442303002/diff/1/chrome/browser/devtools/devtools_window.cc File chrome/browser/devtools/devtools_window.cc (right): https://codereview.chromium.org/442303002/diff/1/chrome/browser/devtools/devtools_window.cc#newcode467 chrome/browser/devtools/devtools_window.cc:467: DevToolsWindow* result = ToggleDevToolsWindow( On 2014/08/06 13:06:09, dgozman wrote: ...
6 years, 4 months ago (2014-08-06 16:26:35 UTC) #3
dgozman
Should we rename RenderViewDevToolsAgentHost into WebContentsDevToolsAgentHost in a follow-up? https://codereview.chromium.org/442303002/diff/40001/chrome/browser/devtools/devtools_target_impl.cc File chrome/browser/devtools/devtools_target_impl.cc (right): https://codereview.chromium.org/442303002/diff/40001/chrome/browser/devtools/devtools_target_impl.cc#newcode70 chrome/browser/devtools/devtools_target_impl.cc:70: ...
6 years, 4 months ago (2014-08-06 19:22:21 UTC) #4
pfeldman
https://codereview.chromium.org/442303002/diff/40001/chrome/browser/devtools/devtools_target_impl.cc File chrome/browser/devtools/devtools_target_impl.cc (right): https://codereview.chromium.org/442303002/diff/40001/chrome/browser/devtools/devtools_target_impl.cc#newcode70 chrome/browser/devtools/devtools_target_impl.cc:70: if (rfh->IsCrossProcessSubframe()) { We'll figure it out. Updated the ...
6 years, 4 months ago (2014-08-07 09:03:09 UTC) #5
dgozman
lgtm https://codereview.chromium.org/442303002/diff/60001/chrome/browser/ui/webui/extensions/extension_error_ui_util.cc File chrome/browser/ui/webui/extensions/extension_error_ui_util.cc (right): https://codereview.chromium.org/442303002/diff/60001/chrome/browser/ui/webui/extensions/extension_error_ui_util.cc#newcode160 chrome/browser/ui/webui/extensions/extension_error_ui_util.cc:160: // Once we open the inspector, we focus ...
6 years, 4 months ago (2014-08-07 12:06:27 UTC) #6
pfeldman
@jam: Could you take a look at content/public/browser/devtools_agent_host.h? That's where RenderViewHost was replaced with WebContents. ...
6 years, 4 months ago (2014-08-07 12:15:24 UTC) #7
jam
On 2014/08/07 12:15:24, pfeldman wrote: > @jam: Could you take a look at content/public/browser/devtools_agent_host.h? > ...
6 years, 4 months ago (2014-08-08 01:21:27 UTC) #8
pfeldman
The CQ bit was checked by pfeldman@chromium.org
6 years, 4 months ago (2014-08-08 05:11:33 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pfeldman@chromium.org/442303002/180001
6 years, 4 months ago (2014-08-08 05:13:55 UTC) #10
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: chromium_presubmit on tryserver.chromium.linux ...
6 years, 4 months ago (2014-08-08 08:29:41 UTC) #11
pfeldman
The CQ bit was unchecked by pfeldman@chromium.org
6 years, 4 months ago (2014-08-08 08:35:54 UTC) #12
pfeldman
The CQ bit was checked by pfeldman@chromium.org
6 years, 4 months ago (2014-08-08 08:35:57 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pfeldman@chromium.org/442303002/180001
6 years, 4 months ago (2014-08-08 08:36:17 UTC) #14
pfeldman
6 years, 4 months ago (2014-08-08 10:09:11 UTC) #15
Message was sent while issue was closed. 
Committed patchset #9 manually as 288297 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698