Fixing some more instances where I put in HistoryServiceFactory::GetForProfile which will force cre…

Fixing some more instances where I put in HistoryServiceFactory::GetForProfile which will force creation of the history. Replacing those instantces where we should not force creation and should instead just check if the history has already been created.

BUG=97804
TEST=unittests

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

[rpetterson, 2012-06-21 00:57:25]

Scott, I didn't TBR you on this one because I was wondering if you could take a
quick look to see if these are legitimate cases where the history service could
be null or if we're just checking for null in the case of unittests. Thanks!

[sky, 2012-06-21 17:14:31]

You'll need to ask Brett about history_tab_helper, someone else for the
downloads changes (not sure who) and michaelbai for the android changes. Or
better yet, fork those into a separate patch.

http://codereview.chromium.org/10605003/diff/1/chrome/browser/bookmarks/bookm...
File chrome/browser/bookmarks/bookmark_index.cc (left):

http://codereview.chromium.org/10605003/diff/1/chrome/browser/bookmarks/bookm...
chrome/browser/bookmarks/bookmark_index.cc:115:
HistoryServiceFactory::GetForProfile(profile_,
Keep this as it was.

http://codereview.chromium.org/10605003/diff/1/chrome/browser/bookmarks/bookm...
File chrome/browser/bookmarks/bookmark_model.cc (left):

http://codereview.chromium.org/10605003/diff/1/chrome/browser/bookmarks/bookm...
chrome/browser/bookmarks/bookmark_model.cc:683:
HistoryServiceFactory::GetForProfile(profile_,
Keep this as it was.

http://codereview.chromium.org/10605003/diff/1/chrome/browser/bookmarks/bookm...
File chrome/browser/bookmarks/bookmark_storage.cc (left):

http://codereview.chromium.org/10605003/diff/1/chrome/browser/bookmarks/bookm...
chrome/browser/bookmarks/bookmark_storage.cc:147: HistoryService* history =
Keep this as it was.

http://codereview.chromium.org/10605003/diff/1/chrome/browser/history/android...
File chrome/browser/history/android/android_history_provider_service.cc (right):

http://codereview.chromium.org/10605003/diff/1/chrome/browser/history/android...
chrome/browser/history/android/android_history_provider_service.cc:31:
HistoryService* hs = HistoryServiceFactory::GetForProfileIfExists(
I think you should leave all of these as they were.

[Randy Smith (Not in Mondays), 2012-06-21 19:45:37]

Given that the DownloadService has an explicit dependency on the history
service, and the download service is initialized whenever a DownloadManager is
allocated, we can assume that if we're in DownloadHistory (entered from the
ChromeDownloadManagerDelegate, which is created as part of DownloadService
initialization) that if the history isn't around, it's never going to be around.
 download_history LGTM.

(There's a question in my mind as to whether we'll ever get to that code if the
history isn't around--I think we bail for incognito earlier.   But I don't think
that's relevant for this CL, as it does the right thing if there's no history
service.)

[rpetterson, 2012-06-21 23:45:02]

sky@ -- reverted the files you suggested

brettw@ -- please take a look at the history_tab_helper.cc and let me know if I
have changed it to the correct behavior (that is, I changed it to *not* force
creation of a HistoryService if one did not already exist)

[sky, 2012-06-22 00:15:48]

LGTM

[brettw, 2012-06-22 17:38:44]

rpetterson: you'll have to check that the does the right force creating behavior
in the debugger. I don't remember off the top of my head how this works.

[rpetterson, 2012-07-17 23:37:24]

Fixing some more instances where I put in HistoryServiceFactory::GetForProfile
which will force creation of the history. Replacing those instances where we
should not force creation and should instead just check if the history has
already been created. 

BUG=97804
TEST=unittests

[rpetterson, 2012-07-17 23:38:21]

I've removed history_tab_helper.cc. Based on the previous behavior, it will
force creation of the HistoryService if that's allowed. The check for null
appears to be for unittests.

[commit-bot: I haz the power, 2012-07-17 23:44:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlp@chromium.org/10605003/14001

[commit-bot: I haz the power, 2012-07-18 16:44:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlp@chromium.org/10605003/27001

[commit-bot: I haz the power, 2012-07-18 18:01:08]

Try job failure for 10605003-27001 (retry) on mac_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-07-26 17:51:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlp@chromium.org/10605003/27001

[commit-bot: I haz the power, 2012-07-26 19:13:07]

Try job failure for 10605003-27001 (retry) on linux_clang for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[commit-bot: I haz the power, 2012-07-26 19:16:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlp@chromium.org/10605003/27001

[commit-bot: I haz the power, 2012-07-26 21:55:27]

Change committed as 148626