Skip to content

Commit

Permalink
Add support for time based deletion of browsing data on iOS
Browse files Browse the repository at this point in the history
Change existing browsing data deletion methods to take a browsing_data::TimePeriod as a parameter.

Keep old methods around until all call sites are changed downstream.

BUG=642334

Review-Url: https://codereview.chromium.org/2270063005
Cr-Commit-Position: refs/heads/master@{#419433}
  • Loading branch information
ioanap authored and Commit bot committed Sep 19, 2016
1 parent b22ae2a commit f433931
Show file tree
Hide file tree
Showing 11 changed files with 87 additions and 43 deletions.
3 changes: 2 additions & 1 deletion components/signin/ios/browser/account_consistency_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ class AccountConsistencyService : public KeyedService,
// Does nothing if the cookie is not set on |domain|.
void RemoveChromeConnectedCookieFromDomain(const std::string& domain);

// Notifies the AccountConsistencyService that browsing data has been removed.
// Notifies the AccountConsistencyService that browsing data has been removed
// for any time period.
void OnBrowsingDataRemoved();

private:
Expand Down
16 changes: 12 additions & 4 deletions ios/chrome/browser/browsing_data/browsing_data_remover_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ BrowsingDataRemoverHelper::~BrowsingDataRemoverHelper() {

BrowsingDataRemoverHelper::BrowsingDataRemovalInfo::BrowsingDataRemovalInfo(
int remove_mask,
browsing_data::TimePeriod time_period,
const base::Closure& callback)
: remove_mask(remove_mask) {
: remove_mask(remove_mask), time_period(time_period) {
callbacks.push_back(callback);
}

Expand All @@ -27,6 +28,7 @@ BrowsingDataRemoverHelper::BrowsingDataRemovalInfo::~BrowsingDataRemovalInfo() {

void BrowsingDataRemoverHelper::Remove(ios::ChromeBrowserState* browser_state,
int remove_mask,
browsing_data::TimePeriod time_period,
const base::Closure& callback) {
DCHECK(browser_state);
DCHECK(!browser_state->IsOffTheRecord());
Expand All @@ -43,19 +45,25 @@ void BrowsingDataRemoverHelper::Remove(ios::ChromeBrowserState* browser_state,
auto pending_removals_iter = pending_removals_.find(browser_state);
if (pending_removals_iter == pending_removals_.end()) {
std::unique_ptr<BrowsingDataRemovalInfo> removal_info(
new BrowsingDataRemovalInfo(remove_mask, callback));
new BrowsingDataRemovalInfo(remove_mask, time_period, callback));
pending_removals_[browser_state] = std::move(removal_info);
} else {
pending_removals_iter->second->remove_mask |= remove_mask;
pending_removals_iter->second->callbacks.push_back(callback);
}
} else {
std::unique_ptr<BrowsingDataRemovalInfo> removal_info(
new BrowsingDataRemovalInfo(remove_mask, callback));
new BrowsingDataRemovalInfo(remove_mask, time_period, callback));
DoRemove(browser_state, std::move(removal_info));
}
}

void BrowsingDataRemoverHelper::Remove(ios::ChromeBrowserState* browser_state,
int remove_mask,
const base::Closure& callback) {
Remove(browser_state, remove_mask, browsing_data::ALL_TIME, callback);
}

void BrowsingDataRemoverHelper::OnIOSChromeBrowsingDataRemoverDone() {
current_remover_ = nullptr;

Expand Down Expand Up @@ -90,7 +98,7 @@ void BrowsingDataRemoverHelper::DoRemove(
// IOSChromeBrowsingDataRemover deletes itself.
IOSChromeBrowsingDataRemover* remover =
IOSChromeBrowsingDataRemover::CreateForPeriod(
browser_state, IOSChromeBrowsingDataRemover::EVERYTHING);
browser_state, current_removal_info_->time_period);
remover->AddObserver(this);
current_remover_ = remover;
int remove_mask = current_removal_info_->remove_mask;
Expand Down
13 changes: 12 additions & 1 deletion ios/chrome/browser/browsing_data/browsing_data_remover_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ class BrowsingDataRemoverHelper
// |callback| is called on the main thread.
// Note: Removal operations are not necessarily processed in the sequence that
// they are received in.
void Remove(ios::ChromeBrowserState* browser_state,
int remove_mask,
browsing_data::TimePeriod time_period,
const base::Closure& callback);

// DEPRECATED: Same as above, but setting the |time_period| to ALL_TIME.
// TODO(ioanap): Remove after all call sites are changed.
void Remove(ios::ChromeBrowserState* browser_state,
int remove_mask,
const base::Closure& callback);
Expand All @@ -39,11 +46,15 @@ class BrowsingDataRemoverHelper
// a ChromeBrowserState.
struct BrowsingDataRemovalInfo {
// Creates a BrowsingDataRemovalInfo with a single callback |callback|.
BrowsingDataRemovalInfo(int remove_mask, const base::Closure& callback);
BrowsingDataRemovalInfo(int remove_mask,
browsing_data::TimePeriod time_period,
const base::Closure& callback);
~BrowsingDataRemovalInfo();
// The mask of all the types of browsing data that needs to be removed.
// Obtained from BrowsingDataRemoved::RemoveDataMask.
int remove_mask;
// Time period for which the user wants to remove the data.
browsing_data::TimePeriod time_period;
// The vector of callbacks that need to be run when browsing data is
// actually removed.
std::vector<base::Closure> callbacks;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/sequenced_task_runner_helpers.h"
#include "base/task/cancelable_task_tracker.h"
#include "base/time/time.h"
#include "components/browsing_data/core/browsing_data_utils.h"
#include "components/prefs/pref_member.h"
#include "components/search_engines/template_url_service.h"
#include "url/gurl.h"
Expand All @@ -33,9 +34,6 @@ class URLRequestContextGetter;

class IOSChromeBrowsingDataRemover {
public:
// Time period ranges available when doing browsing data removals.
enum TimePeriod { EVERYTHING };

// Mask used for Remove.
enum RemoveDataMask {
REMOVE_APPCACHE = 1 << 0,
Expand Down Expand Up @@ -119,10 +117,7 @@ class IOSChromeBrowsingDataRemover {
// itself once finished.
static IOSChromeBrowsingDataRemover* CreateForPeriod(
ios::ChromeBrowserState* browser_state,
TimePeriod period);

// Calculate the begin time for the deletion range specified by |time_period|.
static base::Time CalculateBeginDeleteTime(TimePeriod time_period);
browsing_data::TimePeriod period);

// Is the IOSChromeBrowsingDataRemover currently in the process of removing
// data?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,26 @@ bool AllDomainsPredicate(const std::string& domain) {
// Static.
IOSChromeBrowsingDataRemover* IOSChromeBrowsingDataRemover::CreateForPeriod(
ios::ChromeBrowserState* browser_state,
TimePeriod period) {
browsing_data::TimePeriod period) {
switch (period) {
case EVERYTHING:
case browsing_data::LAST_HOUR:
base::RecordAction(UserMetricsAction("ClearBrowsingData_LastHour"));
break;
case browsing_data::LAST_DAY:
base::RecordAction(UserMetricsAction("ClearBrowsingData_LastDay"));
break;
case browsing_data::LAST_WEEK:
base::RecordAction(UserMetricsAction("ClearBrowsingData_LastWeek"));
break;
case browsing_data::FOUR_WEEKS:
base::RecordAction(UserMetricsAction("ClearBrowsingData_LastMonth"));
break;
case browsing_data::ALL_TIME:
base::RecordAction(UserMetricsAction("ClearBrowsingData_Everything"));
break;
}
return new IOSChromeBrowsingDataRemover(
browser_state,
IOSChromeBrowsingDataRemover::CalculateBeginDeleteTime(period),
browser_state, browsing_data::CalculateBeginDeleteTime(period),
base::Time::Max());
}

Expand Down Expand Up @@ -303,9 +314,9 @@ bool AllDomainsPredicate(const std::string& domain) {
base::RecordAction(UserMetricsAction("ClearBrowsingData_Cache"));

waiting_for_clear_cache_ = true;
DCHECK(delete_begin_.is_null()) << "Partial clearing not supported";
ClearHttpCache(browser_state_->GetRequestContext(),
WebThread::GetTaskRunnerForThread(WebThread::IO),
delete_begin_, delete_end_,
base::Bind(&IOSChromeBrowsingDataRemover::OnClearedCache,
base::Unretained(this)));
}
Expand Down Expand Up @@ -350,21 +361,6 @@ bool AllDomainsPredicate(const std::string& domain) {
NotifyAndDeleteIfDone();
}

base::Time IOSChromeBrowsingDataRemover::CalculateBeginDeleteTime(
TimePeriod time_period) {
base::TimeDelta diff;
base::Time delete_begin_time = base::Time::Now();
switch (time_period) {
case EVERYTHING:
delete_begin_time = base::Time();
break;
default:
NOTREACHED() << L"Missing item";
break;
}
return delete_begin_time - diff;
}

bool IOSChromeBrowsingDataRemover::AllDone() {
return !waiting_for_clear_autofill_origin_urls_ &&
!waiting_for_clear_cache_ && !waiting_for_clear_channel_ids_ &&
Expand Down
3 changes: 2 additions & 1 deletion ios/chrome/browser/signin/browser_state_data_remover.mm
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@
base::scoped_nsobject<ClearBrowsingDataCommand> command(
[[ClearBrowsingDataCommand alloc]
initWithBrowserState:browser_state_
mask:kRemoveAllDataMask]);
mask:kRemoveAllDataMask
timePeriod:browsing_data::ALL_TIME]);

UIWindow* mainWindow = [[UIApplication sharedApplication] keyWindow];
DCHECK(mainWindow);
Expand Down
15 changes: 13 additions & 2 deletions ios/chrome/browser/ui/commands/clear_browsing_data_command.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#import <Foundation/Foundation.h>

#import "components/browsing_data/core/browsing_data_utils.h"
#import "ios/chrome/browser/ui/commands/generic_chrome_command.h"

namespace ios {
Expand All @@ -20,16 +21,26 @@ class ChromeBrowserState;
- (instancetype)initWithTag:(NSInteger)tag NS_UNAVAILABLE;

// Initializes a command intented to clear browsing data for |browserState|
// that correspong to removal mask |mask|.
// that corresponds to removal mask |mask| for the time period |timePeriod|.
- (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState
mask:(int)mask NS_DESIGNATED_INITIALIZER;
mask:(int)mask
timePeriod:(browsing_data::TimePeriod)timePeriod
NS_DESIGNATED_INITIALIZER;

// DEPRECATED: Same as above, but setting |timePeriod| to ALL_TIME.
// TODO(ioanap): Remove after all call sites are changed.
- (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState
mask:(int)mask;

// When executed this command will remove browsing data for this BrowserState.
@property(nonatomic, readonly) ios::ChromeBrowserState* browserState;

// Removal mask: see BrowsingDataRemover::RemoveDataMask.
@property(nonatomic, readonly) int mask;

// Time period for which the browsing data will be removed.
@property(nonatomic, readonly) browsing_data::TimePeriod timePeriod;

@end

#endif // IOS_CHROME_BROWSER_UI_COMMANDS_CLEAR_BROWSING_DATA_COMMAND_H_
12 changes: 11 additions & 1 deletion ios/chrome/browser/ui/commands/clear_browsing_data_command.mm
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,31 @@ @implementation ClearBrowsingDataCommand

@synthesize browserState = _browserState;
@synthesize mask = _mask;
@synthesize timePeriod = _timePeriod;

- (instancetype)initWithTag:(NSInteger)tag {
NOTREACHED();
return nil;
}

- (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState
mask:(int)mask {
mask:(int)mask
timePeriod:(browsing_data::TimePeriod)timePeriod {
self = [super initWithTag:IDC_CLEAR_BROWSING_DATA_IOS];
if (self) {
DCHECK(browserState);
_browserState = browserState;
_mask = mask;
_timePeriod = timePeriod;
}
return self;
}

- (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState
mask:(int)mask {
return [self initWithBrowserState:browserState
mask:mask
timePeriod:browsing_data::ALL_TIME];
}

@end
6 changes: 3 additions & 3 deletions ios/crnet/crnet_environment.mm
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ bool IsRequestSupported(NSURLRequest* request) override {

void CrNetEnvironment::ClearCache(ClearCacheCallback callback) {
PostToNetworkThread(
FROM_HERE,
base::Bind(&net::ClearHttpCache, main_context_getter_,
network_io_thread_->task_runner(), base::BindBlock(callback)));
FROM_HERE, base::Bind(&net::ClearHttpCache, main_context_getter_,
network_io_thread_->task_runner(), base::Time(),
base::Time::Max(), base::BindBlock(callback)));
}
15 changes: 12 additions & 3 deletions ios/net/http_cache_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,14 @@ void PostCallback(const scoped_refptr<base::TaskRunner>& task_runner,
// Clears the disk_cache::Backend on the IO thread and deletes |backend|.
void DoomHttpCache(std::unique_ptr<disk_cache::Backend*> backend,
const scoped_refptr<base::TaskRunner>& client_task_runner,
const base::Time& delete_begin,
const base::Time& delete_end,
const net::CompletionCallback& callback,
int error) {
// |*backend| may be null in case of error.
if (*backend) {
(*backend)->DoomAllEntries(
(*backend)->DoomEntriesBetween(
delete_begin, delete_end,
base::Bind(&PostCallback, client_task_runner, callback));
} else {
client_task_runner->PostTask(FROM_HERE, base::Bind(callback, error));
Expand All @@ -49,6 +52,8 @@ void DoomHttpCache(std::unique_ptr<disk_cache::Backend*> backend,
void ClearHttpCacheOnIOThread(
const scoped_refptr<net::URLRequestContextGetter>& getter,
const scoped_refptr<base::TaskRunner>& client_task_runner,
const base::Time& delete_begin,
const base::Time& delete_end,
const net::CompletionCallback& callback) {
net::HttpCache* http_cache =
getter->GetURLRequestContext()->http_transaction_factory()->GetCache();
Expand All @@ -72,7 +77,7 @@ void ClearHttpCacheOnIOThread(
disk_cache::Backend** backend_ptr = backend.get();
net::CompletionCallback doom_callback =
base::Bind(&DoomHttpCache, base::Passed(std::move(backend)),
client_task_runner, callback);
client_task_runner, delete_begin, delete_end, callback);

int rv = http_cache->GetBackend(backend_ptr, doom_callback);

Expand All @@ -89,10 +94,14 @@ namespace net {

void ClearHttpCache(const scoped_refptr<net::URLRequestContextGetter>& getter,
const scoped_refptr<base::TaskRunner>& network_task_runner,
const base::Time& delete_begin,
const base::Time& delete_end,
const net::CompletionCallback& callback) {
DCHECK(delete_end != base::Time());
network_task_runner->PostTask(
FROM_HERE, base::Bind(&ClearHttpCacheOnIOThread, getter,
base::ThreadTaskRunnerHandle::Get(), callback));
base::ThreadTaskRunnerHandle::Get(), delete_begin,
delete_end, callback));
}

} // namespace net
2 changes: 2 additions & 0 deletions ios/net/http_cache_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ class URLRequestContextGetter;
// Clears the HTTP cache and calls |closure| back.
void ClearHttpCache(const scoped_refptr<net::URLRequestContextGetter>& getter,
const scoped_refptr<base::TaskRunner>& network_task_runner,
const base::Time& delete_begin,
const base::Time& delete_end,
const net::CompletionCallback& callback);

} // namespace net
Expand Down

0 comments on commit f433931

Please sign in to comment.