Add ChromeRenderViewHostTestHarness to get rid of the dependency from RVHTH to profile

Add ChromeRenderViewHostTestHarness to get rid of the dependency from RVHTH to profile

BUG=90443
TEST=everything still works

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=101037

[jochen (gone - plz use gerrit), 2011-09-13 14:33:12]

plz review

With this CL it should be possible to move more tests from unit_tests to
content_unittests and use RVHTH instead of CRVHTH everywhere in content/ - I'll
do that in a follow-up CL

[jam, 2011-09-13 18:04:03]

lgtm

http://codereview.chromium.org/7892007/diff/1003/chrome/browser/content_setti...
File chrome/browser/content_settings/tab_specific_content_settings_unittest.cc
(right):

http://codereview.chromium.org/7892007/diff/1003/chrome/browser/content_setti...
chrome/browser/content_settings/tab_specific_content_settings_unittest.cc:16: :
ChromeRenderViewHostTestHarness(),
nit: not really needed

http://codereview.chromium.org/7892007/diff/1003/chrome/browser/password_mana...
File chrome/browser/password_manager/password_manager_unittest.cc (right):

http://codereview.chromium.org/7892007/diff/1003/chrome/browser/password_mana...
chrome/browser/password_manager/password_manager_unittest.cc:99: virtual void
TearDown() {
nit: should probably call the parent TearDown here?

[Paweł Hajdan Jr., 2011-09-13 19:16:36]

http://codereview.chromium.org/7892007/diff/4001/chrome/browser/tab_contents/...
File chrome/browser/tab_contents/web_contents_unittest.cc (right):

http://codereview.chromium.org/7892007/diff/4001/chrome/browser/tab_contents/...
chrome/browser/tab_contents/web_contents_unittest.cc:175: :
ChromeRenderViewHostTestHarness(),
nit: How about removing this?

http://codereview.chromium.org/7892007/diff/4001/chrome/browser/tab_contents/...
chrome/browser/tab_contents/web_contents_unittest.cc:184:
browser_context_.reset(profile);
nit: ChromeRender...TestHarness is going to initialize that anyway, right?

http://codereview.chromium.org/7892007/diff/4001/chrome/browser/tabs/tab_stri...
File chrome/browser/tabs/tab_strip_model_unittest.cc (right):

http://codereview.chromium.org/7892007/diff/4001/chrome/browser/tabs/tab_stri...
chrome/browser/tabs/tab_strip_model_unittest.cc:153: :
ChromeRenderViewHostTestHarness(),
nit: Remove?

http://codereview.chromium.org/7892007/diff/4001/chrome/browser/ui/tab_conten...
File chrome/browser/ui/tab_contents/test_tab_contents_wrapper.cc (right):

http://codereview.chromium.org/7892007/diff/4001/chrome/browser/ui/tab_conten...
chrome/browser/ui/tab_contents/test_tab_contents_wrapper.cc:32: if
(!browser_context_.get())
Is this needed? Why don't we call ChromeRender...TestHarness::SetUp here?

http://codereview.chromium.org/7892007/diff/4001/chrome/browser/ui/tab_conten...
chrome/browser/ui/tab_contents/test_tab_contents_wrapper.cc:46:
message_loop_.RunAllPending();
Why don't we call parent's TearDown here?

http://codereview.chromium.org/7892007/diff/4001/chrome/browser/visitedlink/v...
File chrome/browser/visitedlink/visitedlink_unittest.cc (right):

http://codereview.chromium.org/7892007/diff/4001/chrome/browser/visitedlink/v...
chrome/browser/visitedlink/visitedlink_unittest.cc:569: :
ChromeRenderViewHostTestHarness(),
nit: Needed?

http://codereview.chromium.org/7892007/diff/4001/chrome/browser/web_applicati...
File chrome/browser/web_applications/web_app_unittest.cc (right):

http://codereview.chromium.org/7892007/diff/4001/chrome/browser/web_applicati...
chrome/browser/web_applications/web_app_unittest.cc:20: :
TabContentsWrapperTestHarness(),
nit: Needed?

http://codereview.chromium.org/7892007/diff/4001/chrome/test/base/chrome_rend...
File chrome/test/base/chrome_render_view_host_test_harness.cc (right):

http://codereview.chromium.org/7892007/diff/4001/chrome/test/base/chrome_rend...
chrome/test/base/chrome_render_view_host_test_harness.cc:10: :
RenderViewHostTestHarness() {
nit: Needed?

http://codereview.chromium.org/7892007/diff/4001/chrome/test/base/chrome_rend...
chrome/test/base/chrome_render_view_host_test_harness.cc:17: return
browser_context_.get() ?
Why don't just static_cast always? Casting NULL should be fine.

http://codereview.chromium.org/7892007/diff/4001/chrome/test/base/chrome_rend...
File chrome/test/base/chrome_render_view_host_test_harness.h (right):

http://codereview.chromium.org/7892007/diff/4001/chrome/test/base/chrome_rend...
chrome/test/base/chrome_render_view_host_test_harness.h:29:
DISALLOW_COPY_AND_ASSIGN(ChromeRenderViewHostTestHarness);
nit: No need for that for test fixtures.

[jochen (gone - plz use gerrit), 2011-09-13 19:39:35]

I've addressed all other comments

http://codereview.chromium.org/7892007/diff/4001/chrome/browser/ui/tab_conten...
File chrome/browser/ui/tab_contents/test_tab_contents_wrapper.cc (right):

http://codereview.chromium.org/7892007/diff/4001/chrome/browser/ui/tab_conten...
chrome/browser/ui/tab_contents/test_tab_contents_wrapper.cc:32: if
(!browser_context_.get())
On 2011/09/13 19:16:36, Paweł Hajdan Jr. wrote:
> Is this needed? Why don't we call ChromeRender...TestHarness::SetUp here?

because that would create a "raw" tab contents. TabContentsWrapperTestHarness
overrides all those methods to use contents_wrapper_ instead

http://codereview.chromium.org/7892007/diff/4001/chrome/browser/ui/tab_conten...
chrome/browser/ui/tab_contents/test_tab_contents_wrapper.cc:46:
message_loop_.RunAllPending();
On 2011/09/13 19:16:36, Paweł Hajdan Jr. wrote:
> Why don't we call parent's TearDown here?

because we didn't invoke the parent's setup

[Paweł Hajdan Jr., 2011-09-13 19:41:09]

lgtm