Reporting: Plumb from //chrome/browser/browsing_data.

Reporting: Plumb from //chrome/browser/browsing_data.

Reporting is a spec for delivering out-of-band reports from various
other parts of the browser. See http://wicg.github.io/reporting/ for
the spec, or https://goo.gl/pygX5I for details of the planned
implementation in Chromium.

This CL plumbs from Chromium's browsing data removal code to Reporting
(and also makes a few tweaks to Reporting's browsing data removal).

BUG=704259
Review-Url: https://codereview.chromium.org/2829683004
Cr-Commit-Position: refs/heads/master@{#469474}
Committed: https://chromium.googlesource.com/chromium/src/+/aeb1abc9b5177c2817b21fe3a366fc0f23450607

[Julia Tuttle, 2017-04-19 18:59:15]

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

[commit-bot: I haz the power, 2017-04-19 18:59:54]

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

[Julia Tuttle, 2017-04-19 19:17:18]

Description was changed from

==========
Reporting: Plumbing from //chrome/browser/browsing_data.

Reporting is a spec for delivering out-of-band reports from various
other parts of the browser. See http://wicg.github.io/reporting/ for
the spec, or https://goo.gl/pygX5I for details of the planned
implementation in Chromium.

This CL plumbs from Chromium's browsing data removal code to Reporting
(and also makes a few tweaks to Reporting's browsing data removal).

BUG=704259
==========

to

==========
Reporting: Plumb from //chrome/browser/browsing_data.

Reporting is a spec for delivering out-of-band reports from various
other parts of the browser. See http://wicg.github.io/reporting/ for
the spec, or https://goo.gl/pygX5I for details of the planned
implementation in Chromium.

This CL plumbs from Chromium's browsing data removal code to Reporting
(and also makes a few tweaks to Reporting's browsing data removal).

BUG=704259
==========

[Julia Tuttle, 2017-04-19 19:17:59]

juliatuttle@chromium.org changed reviewers:
+ shivanisha@chromium.org

[Julia Tuttle, 2017-04-19 19:17:59]

PTAL, shivanisha.

[commit-bot: I haz the power, 2017-04-19 19:57:33]

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

[commit-bot: I haz the power, 2017-04-19 19:57:34]

Dry run: This issue passed the CQ dry run.

[shivanisha, 2017-04-20 18:21:52]

https://codereview.chromium.org/2829683004/diff/1/chrome/browser/browsing_dat...
File chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
(right):

https://codereview.chromium.org/2829683004/diff/1/chrome/browser/browsing_dat...
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc:1013:
(BrowsingDataRemover::DATA_TYPE_COOKIES | DATA_TYPE_HISTORY)) {
if ((remove_mask & DATA_TYPE_HISTORY) || (remove_mask &
BrowsingDataRemover::DATA_TYPE_COOKIES))

https://codereview.chromium.org/2829683004/diff/1/chrome/browser/browsing_dat...
File
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
(right):

https://codereview.chromium.org/2829683004/diff/1/chrome/browser/browsing_dat...
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc:2235:
// Disabled because history is not iterable yet.
Is iterable history something that is being worked on? Can a crbug/todo
reference be included here?

https://codereview.chromium.org/2829683004/diff/1/net/reporting/reporting_bro...
File net/reporting/reporting_browsing_data_remover.cc (right):

https://codereview.chromium.org/2829683004/diff/1/net/reporting/reporting_bro...
net/reporting/reporting_browsing_data_remover.cc:33: 
ReportingCache* cache = context_->cache(); instead of invoking cache() every
time and then cache() can also be removed.

[Julia Tuttle, 2017-04-21 13:34:52]

juliatuttle@chromium.org changed reviewers:
+ bauerb@chromium.org

[Julia Tuttle, 2017-04-21 13:34:53]

PTAL, shivanisha.

Also, bauerb, can you take a look at //chrome/browser/browsing_data? If not, let
me know and I'll find someone else.

https://codereview.chromium.org/2829683004/diff/1/chrome/browser/browsing_dat...
File chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
(right):

https://codereview.chromium.org/2829683004/diff/1/chrome/browser/browsing_dat...
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc:1013:
(BrowsingDataRemover::DATA_TYPE_COOKIES | DATA_TYPE_HISTORY)) {
On 2017/04/20 18:21:52, shivanisha wrote:
> if ((remove_mask & DATA_TYPE_HISTORY) || (remove_mask &
> BrowsingDataRemover::DATA_TYPE_COOKIES))
> 

Done.

https://codereview.chromium.org/2829683004/diff/1/chrome/browser/browsing_dat...
File
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
(right):

https://codereview.chromium.org/2829683004/diff/1/chrome/browser/browsing_dat...
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc:2235:
// Disabled because history is not iterable yet.
On 2017/04/20 18:21:52, shivanisha wrote:
> Is iterable history something that is being worked on? Can a crbug/todo
> reference be included here?

I have no idea -- I just know that the similar Domain Reliability test is also
disabled, and that this test fails a CHECK if I enable it.

I'm assuming whoever fixes it will re-enable all the relevant tests.

https://codereview.chromium.org/2829683004/diff/1/net/reporting/reporting_bro...
File net/reporting/reporting_browsing_data_remover.cc (right):

https://codereview.chromium.org/2829683004/diff/1/net/reporting/reporting_bro...
net/reporting/reporting_browsing_data_remover.cc:33: 
On 2017/04/20 18:21:52, shivanisha wrote:
> ReportingCache* cache = context_->cache(); instead of invoking cache() every
> time and then cache() can also be removed.

Done.

[Julia Tuttle, 2017-04-21 13:35:00]

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

[commit-bot: I haz the power, 2017-04-21 13:35:07]

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

[shivanisha, 2017-04-21 13:52:19]

On 2017/04/21 at 13:34:53, juliatuttle wrote:
> PTAL, shivanisha.
> 
> Also, bauerb, can you take a look at //chrome/browser/browsing_data? If not,
let me know and I'll find someone else.
> 
>
https://codereview.chromium.org/2829683004/diff/1/chrome/browser/browsing_dat...
> File chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
(right):
> 
>
https://codereview.chromium.org/2829683004/diff/1/chrome/browser/browsing_dat...
> chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc:1013:
(BrowsingDataRemover::DATA_TYPE_COOKIES | DATA_TYPE_HISTORY)) {
> On 2017/04/20 18:21:52, shivanisha wrote:
> > if ((remove_mask & DATA_TYPE_HISTORY) || (remove_mask &
> > BrowsingDataRemover::DATA_TYPE_COOKIES))
> > 
> 
> Done.
> 
>
https://codereview.chromium.org/2829683004/diff/1/chrome/browser/browsing_dat...
> File
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
(right):
> 
>
https://codereview.chromium.org/2829683004/diff/1/chrome/browser/browsing_dat...
>
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc:2235:
// Disabled because history is not iterable yet.
> On 2017/04/20 18:21:52, shivanisha wrote:
> > Is iterable history something that is being worked on? Can a crbug/todo
> > reference be included here?
> 
> I have no idea -- I just know that the similar Domain Reliability test is also
disabled, and that this test fails a CHECK if I enable it.
> 
> I'm assuming whoever fixes it will re-enable all the relevant tests.
> 
>
https://codereview.chromium.org/2829683004/diff/1/net/reporting/reporting_bro...
> File net/reporting/reporting_browsing_data_remover.cc (right):
> 
>
https://codereview.chromium.org/2829683004/diff/1/net/reporting/reporting_bro...
> net/reporting/reporting_browsing_data_remover.cc:33: 
> On 2017/04/20 18:21:52, shivanisha wrote:
> > ReportingCache* cache = context_->cache(); instead of invoking cache() every
> > time and then cache() can also be removed.
> 
> Done.

lgtm

[Julia Tuttle, 2017-04-21 13:53:57]

Thanks, shivanisha!

[commit-bot: I haz the power, 2017-04-21 14:30:43]

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

[commit-bot: I haz the power, 2017-04-21 14:30:44]

Dry run: This issue passed the CQ dry run.

[Bernhard Bauer, 2017-04-21 16:49:27]

https://codereview.chromium.org/2829683004/diff/20001/chrome/browser/browsing...
File chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
(right):

https://codereview.chromium.org/2829683004/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc:244:
base::Callback<bool(const GURL&)> origin_filter) {
Nit: Callbacks are usually passed by const-ref.

https://codereview.chromium.org/2829683004/diff/20001/chrome/browser/browsing...
File
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
(right):

https://codereview.chromium.org/2829683004/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc:896:
content::BrowserThread::IO, FROM_HERE,
I'm not sure this thread jumping is necessary -- these BrowserThreads all run on
the same physical thread due to the TestingBrowserThreadBundle, and a
DCHECK_CURRENTLY_ON(IO) in the code under test will also succeed, because those
look at physical thread (or rather message loops).

[Julia Tuttle, 2017-04-21 18:25:11]

PTAL, bauerb.

https://codereview.chromium.org/2829683004/diff/20001/chrome/browser/browsing...
File chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
(right):

https://codereview.chromium.org/2829683004/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc:244:
base::Callback<bool(const GURL&)> origin_filter) {
On 2017/04/21 16:49:26, Bernhard Bauer wrote:
> Nit: Callbacks are usually passed by const-ref.

Oops!

https://codereview.chromium.org/2829683004/diff/20001/chrome/browser/browsing...
File
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
(right):

https://codereview.chromium.org/2829683004/diff/20001/chrome/browser/browsing...
chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc:896:
content::BrowserThread::IO, FROM_HERE,
On 2017/04/21 16:49:26, Bernhard Bauer wrote:
> I'm not sure this thread jumping is necessary -- these BrowserThreads all run
on
> the same physical thread due to the TestingBrowserThreadBundle, and a
> DCHECK_CURRENTLY_ON(IO) in the code under test will also succeed, because
those
> look at physical thread (or rather message loops).

I'll remove it and add a DCHECK.

[Bernhard Bauer, 2017-04-24 11:48:07]

lgtm

[Julia Tuttle, 2017-04-25 21:11:28]

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

[commit-bot: I haz the power, 2017-04-25 21:12:24]

Dry run: 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-04-25 21:34:20]

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

[commit-bot: I haz the power, 2017-04-25 21:34:21]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Julia Tuttle, 2017-04-27 16:48:11]

The CQ bit was checked by juliatuttle@chromium.org

[Julia Tuttle, 2017-04-27 16:48:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
shivanisha@chromium.org, bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/2829683004/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2017-04-27 16:48:39]

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-04-27 17:00:34]

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

[commit-bot: I haz the power, 2017-04-27 17:00:35]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Julia Tuttle, 2017-04-28 14:50:49]

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

[commit-bot: I haz the power, 2017-04-28 14:51:29]

Dry run: 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-04-28 15:03:21]

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

[commit-bot: I haz the power, 2017-04-28 15:03:22]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Julia Tuttle, 2017-04-28 15:59:29]

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

[commit-bot: I haz the power, 2017-04-28 16:00:02]

Dry run: 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-04-28 16:11:24]

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

[commit-bot: I haz the power, 2017-04-28 16:11:25]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Julia Tuttle, 2017-04-28 19:11:34]

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

[commit-bot: I haz the power, 2017-04-28 19:12:19]

Dry run: 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-04-28 20:42:53]

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

[commit-bot: I haz the power, 2017-04-28 20:42:54]

Dry run: This issue passed the CQ dry run.

[Julia Tuttle, 2017-05-04 19:50:22]

The CQ bit was checked by juliatuttle@chromium.org

[Julia Tuttle, 2017-05-04 19:50:22]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
shivanisha@chromium.org
Link to the patchset: https://codereview.chromium.org/2829683004/#ps120001
(title: "Fix.")

[commit-bot: I haz the power, 2017-05-04 19:50:52]

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-05-04 21:14:42]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1493927422308560,
"parent_rev": "0adabba19edd06f92b865d1ca35981ddadb955c9", "commit_rev":
"aeb1abc9b5177c2817b21fe3a366fc0f23450607"}

[commit-bot: I haz the power, 2017-05-04 21:14:52]

Description was changed from

==========
Reporting: Plumb from //chrome/browser/browsing_data.

Reporting is a spec for delivering out-of-band reports from various
other parts of the browser. See http://wicg.github.io/reporting/ for
the spec, or https://goo.gl/pygX5I for details of the planned
implementation in Chromium.

This CL plumbs from Chromium's browsing data removal code to Reporting
(and also makes a few tweaks to Reporting's browsing data removal).

BUG=704259
==========

to

==========
Reporting: Plumb from //chrome/browser/browsing_data.

Reporting is a spec for delivering out-of-band reports from various
other parts of the browser. See http://wicg.github.io/reporting/ for
the spec, or https://goo.gl/pygX5I for details of the planned
implementation in Chromium.

This CL plumbs from Chromium's browsing data removal code to Reporting
(and also makes a few tweaks to Reporting's browsing data removal).

BUG=704259

Review-Url: https://codereview.chromium.org/2829683004
Cr-Commit-Position: refs/heads/master@{#469474}
Committed:
https://chromium.googlesource.com/chromium/src/+/aeb1abc9b5177c2817b21fe3a366...
==========

[commit-bot: I haz the power, 2017-05-04 21:14:54]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/aeb1abc9b5177c2817b21fe3a366...