Implement host-based deletion for the hostname resolution cache.

Implement host-based deletion for the hostname resolution cache.

To be used by BrowsingDataRemover when the user chooses to delete data
for a particular site.

This is implemented as a new method HostCache::ClearForHosts() as opposed
to extending the existing HostCache::clear(), as clear() has a lot
of callsites and thus it seems to be useful to keep this more specific
version.

On the other hand, the fact that there are many callsites of clear()
also means that in many cases it's acceptable to clear the entire cache
even if clearing less would be sufficient. In that sense, this CL might
be unnecessary and it might be acceptable for BrowsingDataRemover to also
clear the entire cache even if the user only needs to delete data for one
site.

TBR=achuith@chromium.org
BUG=589586
Committed: https://crrev.com/94f5656c8278b8a8080f465a97465799b9fd5f7e
Cr-Commit-Position: refs/heads/master@{#415664}

[msramek, 2016-08-31 09:58:18]

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

[commit-bot: I haz the power, 2016-08-31 09:58:38]

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, 2016-08-31 10:11:21]

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

[commit-bot: I haz the power, 2016-08-31 10:11:22]

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

[msramek, 2016-08-31 11:21:20]

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

[commit-bot: I haz the power, 2016-08-31 11:21:33]

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, 2016-08-31 13:36:52]

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

[commit-bot: I haz the power, 2016-08-31 13:36:53]

Dry run: This issue passed the CQ dry run.

[msramek, 2016-08-31 15:05:41]

msramek@chromium.org changed reviewers:
+ juliatuttle@chromium.org, mmenke@chromium.org

[msramek, 2016-08-31 15:05:41]

Hi Matt and Julia,

Please have a look! Another small change in the series of my CLs of selective
browsing data deletion.

First of all I would like to hear your insights on whether this is actually
needed at all - see my reasoning in the CL description.

Thanks,
Martin

[Julia Tuttle, 2016-08-31 15:08:51]

lgtm; it's not super complex and, while the host cache is cleared on network
transitions, I'd still rather not clear the whole thing when we don't need to.

[mmenke, 2016-08-31 15:24:37]

LGTM as well.  If this were any more complicated, I'd say clear() is good enough
for a feature I expect isn't used that often, but given how simple it is, seems
reasonable.

https://codereview.chromium.org/2298173002/diff/20001/net/dns/host_cache.cc
File net/dns/host_cache.cc (right):

https://codereview.chromium.org/2298173002/diff/20001/net/dns/host_cache.cc#n...
net/dns/host_cache.cc:214: RecordErase(ERASE_CLEAR, base::TimeTicks::Now(),
it->second);
Maybe just call base::TimeTicks::Now() once, outside of the loop?  Not sure how
expensive it is, and this is called rarely enough that it probably doesn't
really matter, but does seem like we don't need to keep on getting the time.

[msramek, 2016-08-31 16:00:20]

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

[commit-bot: I haz the power, 2016-08-31 16:00:44]

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

[msramek, 2016-08-31 16:02:55]

Thank you both!

https://codereview.chromium.org/2298173002/diff/20001/net/dns/host_cache.cc
File net/dns/host_cache.cc (right):

https://codereview.chromium.org/2298173002/diff/20001/net/dns/host_cache.cc#n...
net/dns/host_cache.cc:214: RecordErase(ERASE_CLEAR, base::TimeTicks::Now(),
it->second);
On 2016/08/31 15:24:37, mmenke wrote:
> Maybe just call base::TimeTicks::Now() once, outside of the loop?  Not sure
how
> expensive it is, and this is called rarely enough that it probably doesn't
> really matter, but does seem like we don't need to keep on getting the time.

Done. Good point - it's even better for consistency, as clear() also uses the
same timestamp for all entries.

[msramek, 2016-08-31 16:03:52]

Description was changed from

==========
Implement host-based deletion for the hostname resolution cache.

To be used by BrowsingDataRemover when the user chooses to delete data
for a particular site.

This is implemented as a new method HostCache::ClearForHosts() as opposed
to extending the existing HostCache::clear(), as clear() has a lot
of callsites and thus it seems to be useful to keep this more specific
version.

On the other hand, the fact that there are many callsites of clear()
also means that in many cases it's acceptable to clear the entire cache
even if clearing less would be sufficient. In that sense, this CL might
be unnecessary and it might be acceptable for BrowsingDataRemover to also
clear the entire cache even if the user only needs to delete data for one
site.

BUG=589586
==========

to

==========
Implement host-based deletion for the hostname resolution cache.

To be used by BrowsingDataRemover when the user chooses to delete data
for a particular site.

This is implemented as a new method HostCache::ClearForHosts() as opposed
to extending the existing HostCache::clear(), as clear() has a lot
of callsites and thus it seems to be useful to keep this more specific
version.

On the other hand, the fact that there are many callsites of clear()
also means that in many cases it's acceptable to clear the entire cache
even if clearing less would be sufficient. In that sense, this CL might
be unnecessary and it might be acceptable for BrowsingDataRemover to also
clear the entire cache even if the user only needs to delete data for one
site.

TBR=achuith@chromium.org
BUG=589586
==========

[msramek, 2016-08-31 16:04:20]

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

[msramek, 2016-08-31 16:04:21]

Adding +Achuith as TBR for the updated callsite in gaia_screen_handler.cc.

[msramek, 2016-08-31 16:04:39]

The CQ bit was unchecked by msramek@chromium.org

[msramek, 2016-08-31 16:04:46]

The CQ bit was checked by msramek@chromium.org

[msramek, 2016-08-31 16:04:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
juliatuttle@chromium.org, mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2298173002/#ps40001
(title: "Take TimeTicks::Now() out of the loop")

[commit-bot: I haz the power, 2016-08-31 16:05:46]

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, 2016-08-31 17:09:54]

Description was changed from

==========
Implement host-based deletion for the hostname resolution cache.

To be used by BrowsingDataRemover when the user chooses to delete data
for a particular site.

This is implemented as a new method HostCache::ClearForHosts() as opposed
to extending the existing HostCache::clear(), as clear() has a lot
of callsites and thus it seems to be useful to keep this more specific
version.

On the other hand, the fact that there are many callsites of clear()
also means that in many cases it's acceptable to clear the entire cache
even if clearing less would be sufficient. In that sense, this CL might
be unnecessary and it might be acceptable for BrowsingDataRemover to also
clear the entire cache even if the user only needs to delete data for one
site.

TBR=achuith@chromium.org
BUG=589586
==========

to

==========
Implement host-based deletion for the hostname resolution cache.

To be used by BrowsingDataRemover when the user chooses to delete data
for a particular site.

This is implemented as a new method HostCache::ClearForHosts() as opposed
to extending the existing HostCache::clear(), as clear() has a lot
of callsites and thus it seems to be useful to keep this more specific
version.

On the other hand, the fact that there are many callsites of clear()
also means that in many cases it's acceptable to clear the entire cache
even if clearing less would be sufficient. In that sense, this CL might
be unnecessary and it might be acceptable for BrowsingDataRemover to also
clear the entire cache even if the user only needs to delete data for one
site.

TBR=achuith@chromium.org
BUG=589586
==========

[commit-bot: I haz the power, 2016-08-31 17:09:56]

Committed patchset #3 (id:40001)

[achuithb, 2016-08-31 17:10:00]

gaia_screen_handler.cc lgtm

[commit-bot: I haz the power, 2016-08-31 17:12:33]

Description was changed from

==========
Implement host-based deletion for the hostname resolution cache.

To be used by BrowsingDataRemover when the user chooses to delete data
for a particular site.

This is implemented as a new method HostCache::ClearForHosts() as opposed
to extending the existing HostCache::clear(), as clear() has a lot
of callsites and thus it seems to be useful to keep this more specific
version.

On the other hand, the fact that there are many callsites of clear()
also means that in many cases it's acceptable to clear the entire cache
even if clearing less would be sufficient. In that sense, this CL might
be unnecessary and it might be acceptable for BrowsingDataRemover to also
clear the entire cache even if the user only needs to delete data for one
site.

TBR=achuith@chromium.org
BUG=589586
==========

to

==========
Implement host-based deletion for the hostname resolution cache.

To be used by BrowsingDataRemover when the user chooses to delete data
for a particular site.

This is implemented as a new method HostCache::ClearForHosts() as opposed
to extending the existing HostCache::clear(), as clear() has a lot
of callsites and thus it seems to be useful to keep this more specific
version.

On the other hand, the fact that there are many callsites of clear()
also means that in many cases it's acceptable to clear the entire cache
even if clearing less would be sufficient. In that sense, this CL might
be unnecessary and it might be acceptable for BrowsingDataRemover to also
clear the entire cache even if the user only needs to delete data for one
site.

TBR=achuith@chromium.org
BUG=589586

Committed: https://crrev.com/94f5656c8278b8a8080f465a97465799b9fd5f7e
Cr-Commit-Position: refs/heads/master@{#415664}
==========

[commit-bot: I haz the power, 2016-08-31 17:12:34]

Patchset 3 (id:??) landed as
https://crrev.com/94f5656c8278b8a8080f465a97465799b9fd5f7e
Cr-Commit-Position: refs/heads/master@{#415664}