Remove dependency on visitedlink from history

Remove dependency on Profile from HistoryService

Introduce an abstract interface history::VisitDelegate to abstract
visitedlink::VisitedLinkMaster from visitedlink component that has
a dependency on //content and thus cannot be used on iOS.

Provides an implementation of history::VisitDelegate that forward
to visitedlink::VisitedLinkMaster and inject it into HistoryService
via the HistoryServiceFactory.

Remove dependency on visitedlink component from HistoryService and
HistoryBackend.

Fix file that dependended on the forward-declaration of Profile
from history_service.h

Cleanup //chrome/browser/history/DEPS.

Fix typos and some style issues.

BUG=453790
Committed: https://crrev.com/d4d9fe44d9fcfa7a3c00fe8cdc9569a8690c28a7
Cr-Commit-Position: refs/heads/master@{#316836}

[droger, 2015-02-02 09:39:21]

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/conte...
File chrome/browser/history/content_visit_delegate.cc (right):

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/conte...
chrome/browser/history/content_visit_delegate.cc:11: #include
"components/visitedlink/browser/visitedlink_master.h"
#include "base/logging.h"
#include "base/memory/ref_counted.h"
#include "url/gurl.h"

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/conte...
File chrome/browser/history/content_visit_delegate.h (right):

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/conte...
chrome/browser/history/content_visit_delegate.h:9: 
#include "base/macros.h"

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/conte...
chrome/browser/history/content_visit_delegate.h:43: HistoryService*
history_service_;  // weak
Nit: Weak. (Capital + dot).

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/histo...
File chrome/browser/history/history_service.cc (right):

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_service.cc:417: // and will wrap around when we
subtract one from a 0 size.
This comment seems outdated.
I am also not convinced that the new code is equivalent to the old one:
add_page_args.url was always added, but now it's only added if the redirects are
empty.

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/histo...
File chrome/browser/history/history_service.h (right):

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_service.h:90:
HistoryService(history::HistoryClient* history_client,
Do we need a comment here about null arguments?
Do we expect a null |history_client|? If not, should we DCHECK that?
What about |visit_delegate|? It is null at least in a test.

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_service.h:116: // in-memory URL database. The
returned pointer MAY BE null if the in-memory
Nit: maybe this should stay in all caps? not sure.

[sdefresne, 2015-02-04 18:01:54]

Thank you for the review, please take another look.

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/conte...
File chrome/browser/history/content_visit_delegate.cc (right):

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/conte...
chrome/browser/history/content_visit_delegate.cc:11: #include
"components/visitedlink/browser/visitedlink_master.h"
On 2015/02/02 at 09:39:20, droger wrote:
> #include "base/logging.h"
> #include "base/memory/ref_counted.h"
> #include "url/gurl.h"

Done, thank you.

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/conte...
File chrome/browser/history/content_visit_delegate.h (right):

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/conte...
chrome/browser/history/content_visit_delegate.h:9: 
On 2015/02/02 at 09:39:20, droger wrote:
> #include "base/macros.h"

Done.

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/conte...
chrome/browser/history/content_visit_delegate.h:43: HistoryService*
history_service_;  // weak
On 2015/02/02 at 09:39:21, droger wrote:
> Nit: Weak. (Capital + dot).

Done.

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/histo...
File chrome/browser/history/history_service.cc (right):

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_service.cc:417: // and will wrap around when we
subtract one from a 0 size.
On 2015/02/02 at 09:39:21, droger wrote:
> This comment seems outdated.
> I am also not convinced that the new code is equivalent to the old one:
> add_page_args.url was always added, but now it's only added if the redirects
are empty.

The DCHECK() verify that the last element of add_page_args.redirect is
add_page_args.url. The old code was first sending add_page_args.url and then
every elements of add_page_args.redirect except the last when the array was not
empty. The new code always sends all the element of the array, so the same
elements will be sent, just in a different order (and the order is not important
in the delegate treat the array as a set).

Removed the comment.

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/histo...
File chrome/browser/history/history_service.h (right):

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_service.h:90:
HistoryService(history::HistoryClient* history_client,
On 2015/02/02 at 09:39:21, droger wrote:
> Do we need a comment here about null arguments?
> Do we expect a null |history_client|? If not, should we DCHECK that?
> What about |visit_delegate|? It is null at least in a test.

Done.

https://codereview.chromium.org/872313005/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_service.h:116: // in-memory URL database. The
returned pointer MAY BE null if the in-memory
On 2015/02/02 at 09:39:21, droger wrote:
> Nit: maybe this should stay in all caps? not sure.

Done.

[droger, 2015-02-05 10:12:59]

Thanks, LGTM.

[sdefresne, 2015-02-05 10:17:17]

New patchsets have been uploaded after l-g-t-m from droger@chromium.org

[sdefresne, 2015-02-05 10:25:00]

sdefresne@chromium.org changed reviewers:
+ jcivelli@chromium.org, shess@chromium.org, zea@chromium.org

[sdefresne, 2015-02-05 10:25:00]

shess: OWNERS of //chrome/browser/safe_browsing
jcivelli: OWNERS of //chrome/test/base
zea: OWNERS of //chrome/browser/sync

[Scott Hess - ex-Googler, 2015-02-05 15:00:47]

safe-browsing lgtm

[Nicolas Zea, 2015-02-05 18:50:20]

sync lgtm

[sdefresne, 2015-02-06 16:49:24]

New patchsets have been uploaded after l-g-t-m from
shess@chromium.org,zea@chromium.org

[sdefresne, 2015-02-09 08:58:35]

jcivelli: ping for OWNERS of //chrome/test/base/testing_profile.h

[Jay Civelli, 2015-02-09 17:12:09]

LGTM for chrome/test

[sdefresne, 2015-02-16 14:37:56]

New patchsets have been uploaded after l-g-t-m from jcivelli@chromium.org

[sdefresne, 2015-02-16 14:39:05]

sdefresne@chromium.org changed reviewers:
+ pkasting@chromium.org

[sdefresne, 2015-02-16 14:39:06]

+pkasting for OWNERS of chrome/browser/autocomplete

[Peter Kasting, 2015-02-16 20:03:27]

LGTM

[sdefresne, 2015-02-17 10:12:25]

New patchsets have been uploaded after l-g-t-m from pkasting@chromium.org

[sdefresne, 2015-02-18 15:18:41]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2015-02-18 15:19:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/872313005/120001

[commit-bot: I haz the power, 2015-02-18 16:18:57]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-02-18 16:20:01]

Patchset 7 (id:??) landed as
https://crrev.com/d4d9fe44d9fcfa7a3c00fe8cdc9569a8690c28a7
Cr-Commit-Position: refs/heads/master@{#316836}