Pass the time parameter to PrefServiceBridge.clearBrowsingData() explicitly.

Pass the time parameter to PrefServiceBridge.clearBrowsingData() explicitly.

The browsing data deletion on Android relied on the prefs::kDeleteTimePeriod
to determine the time period selection, similarly as we do on Desktop.

However, this is incorrect with respect to callsites in ChromeApplication and
ConfirmImportSyncDataDialog, that rely on the time period used being EVERYTHING.

Pass the time time period parameter explicitly, so that different callsites
can specify which time period to use.

BUG=595876
Committed: https://crrev.com/5f5b82dc246c721ceb634983719494519694e57b
Cr-Commit-Position: refs/heads/master@{#382317}

[msramek, 2016-03-21 10:58:19]

msramek@chromium.org changed reviewers:
+ newt@chromium.org, peconn@chromium.org

[msramek, 2016-03-21 10:58:19]

Hi guys,

please have a look!

Please see https://bugs.chromium.org/p/chromium/issues/detail?id=595876#c17 for
more details.

Thanks,
Martin

[newt (away), 2016-03-21 17:01:52]

lgtm. I think this is a good change independent of the bug it fixes. Thanks!

[msramek, 2016-03-21 17:16:56]

Thanks, Newt!

I'll land the fix now to merge it ASAP, and will think of ways to test it when I
have a spare cycle sometime this week.

My current plan is to make pref_service_bridge.cc return a TestingProfile in
GetOriginalProfile(), and then expose one of the browsing data backends (adding
/ deleting / counting entries) to the Java side to test that entries in the
correct time interval are deleted.

[msramek, 2016-03-21 17:17:10]

The CQ bit was checked by msramek@chromium.org

[commit-bot: I haz the power, 2016-03-21 17:17:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1823593002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1823593002/1

[newt (away), 2016-03-21 17:25:44]

On 2016/03/21 17:16:56, msramek wrote:
> Thanks, Newt!
> 
> I'll land the fix now to merge it ASAP, and will think of ways to test it when
I
> have a spare cycle sometime this week.
> 
> My current plan is to make pref_service_bridge.cc return a TestingProfile in
> GetOriginalProfile(), and then expose one of the browsing data backends
(adding
> / deleting / counting entries) to the Java side to test that entries in the
> correct time interval are deleted.

Sounds like a good plan.

While you're at it, could you move the code related to clearing browsing data
from PrefServiceBridge into a ClearBrowsingDataBridge (or
BrowsingDataRemovedBridge, or whatever name makes sense)? That's a big enough
and complex enough chunk of code that it probably deserves its own home. It
might make testing clearer as well.

[commit-bot: I haz the power, 2016-03-21 17:27:32]

Committed patchset #1 (id:1)

[msramek, 2016-03-21 17:29:56]

On 2016/03/21 17:25:44, newt wrote:
> On 2016/03/21 17:16:56, msramek wrote:
> > Thanks, Newt!
> > 
> > I'll land the fix now to merge it ASAP, and will think of ways to test it
when
> I
> > have a spare cycle sometime this week.
> > 
> > My current plan is to make pref_service_bridge.cc return a TestingProfile in
> > GetOriginalProfile(), and then expose one of the browsing data backends
> (adding
> > / deleting / counting entries) to the Java side to test that entries in the
> > correct time interval are deleted.
> 
> Sounds like a good plan.
> 
> While you're at it, could you move the code related to clearing browsing data
> from PrefServiceBridge into a ClearBrowsingDataBridge (or
> BrowsingDataRemovedBridge, or whatever name makes sense)? That's a big enough
> and complex enough chunk of code that it probably deserves its own home. It
> might make testing clearer as well.

Yep, SGTM.

[commit-bot: I haz the power, 2016-03-21 17:30:04]

Description was changed from

==========
Pass the time parameter to PrefServiceBridge.clearBrowsingData() explicitly.

The browsing data deletion on Android relied on the prefs::kDeleteTimePeriod
to determine the time period selection, similarly as we do on Desktop.

However, this is incorrect with respect to callsites in ChromeApplication and
ConfirmImportSyncDataDialog, that rely on the time period used being EVERYTHING.

Pass the time time period parameter explicitly, so that different callsites
can specify which time period to use.

BUG=595876
==========

to

==========
Pass the time parameter to PrefServiceBridge.clearBrowsingData() explicitly.

The browsing data deletion on Android relied on the prefs::kDeleteTimePeriod
to determine the time period selection, similarly as we do on Desktop.

However, this is incorrect with respect to callsites in ChromeApplication and
ConfirmImportSyncDataDialog, that rely on the time period used being EVERYTHING.

Pass the time time period parameter explicitly, so that different callsites
can specify which time period to use.

BUG=595876

Committed: https://crrev.com/5f5b82dc246c721ceb634983719494519694e57b
Cr-Commit-Position: refs/heads/master@{#382317}
==========

[commit-bot: I haz the power, 2016-03-21 17:30:05]

Patchset 1 (id:??) landed as
https://crrev.com/5f5b82dc246c721ceb634983719494519694e57b
Cr-Commit-Position: refs/heads/master@{#382317}