Fix tests that were setting safebrowsing factories without unsetting them.

Fix tests that were setting safebrowsing factories without unsetting them.

BUG=313141
R=shess@chromium.org, tburkard@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=232882

[mattm, 2013-10-31 21:16:20]

shess: chrome/browser/safe_browsing
tburkard: chrome/browser/prerender

[Scott Hess - ex-Googler, 2013-10-31 22:07:20]

Good catch.

https://codereview.chromium.org/55323002/diff/1/chrome/browser/safe_browsing/...
File chrome/browser/safe_browsing/database_manager_unittest.cc (right):

https://codereview.chromium.org/55323002/diff/1/chrome/browser/safe_browsing/...
chrome/browser/safe_browsing/database_manager_unittest.cc:48:
SafeBrowsingService::RegisterFactory(NULL);
PlatformTest::TearDown() after.

https://codereview.chromium.org/55323002/diff/1/chrome/browser/safe_browsing/...
File chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
(left):

https://codereview.chromium.org/55323002/diff/1/chrome/browser/safe_browsing/...
chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc:132:
MalwareDetails::RegisterFactory(NULL);  // Create it fresh each time.
Shouldn't this move to TearDown() rather than being deleted?  Or is it just not
used here?  In that case can malware_details.h be removed?

[mattm, 2013-10-31 22:39:22]

https://codereview.chromium.org/55323002/diff/1/chrome/browser/safe_browsing/...
File chrome/browser/safe_browsing/database_manager_unittest.cc (right):

https://codereview.chromium.org/55323002/diff/1/chrome/browser/safe_browsing/...
chrome/browser/safe_browsing/database_manager_unittest.cc:48:
SafeBrowsingService::RegisterFactory(NULL);
On 2013/10/31 22:07:20, shess wrote:
> PlatformTest::TearDown() after.

Oops, Done.

https://codereview.chromium.org/55323002/diff/1/chrome/browser/safe_browsing/...
File chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
(left):

https://codereview.chromium.org/55323002/diff/1/chrome/browser/safe_browsing/...
chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc:132:
MalwareDetails::RegisterFactory(NULL);  // Create it fresh each time.
On 2013/10/31 22:07:20, shess wrote:
> Shouldn't this move to TearDown() rather than being deleted?  Or is it just
not
> used here?  In that case can malware_details.h be removed?

I believe it is just not being used. (Nothing was setting it.)

Well looked up the history, I guess it was part of
https://code.google.com/p/chromium/issues/detail?id=110594 ...
I moved it to TearDown and added a better comment, but that approach of having
every test try to clean up after any singletons it happens to activate seems
kind of doomed though.

[Scott Hess - ex-Googler, 2013-10-31 22:58:58]

I agree about the doomed part.

LGTM.

[tburkard, 2013-11-04 20:29:28]

lgtm

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2013-11-04 20:56:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mattm@chromium.org/55323002/100001

[commit-bot: I haz the power, 2013-11-05 01:05:00]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[mattm, 2013-11-05 02:08:47]

Committed patchset #2 manually as r232882 (presubmit successful).