BrowsingDataRemover: Work around HistoryService not handling delete_end as expected.

BrowsingDataRemover: Work around HistoryService not handling delete_end as expected.

BUG=145680

[rohitrao (ping after 24h), 2012-08-30 15:25:15]

http://codereview.chromium.org/10915010/diff/1/chrome/browser/browsing_data/b...
File chrome/browser/browsing_data/browsing_data_remover.cc (right):

http://codereview.chromium.org/10915010/diff/1/chrome/browser/browsing_data/b...
chrome/browser/browsing_data/browsing_data_remover.cc:249: delete_begin_,
base::Time(),
This hack only works if callers always pass Time::Now() for delete_end_.  Is
that assumption true?  Does anyone ever create a BrowsingDataRemover object for
an arbitrary time range in the past?

Explicitly checking for Time::Max() would remove this assumption, but then this
CL would be blocked on changing all callers to pass Time::Max() instead of
Time::Now().

[marja, 2012-08-30 15:28:44]

http://codereview.chromium.org/10915010/diff/1/chrome/browser/browsing_data/b...
File chrome/browser/browsing_data/browsing_data_remover.cc (right):

http://codereview.chromium.org/10915010/diff/1/chrome/browser/browsing_data/b...
chrome/browser/browsing_data/browsing_data_remover.cc:249: delete_begin_,
base::Time(),
On 2012/08/30 15:25:16, rohitrao wrote:
> This hack only works if callers always pass Time::Now() for delete_end_.  Is
> that assumption true?  Does anyone ever create a BrowsingDataRemover object
for
> an arbitrary time range in the past?
> 
> Explicitly checking for Time::Max() would remove this assumption, but then
this
> CL would be blocked on changing all callers to pass Time::Max() instead of
> Time::Now().

Via the extension API it's possible to invoke BrowsingDataRemover with arbitrary
delete_end_s. So this hack will break them. But if the history service is not
anyway obeying delete_end_...

Yep, Time::Max() is not yet in use, and to make this patch mergeable to earlier
branches, we can't rely on it.

[marja, 2012-08-31 09:56:17]

mkwst & sky, could you review this workaround? Thanks!

[sky, 2012-08-31 15:50:06]

http://codereview.chromium.org/10915010/diff/4002/chrome/browser/browsing_dat...
File chrome/browser/browsing_data/browsing_data_remover.cc (right):

http://codereview.chromium.org/10915010/diff/4002/chrome/browser/browsing_dat...
chrome/browser/browsing_data/browsing_data_remover.cc:247: // delete_begin_).
TODO(sky, rohitrao): Remove this workaround when the
This worries me in so far as you're assuming delete_end_ is always going to be
nowish. If that's the case, can we explicitly remove delete_end_ to make that
clear?

[rohitrao (ping after 24h), 2012-08-31 15:53:35]

http://codereview.chromium.org/10915010/diff/4002/chrome/browser/browsing_dat...
File chrome/browser/browsing_data/browsing_data_remover.cc (right):

http://codereview.chromium.org/10915010/diff/4002/chrome/browser/browsing_dat...
chrome/browser/browsing_data/browsing_data_remover.cc:247: // delete_begin_).
TODO(sky, rohitrao): Remove this workaround when the
On 2012/08/31 15:50:06, sky wrote:
> This worries me in so far as you're assuming delete_end_ is always going to be
> nowish. If that's the case, can we explicitly remove delete_end_ to make that
> clear?

Per an earlier comment, the Extensions API allows extensions to clear arbitrary
time ranges, so this will break for any extension that does so.  That seems bad.

Is there any way to test for a "nowish" Time?