Delete cookies for site when deleting locally stored data.

And, in the process, fix a bug in the CookieTreeModel, where it was prematurely sending BatchEnded events.

BUG=445962, 447395

Review URL: https://codereview.chromium.org/863503002

Cr-Commit-Position: refs/heads/master@{#313902}

chrome/android/java/src/org/chromium/chrome/browser/preferences/website/LocalStorageInfo.java

@@ -19,6 +19,7 @@ public class LocalStorageInfo implements Serializable {
     }
 
     public void clear() {
+        WebsitePreferenceBridge.nativeClearCookieData(mOrigin);
         WebsitePreferenceBridge.nativeClearLocalStorageData(mOrigin);
     }
 

chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePreferenceBridge.java

@@ -215,6 +215,7 @@ public abstract class WebsitePreferenceBridge {
     private static native void nativeGetCookieOrigins(Object list, boolean managedOnly);
     static native int nativeGetCookieSettingForOrigin(String origin, String embedder);
     static native void nativeSetCookieSettingForOrigin(String origin, String embedder, int setting);
+    static native void nativeClearCookieData(String path);
     static native void nativeClearLocalStorageData(String path);
     static native void nativeClearStorageData(String origin, int type, Object callback);
     private static native void nativeFetchLocalStorageInfo(Object callback);

chrome/android/java/strings/android_chrome_strings.grd

@@ -328,13 +328,13 @@
         Clear & reset
       </message>
       <message name="IDS_WEBSITE_RESET_CONFIRMATION" desc="The confirmation text asking if the user is sure about clearing all data and reset the permissions for a site.">
-        Are you sure you want to clear all data for this website and reset all its permissions?
+        Are you sure you want to clear all local data, including cookies, and reset all permissions for this website?
       </message>
       <message name="IDS_WEBSTORAGE_CLEAR_DATA_DIALOG_TITLE" desc="Title of the confirmation dialog when the user ask to clear all data for an origin">
         Clear stored data
       </message>
       <message name="IDS_WEBSTORAGE_CLEAR_DATA_DIALOG_MESSAGE" desc="Text of the confirmation dialog when the user ask to clear all data for an origin">
-        All data stored by this website will be deleted
+        All local data stored by this website, including cookies, will be deleted.
       </message>
       <message name="IDS_WEBSTORAGE_CLEAR_DATA_DIALOG_OK_BUTTON" desc="Button confirming site data cleanup [CHAR-LIMIT=20]">
         Clear all

chrome/browser/android/preferences/website_preference_bridge.cc

@@ -12,6 +12,8 @@
 #include "base/files/file_path.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browsing_data/browsing_data_local_storage_helper.h"
+#include "chrome/browser/browsing_data/cookies_tree_model.h"
+#include "chrome/browser/browsing_data/local_data_container.h"
 #include "chrome/browser/content_settings/cookie_settings.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -285,6 +287,100 @@ static void SetCookieSettingForOrigin(JNIEnv* env, jclass clazz,
 
 namespace {
 
+class SiteDataDeleteHelper :
+      public base::RefCountedThreadSafe<SiteDataDeleteHelper>,
+      public CookiesTreeModel::Observer {
+ public:
+  SiteDataDeleteHelper(Profile* profile, const GURL& domain)
+      : profile_(profile), domain_(domain), ending_batch_processing_(false) {
+  }
+
+  void Run() {
+    AddRef();  // Balanced in TreeModelEndBatch.
+
+    content::StoragePartition* storage_partition =
+        content::BrowserContext::GetDefaultStoragePartition(profile_);
+    content::IndexedDBContext* indexed_db_context =
+        storage_partition->GetIndexedDBContext();
+    content::ServiceWorkerContext* service_worker_context =
+        storage_partition->GetServiceWorkerContext();
+    storage::FileSystemContext* file_system_context =
+        storage_partition->GetFileSystemContext();
+    LocalDataContainer* container = new LocalDataContainer(
+        new BrowsingDataCookieHelper(profile_->GetRequestContext()),
+        new BrowsingDataDatabaseHelper(profile_),
+        new BrowsingDataLocalStorageHelper(profile_),
+        NULL,
+        new BrowsingDataAppCacheHelper(profile_),
+        new BrowsingDataIndexedDBHelper(indexed_db_context),
+        BrowsingDataFileSystemHelper::Create(file_system_context),
+        BrowsingDataQuotaHelper::Create(profile_),
+        BrowsingDataChannelIDHelper::Create(profile_->GetRequestContext()),
+        new BrowsingDataServiceWorkerHelper(service_worker_context),
+        NULL);
+
+    cookies_tree_model_.reset(new CookiesTreeModel(
+        container, profile_->GetExtensionSpecialStoragePolicy(), false));
+    cookies_tree_model_->AddCookiesTreeObserver(this);
+  }
+
+  // TreeModelObserver:
+  void TreeNodesAdded(ui::TreeModel* model,
+                              ui::TreeModelNode* parent,
+                              int start,
+                              int count) override {}
+  void TreeNodesRemoved(ui::TreeModel* model,
+                                ui::TreeModelNode* parent,
+                                int start,
+                                int count) override {}
+
+  // CookiesTreeModel::Observer:
+  void TreeNodeChanged(ui::TreeModel* model, ui::TreeModelNode* node) override {
+  }
+
+  void TreeModelBeginBatch(CookiesTreeModel* model) override {
+    DCHECK(!ending_batch_processing_);  // Extra batch-start sent.
+  }
+
+  void TreeModelEndBatch(CookiesTreeModel* model) override {
+    DCHECK(!ending_batch_processing_);  // Already in end-stage.
+    ending_batch_processing_ = true;
+
+    RecursivelyFindSiteAndDelete(cookies_tree_model_->GetRoot());
+
+    // This will result in this class getting deleted.
+    Release();
+  }
+
+  void RecursivelyFindSiteAndDelete(CookieTreeNode* node) {
+    CookieTreeNode::DetailedInfo info = node->GetDetailedInfo();
+    for (int i = node->child_count(); i > 0; --i)
+      RecursivelyFindSiteAndDelete(node->GetChild(i - 1));
+
+    if (info.node_type == CookieTreeNode::DetailedInfo::TYPE_COOKIE &&
+        info.cookie &&
+        domain_.DomainIs(info.cookie->Domain().c_str()))
+      cookies_tree_model_->DeleteCookieNode(node);
+  }
+
+ private:
+  friend class base::RefCountedThreadSafe<SiteDataDeleteHelper>;
+
+  virtual ~SiteDataDeleteHelper() {}
+
+  Profile* profile_;
+
+  // The domain we want to delete data for.
+  GURL domain_;
+
+  // Keeps track of when we're ready to close batch processing.
+  bool ending_batch_processing_;
+
+  scoped_ptr<CookiesTreeModel> cookies_tree_model_;
+
+  DISALLOW_COPY_AND_ASSIGN(SiteDataDeleteHelper);
+};
+
 class StorageInfoFetcher :
       public base::RefCountedThreadSafe<StorageInfoFetcher> {
  public:
@@ -500,6 +596,14 @@ static void ClearStorageData(JNIEnv* env,
   storage_data_deleter->Run();
 }
 
+static void ClearCookieData(JNIEnv* env, jclass clazz, jstring jorigin) {
+  Profile* profile = ProfileManager::GetActiveUserProfile();
+  GURL url(ConvertJavaStringToUTF8(env, jorigin));
+  scoped_refptr<SiteDataDeleteHelper> site_data_deleter(
+      new SiteDataDeleteHelper(profile, url));
+  site_data_deleter->Run();
+}
+
 // Register native methods
 bool RegisterWebsitePreferenceBridge(JNIEnv* env) {
   return RegisterNativesImpl(env);

chrome/browser/browsing_data/cookies_tree_model.cc

@@ -910,12 +910,17 @@ CookieTreeNode::DetailedInfo CookieTreeFlashLSONode::GetDetailedInfo() const {
 CookiesTreeModel::ScopedBatchUpdateNotifier::ScopedBatchUpdateNotifier(
   CookiesTreeModel* model, CookieTreeNode* node)
       : model_(model), node_(node), batch_in_progress_(false) {
+  model_->RecordBatchSeen();
 }
 
 CookiesTreeModel::ScopedBatchUpdateNotifier::~ScopedBatchUpdateNotifier() {
   if (batch_in_progress_) {
     model_->NotifyObserverTreeNodeChanged(node_);
     model_->NotifyObserverEndBatch();
+  } else {
+    // If no batch started, and this is the last batch, give the model a chance
+    // to send out a final notification.
+    model_->MaybeNotifyBatchesEnded();
   }
 }
 
@@ -938,7 +943,10 @@ CookiesTreeModel::CookiesTreeModel(
       special_storage_policy_(special_storage_policy),
 #endif
       group_by_cookie_source_(group_by_cookie_source),
-      batch_update_(0) {
+      batches_expected_(0),
+      batches_seen_(0),
+      batches_started_(0),
+      batches_ended_(0) {
   data_container_->Init(this);
 }
 
@@ -1017,6 +1025,7 @@ void CookiesTreeModel::DeleteCookieNode(CookieTreeNode* cookie_node) {
 
 void CookiesTreeModel::UpdateSearchResults(const base::string16& filter) {
   CookieTreeNode* root = GetRoot();
+  SetBatchExpectation(1, true);
   ScopedBatchUpdateNotifier notifier(this, root);
   int num_children = root->child_count();
   notifier.StartBatchUpdate();
@@ -1425,9 +1434,24 @@ void CookiesTreeModel::PopulateFlashLSOInfoWithFilter(
   }
 }
 
+void CookiesTreeModel::SetBatchExpectation(int batches_expected, bool reset) {
+  batches_expected_ = batches_expected;
+  if (reset) {
+    batches_seen_ = 0;
+    batches_started_ = 0;
+    batches_ended_ = 0;
+  } else {
+    MaybeNotifyBatchesEnded();
+  }
+}
+
+void CookiesTreeModel::RecordBatchSeen() {
+  batches_seen_++;
+}
+
 void CookiesTreeModel::NotifyObserverBeginBatch() {
   // Only notify the model once if we're batching in a nested manner.
-  if (batch_update_++ == 0) {
+  if (batches_started_++ == 0) {
     FOR_EACH_OBSERVER(Observer,
                       cookies_observer_list_,
                       TreeModelBeginBatch(this));
@@ -1435,9 +1459,15 @@ void CookiesTreeModel::NotifyObserverBeginBatch() {
 }
 
 void CookiesTreeModel::NotifyObserverEndBatch() {
+  batches_ended_++;
+  MaybeNotifyBatchesEnded();
+}
+
+void CookiesTreeModel::MaybeNotifyBatchesEnded() {
   // Only notify the observers if this is the outermost call to EndBatch() if
   // called in a nested manner.
-  if (--batch_update_ == 0) {
+  if (batches_ended_ == batches_started_ &&
+      batches_seen_ == batches_expected_) {
     FOR_EACH_OBSERVER(Observer,
                       cookies_observer_list_,
                       TreeModelEndBatch(this));

chrome/browser/browsing_data/cookies_tree_model.h

@@ -719,6 +719,12 @@ class CookiesTreeModel : public ui::TreeNodeModel<CookieTreeNode> {
     return data_container_.get();
   }
 
+  // Set the number of |batches_expected| this class should expect to receive.
+  // If |reset| is true, then this is a new set of batches, but if false, then
+  // this is a revised number (batches originally counted should no longer be
+  // expected).
+  void SetBatchExpectation(int batches_expected, bool reset);
+
  private:
   enum CookieIconIndex {
     ORIGIN = 0,
@@ -726,9 +732,24 @@ class CookiesTreeModel : public ui::TreeNodeModel<CookieTreeNode> {
     DATABASE = 2
   };
 
+  // Reset the counters for batches.
+  void ResetBatches();
+
+  // Record that one batch has been delivered.
+  void RecordBatchSeen();
+
+  // Record that one batch has begun processing. If this is the first batch then
+  // observers will be notified that batch processing has started.
   void NotifyObserverBeginBatch();
+
+  // Record that one batch has finished processing. If this is the last batch
+  // then observers will be notified that batch processing has ended.
   void NotifyObserverEndBatch();
 
+  // Notifies observers if expected batch count has been delievered and all
+  // batches have finished processing.
+  void MaybeNotifyBatchesEnded();
+
   void PopulateAppCacheInfoWithFilter(LocalDataContainer* container,
                                       ScopedBatchUpdateNotifier* notifier,
                                       const base::string16& filter);
@@ -782,10 +803,19 @@ class CookiesTreeModel : public ui::TreeNodeModel<CookieTreeNode> {
   // Otherwise, use the CanonicalCookie::Domain attribute.
   bool group_by_cookie_source_;
 
-  // If this is non-zero, then this model is batching updates (there's a lot of
-  // notifications coming down the pipe). This is an integer is used to balance
-  // calls to Begin/EndBatch() if they're called in a nested manner.
-  int batch_update_;
+  // Keeps track of how many batches the consumer of this class says it is going
+  // to send.
+  int batches_expected_;
+
+  // Keeps track of how many batches we've seen.
+  int batches_seen_;
+
+  // Counts how many batches have started already. If this is non-zero and lower
+  // than batches_ended_, then this model is still batching updates.
+  int batches_started_;
+
+  // Counts how many batches have finished.
+  int batches_ended_;
 };
 
 #endif  // CHROME_BROWSER_BROWSING_DATA_COOKIES_TREE_MODEL_H_

chrome/browser/browsing_data/local_data_container.cc

@@ -39,6 +39,7 @@ LocalDataContainer::LocalDataContainer(
       service_worker_helper_(service_worker_helper),
       flash_lso_helper_(flash_lso_helper),
       model_(NULL),
+      batches_started_(0),
       weak_ptr_factory_(this) {
 }
 
@@ -48,24 +49,28 @@ void LocalDataContainer::Init(CookiesTreeModel* model) {
   DCHECK(!model_);
   model_ = model;
 
+  batches_started_ = 1;
   DCHECK(cookie_helper_.get());
   cookie_helper_->StartFetching(
       base::Bind(&LocalDataContainer::OnCookiesModelInfoLoaded,
                  weak_ptr_factory_.GetWeakPtr()));
 
   if (database_helper_.get()) {
+    batches_started_++;
     database_helper_->StartFetching(
         base::Bind(&LocalDataContainer::OnDatabaseModelInfoLoaded,
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (local_storage_helper_.get()) {
+    batches_started_++;
     local_storage_helper_->StartFetching(
         base::Bind(&LocalDataContainer::OnLocalStorageModelInfoLoaded,
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (session_storage_helper_.get()) {
+    batches_started_++;
     session_storage_helper_->StartFetching(
         base::Bind(&LocalDataContainer::OnSessionStorageModelInfoLoaded,
                    weak_ptr_factory_.GetWeakPtr()));
@@ -74,46 +79,55 @@ void LocalDataContainer::Init(CookiesTreeModel* model) {
   // TODO(michaeln): When all of the UI implementations have been updated, make
   // this a required parameter.
   if (appcache_helper_.get()) {
+    batches_started_++;
     appcache_helper_->StartFetching(
         base::Bind(&LocalDataContainer::OnAppCacheModelInfoLoaded,
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (indexed_db_helper_.get()) {
+    batches_started_++;
     indexed_db_helper_->StartFetching(
         base::Bind(&LocalDataContainer::OnIndexedDBModelInfoLoaded,
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (file_system_helper_.get()) {
+    batches_started_++;
     file_system_helper_->StartFetching(
         base::Bind(&LocalDataContainer::OnFileSystemModelInfoLoaded,
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (quota_helper_.get()) {
+    batches_started_++;
     quota_helper_->StartFetching(
         base::Bind(&LocalDataContainer::OnQuotaModelInfoLoaded,
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (channel_id_helper_.get()) {
+    batches_started_++;
     channel_id_helper_->StartFetching(
         base::Bind(&LocalDataContainer::OnChannelIDModelInfoLoaded,
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (service_worker_helper_.get()) {
+    batches_started_++;
     service_worker_helper_->StartFetching(
         base::Bind(&LocalDataContainer::OnServiceWorkerModelInfoLoaded,
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
   if (flash_lso_helper_.get()) {
+    batches_started_++;
     flash_lso_helper_->StartFetching(
         base::Bind(&LocalDataContainer::OnFlashLSOInfoLoaded,
                    weak_ptr_factory_.GetWeakPtr()));
   }
+
+  model_->SetBatchExpectation(batches_started_, true);
 }
 
 void LocalDataContainer::OnAppCacheModelInfoLoaded() {
@@ -124,8 +138,11 @@ void LocalDataContainer::OnAppCacheModelInfoLoaded() {
 
   scoped_refptr<AppCacheInfoCollection> appcache_info =
       appcache_helper_->info_collection();
-  if (!appcache_info.get() || appcache_info->infos_by_origin.empty())
+  if (!appcache_info.get() || appcache_info->infos_by_origin.empty()) {
+    // This batch has been canceled, so let the model know it won't be arriving.
+    model_->SetBatchExpectation(--batches_started_, false);
     return;
+  }
 
   for (InfoByOrigin::const_iterator origin =
            appcache_info->infos_by_origin.begin();

chrome/browser/browsing_data/local_data_container.h

@@ -143,6 +143,9 @@ class LocalDataContainer {
   // delegate to deliver the updated data to the CookieTreeModel.
   CookiesTreeModel* model_;
 
+  // Keeps track of how many batches are expected to start.
+  int batches_started_;
+
   base::WeakPtrFactory<LocalDataContainer> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(LocalDataContainer);