GTTF: Use a fresh TestingBrowserProcess for each test, part #4

GTTF: Use a fresh TestingBrowserProcess for each test, part #4

For a detailed description see part #1, http://codereview.chromium.org/6478005/

BUG=61062
TEST=unit_tests, possibly more

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

[sky, 2011-07-28 00:28:03]

LGTM

http://codereview.chromium.org/7465041/diff/5001/chrome/browser/tab_contents/...
File chrome/browser/tab_contents/thumbnail_generator_unittest.cc (right):

http://codereview.chromium.org/7465041/diff/5001/chrome/browser/tab_contents/...
chrome/browser/tab_contents/thumbnail_generator_unittest.cc:211: class
ThumbnailGeneratorSimpleTest : public TestingBrowserProcessTest {
nit: typedef TestingBrowserProcessTest ThumbnailGeneratorSimpleTest;

http://codereview.chromium.org/7465041/diff/5001/content/browser/tab_contents...
File content/browser/tab_contents/navigation_entry_unittest.cc (right):

http://codereview.chromium.org/7465041/diff/5001/content/browser/tab_contents...
content/browser/tab_contents/navigation_entry_unittest.cc:10: #include
"chrome/test/testing_browser_process_test.h"
nit: before content

http://codereview.chromium.org/7465041/diff/5001/content/common/notification_...
File content/common/notification_service_unittest.cc (right):

http://codereview.chromium.org/7465041/diff/5001/content/common/notification_...
content/common/notification_service_unittest.cc:8: #include
"chrome/test/testing_browser_process_test.h"
nit: before content

[jam, 2011-07-28 01:41:21]

(just looked at the content dir, which is what I assume you wanted me to look
at)

http://codereview.chromium.org/7465041/diff/5001/content/common/DEPS
File content/common/DEPS (right):

http://codereview.chromium.org/7465041/diff/5001/content/common/DEPS#newcode3
content/common/DEPS:3: "+chrome/test",
this is already in content\DEPS

http://codereview.chromium.org/7465041/diff/5001/content/common/notification_...
File content/common/notification_service_unittest.cc (right):

http://codereview.chromium.org/7465041/diff/5001/content/common/notification_...
content/common/notification_service_unittest.cc:35: class
NotificationServiceTest : public TestingBrowserProcessTest {
why should a unit test derive from BrowserProcessTest? or is that something
you'll do in a future cl?

[Paweł Hajdan Jr., 2011-07-28 03:49:17]

http://codereview.chromium.org/7465041/diff/5001/content/common/DEPS
File content/common/DEPS (right):

http://codereview.chromium.org/7465041/diff/5001/content/common/DEPS#newcode3
content/common/DEPS:3: "+chrome/test",
On 2011/07/28 01:41:21, John Abd-El-Malek wrote:
> this is already in content\DEPS

Without this there is a check_deps failure. "-chrome" line above takes
precedence over anything that's in content/DEPS.

http://codereview.chromium.org/7465041/diff/5001/content/common/notification_...
File content/common/notification_service_unittest.cc (right):

http://codereview.chromium.org/7465041/diff/5001/content/common/notification_...
content/common/notification_service_unittest.cc:35: class
NotificationServiceTest : public TestingBrowserProcessTest {
On 2011/07/28 01:41:21, John Abd-El-Malek wrote:
> why should a unit test derive from BrowserProcessTest? or is that something
> you'll do in a future cl?

TestingBrowserProcess has nothing to do with browser tests or
InProcessBrowserTest. Sorry for the confusion; the main idea here is that we
mock out the BrowserProcess object stored in g_browser_process.

[jam, 2011-07-28 16:48:54]

http://codereview.chromium.org/7465041/diff/5001/content/common/DEPS
File content/common/DEPS (right):

http://codereview.chromium.org/7465041/diff/5001/content/common/DEPS#newcode3
content/common/DEPS:3: "+chrome/test",
On 2011/07/28 03:49:18, Paweł Hajdan Jr. wrote:
> On 2011/07/28 01:41:21, John Abd-El-Malek wrote:
> > this is already in content\DEPS
> 
> Without this there is a check_deps failure. "-chrome" line above takes
> precedence over anything that's in content/DEPS.

it seems we can move the "-chrome" up to content\DEPS now. and then we can get
rid of this deps file altogether.

http://codereview.chromium.org/7465041/diff/5001/content/common/notification_...
File content/common/notification_service_unittest.cc (right):

http://codereview.chromium.org/7465041/diff/5001/content/common/notification_...
content/common/notification_service_unittest.cc:35: class
NotificationServiceTest : public TestingBrowserProcessTest {
On 2011/07/28 03:49:18, Paweł Hajdan Jr. wrote:
> On 2011/07/28 01:41:21, John Abd-El-Malek wrote:
> > why should a unit test derive from BrowserProcessTest? or is that something
> > you'll do in a future cl?
> 
> TestingBrowserProcess has nothing to do with browser tests or
> InProcessBrowserTest. Sorry for the confusion; the main idea here is that we
> mock out the BrowserProcess object stored in g_browser_process.

I see, although I'm not sure I understand why unit tests need a BrowserProcess
object around. But we can leave this until we start working on content_unittests

[jam, 2011-07-28 17:45:15]

lgtm