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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2755973003: DevTools: android shouldn't depend on devtools frontend (Closed)

Created:
3 years, 9 months ago by chenwilliam
Modified:
3 years, 9 months ago
Reviewers:
dgozman, altimin
CC:
chromium-reviews, jam, darin-cc_chromium.org, pfeldman, devtools-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

DevTools: android shouldn't depend on devtools frontend Should fix android CQ failures such as these: https://codereview.chromium.org/2747553002/ BUG=none Review-Url: https://codereview.chromium.org/2755973003 Cr-Commit-Position: refs/heads/master@{#458283} Committed: https://chromium.googlesource.com/chromium/src/+/e88d836999be9b40cf7ee7eedcb4da6080cdeee2

Patch Set 1 #

Patch Set 2 : try Dmitry patchset #

Patch Set 3 : fixup #

Patch Set 4 : fixup #

Patch Set 5 : fixup #

Patch Set 6 : back out dmitry changes #

Total comments: 2

Patch Set 7 : cl fb #

Total comments: 2

Patch Set 8 : fixup #

Patch Set 9 : fixup #

Total comments: 3

Patch Set 10 : fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -13 lines) Patch
M chrome/browser/devtools/remote_debugging_server.cc View 1 2 3 4 5 6 7 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/BUILD.gn View 1 2 3 4 5 6 2 chunks +4 lines, -2 lines 0 comments Download
M content/browser/devtools/BUILD.gn View 1 chunk +5 lines, -3 lines 0 comments Download
M content/public/browser/BUILD.gn View 1 2 3 4 5 6 7 2 chunks +4 lines, -1 line 0 comments Download
M content/public/browser/devtools_frontend_host.h View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -0 lines 0 comments Download
M content/shell/BUILD.gn View 1 2 3 1 chunk +5 lines, -3 lines 0 comments Download
M content/shell/browser/shell_devtools_frontend.h View 1 2 3 4 5 6 7 2 chunks +6 lines, -1 line 0 comments Download
M content/shell/browser/shell_devtools_frontend.cc View 1 2 3 4 5 6 7 3 chunks +13 lines, -0 lines 0 comments Download
M content/shell/browser/shell_devtools_manager_delegate.cc View 1 2 3 4 5 6 7 3 chunks +8 lines, -1 line 0 comments Download
M headless/lib/browser/headless_devtools.cc View 1 2 3 4 5 6 7 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 18 (8 generated)
chenwilliam
ptal https://codereview.chromium.org/2755973003/diff/100001/content/browser/devtools/devtools_frontend_host_impl.cc File content/browser/devtools/devtools_frontend_host_impl.cc (right): https://codereview.chromium.org/2755973003/diff/100001/content/browser/devtools/devtools_frontend_host_impl.cc#newcode17 content/browser/devtools/devtools_frontend_host_impl.cc:17: #include "content/browser/devtools/grit/devtools_resources_map.h" // nogncheck gn complains when cross-compiling ...
3 years, 9 months ago (2017-03-16 23:08:13 UTC) #3
dgozman
https://codereview.chromium.org/2755973003/diff/100001/content/browser/devtools/devtools_frontend_host_impl.cc File content/browser/devtools/devtools_frontend_host_impl.cc (right): https://codereview.chromium.org/2755973003/diff/100001/content/browser/devtools/devtools_frontend_host_impl.cc#newcode17 content/browser/devtools/devtools_frontend_host_impl.cc:17: #include "content/browser/devtools/grit/devtools_resources_map.h" // nogncheck On 2017/03/16 23:08:13, chenwilliam wrote: ...
3 years, 9 months ago (2017-03-16 23:09:39 UTC) #4
chenwilliam
ptal
3 years, 9 months ago (2017-03-20 22:39:02 UTC) #6
dgozman
lgtm https://codereview.chromium.org/2755973003/diff/120001/content/shell/browser/shell_devtools_manager_delegate.cc File content/shell/browser/shell_devtools_manager_delegate.cc (right): https://codereview.chromium.org/2755973003/diff/120001/content/shell/browser/shell_devtools_manager_delegate.cc#newcode210 content/shell/browser/shell_devtools_manager_delegate.cc:210: return content::DevToolsFrontendHost::GetFrontendResource(path).as_string(); We should guard the include somewhere ...
3 years, 9 months ago (2017-03-20 22:43:20 UTC) #7
chenwilliam
@altmin - please take a look at the headless/ files https://codereview.chromium.org/2755973003/diff/120001/content/shell/browser/shell_devtools_manager_delegate.cc File content/shell/browser/shell_devtools_manager_delegate.cc (right): https://codereview.chromium.org/2755973003/diff/120001/content/shell/browser/shell_devtools_manager_delegate.cc#newcode210 ...
3 years, 9 months ago (2017-03-20 23:59:40 UTC) #9
dgozman
https://codereview.chromium.org/2755973003/diff/160001/content/public/browser/devtools_frontend_host.h File content/public/browser/devtools_frontend_host.h (right): https://codereview.chromium.org/2755973003/diff/160001/content/public/browser/devtools_frontend_host.h#newcode22 content/public/browser/devtools_frontend_host.h:22: // Note: DevToolsFrontendHost & Impl not included in android; ...
3 years, 9 months ago (2017-03-21 00:08:43 UTC) #10
altimin
lgtm https://codereview.chromium.org/2755973003/diff/160001/headless/lib/browser/headless_devtools_manager_delegate.cc File headless/lib/browser/headless_devtools_manager_delegate.cc (right): https://codereview.chromium.org/2755973003/diff/160001/headless/lib/browser/headless_devtools_manager_delegate.cc#newcode20 headless/lib/browser/headless_devtools_manager_delegate.cc:20: #if !defined(OS_ANDROID) On 2017/03/20 23:59:40, chenwilliam wrote: > ...
3 years, 9 months ago (2017-03-21 00:12:10 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2755973003/180001
3 years, 9 months ago (2017-03-21 00:47:10 UTC) #14
commit-bot: I haz the power
Committed patchset #10 (id:180001) as https://chromium.googlesource.com/chromium/src/+/e88d836999be9b40cf7ee7eedcb4da6080cdeee2
3 years, 9 months ago (2017-03-21 02:35:32 UTC) #17
chenwilliam
3 years, 9 months ago (2017-03-21 04:22:53 UTC) #18
Message was sent while issue was closed. 
A revert of this CL (patchset #10 id:180001) has been created in
https://codereview.chromium.org/2767513002/ by chenwilliam@chromium.org.

The reason for reverting is: crbug.com/703501.

Powered by Google App Engine
This is Rietveld 408576698