Extract MockBrowsingDataRemover to separate file to make it possible to use it

Extract MockBrowsingDataRemover to separate file to make it possible to use it
from other tests.

BUG=681523
Review-Url: https://codereview.chromium.org/2720723002
Cr-Commit-Position: refs/heads/master@{#453556}
Committed: https://chromium.googlesource.com/chromium/src/+/525fa6787336cb9d251ca2c264755bc0625f9b74

[dullweber, 2017-02-27 14:00:17]

The CQ bit was checked by dullweber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-27 14:00:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dullweber, 2017-02-27 14:03:09]

dullweber@chromium.org changed reviewers:
+ msramek@chromium.org

[dullweber, 2017-02-27 14:03:10]

Hi Martin, 
I would like to use this mock in order to test the CBD dialog from Java. Could
your TODO about moving BrowsingDataRemoverImpl to content be a problem?

[msramek, 2017-02-27 14:22:41]

https://codereview.chromium.org/2720723002/diff/1/chrome/browser/browsing_dat...
File chrome/browser/browsing_data/browsing_data_remover_mock.h (right):

https://codereview.chromium.org/2720723002/diff/1/chrome/browser/browsing_dat...
chrome/browser/browsing_data/browsing_data_remover_mock.h:5: #ifndef
CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_REMOVER_MOCK_H_
The file should be called the same way as the class: mock_browsing_data_remover.

https://codereview.chromium.org/2720723002/diff/1/chrome/browser/browsing_dat...
chrome/browser/browsing_data/browsing_data_remover_mock.h:16: class
MockBrowsingDataRemover : public BrowsingDataRemoverImpl {
We should inherit from BrowsingDataRemover (the pure abstract class), since that
one will live in content/public/ and thus will be visible from chrome/. In that
case we can also resolve the TODO.

[commit-bot: I haz the power, 2017-02-27 14:34:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-27 14:34:53]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[dullweber, 2017-02-27 15:08:58]

The CQ bit was checked by dullweber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-27 15:09:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dullweber, 2017-02-27 16:00:07]

The CQ bit was checked by dullweber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-27 16:00:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dullweber, 2017-02-27 16:04:11]

https://codereview.chromium.org/2720723002/diff/1/chrome/browser/browsing_dat...
File chrome/browser/browsing_data/browsing_data_remover_mock.h (right):

https://codereview.chromium.org/2720723002/diff/1/chrome/browser/browsing_dat...
chrome/browser/browsing_data/browsing_data_remover_mock.h:5: #ifndef
CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_REMOVER_MOCK_H_
On 2017/02/27 14:22:41, msramek wrote:
> The file should be called the same way as the class:
mock_browsing_data_remover.

Done.

https://codereview.chromium.org/2720723002/diff/1/chrome/browser/browsing_dat...
chrome/browser/browsing_data/browsing_data_remover_mock.h:16: class
MockBrowsingDataRemover : public BrowsingDataRemoverImpl {
On 2017/02/27 14:22:41, msramek wrote:
> We should inherit from BrowsingDataRemover (the pure abstract class), since
that
> one will live in content/public/ and thus will be visible from chrome/. In
that
> case we can also resolve the TODO.

I changed this class to inherit from the interface.

[msramek, 2017-02-27 16:31:17]

Thanks! LGTM % a few more comments.

https://codereview.chromium.org/2720723002/diff/40001/chrome/browser/browsing...
File chrome/browser/browsing_data/mock_browsing_data_remover.cc (right):

https://codereview.chromium.org/2720723002/diff/40001/chrome/browser/browsing...
chrome/browser/browsing_data/mock_browsing_data_remover.cc:89: void
MockBrowsingDataRemover::RemoveObserver(Observer* observer) {}
style: empty lines between methods, here and below.

https://codereview.chromium.org/2720723002/diff/40001/chrome/browser/browsing...
File chrome/browser/browsing_data/mock_browsing_data_remover.h (right):

https://codereview.chromium.org/2720723002/diff/40001/chrome/browser/browsing...
chrome/browser/browsing_data/mock_browsing_data_remover.h:13: #include
"testing/gtest/include/gtest/gtest.h"
Is this needed?

https://codereview.chromium.org/2720723002/diff/40001/chrome/browser/browsing...
chrome/browser/browsing_data/mock_browsing_data_remover.h:15: // A
BrowsingDataRemover that only records calls.
nit: Please explain that some of the methods are NOTIMPLEMENTED() since they're
not needed right now. People who only look at the .h file when including it
could possibly be surprised by that.

https://codereview.chromium.org/2720723002/diff/40001/chrome/browser/browsing...
chrome/browser/browsing_data/mock_browsing_data_remover.h:23: void Shutdown()
override;
style: Document what inherits from what. Typically the format is:

// BrowsingDataRemover:
void Remove() override;
void RemoveAndReply() override;
...

// KeyedService:
void Shutdown() override;

Note that there are typically no empty lines between methods from one parent
class (so that it's clear that the comments applies to all of them).

Document any public methods which are not inherited (e.g.
VerifyAndClearExpectations).

(I know that this is was my code, but at that time it was just a helper class
inside a test, not a standalone class in its own file)

Please also reorder the .cc file correspondingly.

[commit-bot: I haz the power, 2017-02-27 17:07:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-27 17:07:47]

Dry run: This issue passed the CQ dry run.

[dullweber, 2017-02-28 08:40:19]

https://codereview.chromium.org/2720723002/diff/40001/chrome/browser/browsing...
File chrome/browser/browsing_data/mock_browsing_data_remover.cc (right):

https://codereview.chromium.org/2720723002/diff/40001/chrome/browser/browsing...
chrome/browser/browsing_data/mock_browsing_data_remover.cc:89: void
MockBrowsingDataRemover::RemoveObserver(Observer* observer) {}
On 2017/02/27 16:31:16, msramek (recovering from OOO) wrote:
> style: empty lines between methods, here and below.

Done.

https://codereview.chromium.org/2720723002/diff/40001/chrome/browser/browsing...
File chrome/browser/browsing_data/mock_browsing_data_remover.h (right):

https://codereview.chromium.org/2720723002/diff/40001/chrome/browser/browsing...
chrome/browser/browsing_data/mock_browsing_data_remover.h:13: #include
"testing/gtest/include/gtest/gtest.h"
On 2017/02/27 16:31:16, msramek (recovering from OOO) wrote:
> Is this needed?

only in the .cc for an EXPECT_EQ(), fixed

https://codereview.chromium.org/2720723002/diff/40001/chrome/browser/browsing...
chrome/browser/browsing_data/mock_browsing_data_remover.h:15: // A
BrowsingDataRemover that only records calls.
On 2017/02/27 16:31:16, msramek (recovering from OOO) wrote:
> nit: Please explain that some of the methods are NOTIMPLEMENTED() since
they're
> not needed right now. People who only look at the .h file when including it
> could possibly be surprised by that.

Done.

https://codereview.chromium.org/2720723002/diff/40001/chrome/browser/browsing...
chrome/browser/browsing_data/mock_browsing_data_remover.h:23: void Shutdown()
override;
On 2017/02/27 16:31:16, msramek (recovering from OOO) wrote:
> style: Document what inherits from what. Typically the format is:
> 
> // BrowsingDataRemover:
> void Remove() override;
> void RemoveAndReply() override;
> ...
> 
> // KeyedService:
> void Shutdown() override;
> 
> Note that there are typically no empty lines between methods from one parent
> class (so that it's clear that the comments applies to all of them).
> 
> Document any public methods which are not inherited (e.g.
> VerifyAndClearExpectations).
> 
> (I know that this is was my code, but at that time it was just a helper class
> inside a test, not a standalone class in its own file)
> 
> Please also reorder the .cc file correspondingly.

I added comments and removed empty lines.

[dullweber, 2017-02-28 08:40:25]

The CQ bit was checked by dullweber@chromium.org

[dullweber, 2017-02-28 08:40:26]

The patchset sent to the CQ was uploaded after l-g-t-m from msramek@chromium.org
Link to the patchset: https://codereview.chromium.org/2720723002/#ps60001
(title: "fix comments")

[commit-bot: I haz the power, 2017-02-28 08:40:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dullweber, 2017-02-28 08:41:11]

The CQ bit was unchecked by dullweber@chromium.org

[dullweber, 2017-02-28 08:45:31]

The CQ bit was checked by dullweber@chromium.org

[dullweber, 2017-02-28 08:45:31]

The patchset sent to the CQ was uploaded after l-g-t-m from msramek@chromium.org
Link to the patchset: https://codereview.chromium.org/2720723002/#ps80001
(title: "fix include")

[commit-bot: I haz the power, 2017-02-28 08:45:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-28 09:43:07]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1488271531304620,
"parent_rev": "d42421bec3f31371e904543c9e60b287dd3580ae", "commit_rev":
"525fa6787336cb9d251ca2c264755bc0625f9b74"}

[commit-bot: I haz the power, 2017-02-28 09:43:43]

Description was changed from

==========
Extract MockBrowsingDataRemover to separate file to make it possible to use it
from other tests.

BUG=681523
==========

to

==========
Extract MockBrowsingDataRemover to separate file to make it possible to use it
from other tests.

BUG=681523

Review-Url: https://codereview.chromium.org/2720723002
Cr-Commit-Position: refs/heads/master@{#453556}
Committed:
https://chromium.googlesource.com/chromium/src/+/525fa6787336cb9d251ca2c26475...
==========

[commit-bot: I haz the power, 2017-02-28 09:43:44]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/525fa6787336cb9d251ca2c26475...