Move handling of debug urls like chrome://crash, chrome://gpuclean to content. These are for test... (issue 9349010)
j...@chromium.org
Description:
Move handling of debug urls like chrome://crash, chrome://gpuclean to
content.
These are for testing the content layer, so they belong there. This allows
us to
hide GpuProcessHostUIShim from chrome.
I cleaned up the about_handler.* files which were overly complicated. I
also was
able to remove a method from ContentBrowserClient.
BUG=98716
Please review this at http://codereview.chromium.org/9349010/
SVN Base: svn://chrome-svn/chrome/trunk/src/
Affected files:
M chrome/browser/DEPS
M chrome/browser/autofill/autofill_popup_view_browsertest.cc
M chrome/browser/browser_about_handler.cc
M chrome/browser/browser_about_handler_unittest.cc
M chrome/browser/chrome_content_browser_client.h
M chrome/browser/chrome_content_browser_client.cc
M chrome/browser/metrics/metrics_service_uitest.cc
M chrome/browser/prerender/prerender_browsertest.cc
M chrome/browser/ui/webui/about_ui.cc
M chrome/chrome_common.gypi
M chrome/chrome_renderer.gypi
M chrome/chrome_tests.gypi
D chrome/common/about_handler.h
D chrome/common/about_handler.cc
M chrome/common/url_constants.h
M chrome/common/url_constants.cc
M chrome/renderer/chrome_render_view_observer.cc
D chrome/renderer/renderer_about_handler_unittest.cc
M chrome/test/reliability/automated_ui_tests.cc
M chrome/test/reliability/page_load_test.cc
M content/browser/browser_url_handler.cc
M content/browser/child_process_security_policy_unittest.cc
M content/browser/mock_content_browser_client.h
M content/browser/mock_content_browser_client.cc
M content/browser/renderer_host/resource_dispatcher_host_uitest.cc
M content/browser/site_instance_impl.cc
M content/browser/site_instance_impl_unittest.cc
A content/browser/tab_contents/debug_urls.h
A content/browser/tab_contents/debug_urls.cc
M content/browser/tab_contents/navigation_controller_impl.cc
M content/common/test_url_constants.h
M content/common/test_url_constants.cc
M content/content_browser.gypi
M content/public/browser/content_browser_client.h
M content/public/common/url_constants.h
M content/public/common/url_constants.cc
M content/renderer/render_view_impl.cc
M content/shell/shell_content_browser_client.h
M content/shell/shell_content_browser_client.cc
j...@chromium.org
e...@chromium.org
http://codereview.chromium.org/9349010/diff/177/content/public/common/url_constants.cc
File content/public/common/url_constants.cc (right):
http://codereview.chromium.org/9349010/diff/177/content/public/common/url_constants.cc#newcode23
content/public/common/url_constants.cc:23: namespace chrome {
I guess we already have chrome all over here, huh?
http://codereview.chromium.org/9349010/diff/177/content/public/common/url_constants.h
File content/public/common/url_constants.h (right):
http://codereview.chromium.org/9349010/diff/177/content/public/common/url_constants.h#newcode44
content/public/common/url_constants.h:44: CONTENT_EXPORT extern const
char kChromeUIBrowserCrashHost[];
As long as you don't mind "chrome" in these names...
j...@chromium.org
http://codereview.chromium.org/9349010/diff/177/content/public/common/url_constants.cc
File content/public/common/url_constants.cc (right):
http://codereview.chromium.org/9349010/diff/177/content/public/common/url_constants.cc#newcode23
content/public/common/url_constants.cc:23: namespace chrome {
On 2012/02/08 01:06:35, Elliot Glaysher wrote:
> I guess we already have chrome all over here, huh?
yeah we need to rename this namespace to chrome. i'll add it to my list
of todos and add a comment in the header
http://codereview.chromium.org/9349010/diff/177/content/public/common/url_constants.h
File content/public/common/url_constants.h (right):
http://codereview.chromium.org/9349010/diff/177/content/public/common/url_constants.h#newcode44
content/public/common/url_constants.h:44: CONTENT_EXPORT extern const
char kChromeUIBrowserCrashHost[];
On 2012/02/08 01:06:35, Elliot Glaysher wrote:
> As long as you don't mind "chrome" in these names...
i was a bit hesitant. these urls are testing content level stuff. but,
right now chrome only supports these urls when they're in the "chrome"
scheme. i'm not sure we want to expose the chrome/content split in the
UI. so perhaps we can have a way for the embedder to specify the scheme
used for the test urls?
a...@chromium.org
http://codereview.chromium.org/9349010/diff/177/content/browser/tab_contents/debug_urls.cc
File content/browser/tab_contents/debug_urls.cc (right):
http://codereview.chromium.org/9349010/diff/177/content/browser/tab_contents/debug_urls.cc#newcode19
content/browser/tab_contents/debug_urls.cc:19: // Handle URLs to crash
the browser or wreck the gpu process.
Move to be function comment rather than a comment on the code here.
http://codereview.chromium.org/9349010/diff/177/content/public/common/url_constants.cc
File content/public/common/url_constants.cc (right):
http://codereview.chromium.org/9349010/diff/177/content/public/common/url_constants.cc#newcode55
content/public/common/url_constants.cc:55:
Why do some of these end with a slash and some do not?
http://codereview.chromium.org/9349010/diff/177/content/public/common/url_constants.h
File content/public/common/url_constants.h (right):
http://codereview.chromium.org/9349010/diff/177/content/public/common/url_constants.h#newcode44
content/public/common/url_constants.h:44: CONTENT_EXPORT extern const
char kChromeUIBrowserCrashHost[];
Sure, though the scheme used doesn't affect what constant names we use
here.