[ReadingList iOS] Cleanup Offline directory on startup

ios/chrome/browser/reading_list/reading_list_download_service.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ios/chrome/browser/reading_list/reading_list_download_service.h"
 
 #include <utility>
 
 #include "base/bind.h"
+#include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
+#include "base/files/file_util.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "components/reading_list/ios/offline_url_utils.h"
 #include "components/reading_list/ios/reading_list_entry.h"
 #include "components/reading_list/ios/reading_list_model.h"
 #include "ios/chrome/browser/reading_list/reading_list_distiller_page_factory.h"
 #include "ios/web/public/web_thread.h"
 
 namespace {
 // Status of the download when it ends, for UMA report.
@@ skipping 54 matching lines @@
   return reading_list::OfflineRootDirectoryPath(chrome_profile_path_);
 }
 
 void ReadingListDownloadService::Shutdown() {
   reading_list_model_->RemoveObserver(this);
 }
 
 void ReadingListDownloadService::ReadingListModelLoaded(
     const ReadingListModel* model) {
   DCHECK_EQ(reading_list_model_, model);
   DownloadAllEntries();

[gambard, 2017/01/25 09:02:07]

Don't you need to update this?

[Olivier, 2017/01/25 12:25:30]

mmm, yes, I think it would work better if this is updated.

 }
 
 void ReadingListDownloadService::ReadingListWillRemoveEntry(
     const ReadingListModel* model,
     const GURL& url) {
   DCHECK_EQ(reading_list_model_, model);
   DCHECK(model->GetEntryByURL(url));
   RemoveDownloadedEntry(url);
 }
 
@@ skipping 14 matching lines @@
 
 void ReadingListDownloadService::ProcessNewEntry(const GURL& url) {
   const ReadingListEntry* entry = reading_list_model_->GetEntryByURL(url);
   if (!entry || entry->IsRead()) {
     url_downloader_->CancelDownloadOfflineURL(url);
   } else {
     ScheduleDownloadEntry(url);
   }
 }
 
-void ReadingListDownloadService::DownloadAllEntries() {
+void ReadingListDownloadService::SyncWithModel() {
   DCHECK(reading_list_model_->loaded());
+  std::set<std::string> processed_directories;
+  std::set<GURL> unprocessed_entries;
   for (const auto& url : reading_list_model_->Keys()) {
+    const ReadingListEntry* entry = reading_list_model_->GetEntryByURL(url);
+    if (entry->DistilledState() == ReadingListEntry::PROCESSED) {

[jif, 2017/01/25 09:15:51]

You know the drill. Can you do a:

switch (entry->DistilledState()) {

with all the states explicitly handled (i.e. no "default")?

[Olivier, 2017/01/25 12:25:30]

Done.

+      std::string directory_name = reading_list::OfflineURLDirectoryID(url);
+      processed_directories.insert(directory_name);

[jif, 2017/01/25 09:15:51]

can you do:

processed_directories.insert(reading_list::OfflineURLDirectoryID(url));

or:

{
  std::string directory_name = reading_list::OfflineURLDirectoryID(url);
  processed_directories.insert(std::move(directory_name));
}

[Olivier, 2017/01/25 12:25:30]

Done.

+    } else if (entry->DistilledState() != ReadingListEntry::ERROR) {
+      unprocessed_entries.insert(url);
+    }
+  }
+  web::WebThread::PostTaskAndReply(
+      web::WebThread::FILE, FROM_HERE,
+      base::Bind(&ReadingListDownloadService::CleanUpFiles,
+                 base::Unretained(this), processed_directories),
+      base::Bind(&ReadingListDownloadService::DownloadUnprocessedEntries,
+                 base::Unretained(this), unprocessed_entries));
+}
+
+void ReadingListDownloadService::CleanUpFiles(
+    const std::set<std::string>& processed_directories) {
+  base::FileEnumerator file_enumerator(OfflineRoot(), false,
+                                       base::FileEnumerator::DIRECTORIES);
+  for (base::FilePath sub_directory = file_enumerator.Next();
+       !sub_directory.empty(); sub_directory = file_enumerator.Next()) {
+    std::string directory_name = sub_directory.BaseName().value();
+    if (!processed_directories.count(directory_name)) {
+      base::DeleteFile(sub_directory, true);
+    }
+  }
+}
+
+void ReadingListDownloadService::DownloadUnprocessedEntries(
+    const std::set<GURL>& unprocessed_entries) {
+  for (const GURL& url : unprocessed_entries) {
     this->ScheduleDownloadEntry(url);
   }
 }
 
 void ReadingListDownloadService::ScheduleDownloadEntry(const GURL& url) {
   DCHECK(reading_list_model_->loaded());
   const ReadingListEntry* entry = reading_list_model_->GetEntryByURL(url);
   if (!entry || entry->DistilledState() == ReadingListEntry::ERROR ||
       entry->DistilledState() == ReadingListEntry::PROCESSED || entry->IsRead())
     return;
@@ skipping 112 matching lines @@
     for (auto& url : url_to_download_cellular_) {
       ScheduleDownloadEntry(url);
     }
   }
   if (type == net::NetworkChangeNotifier::CONNECTION_WIFI) {
     for (auto& url : url_to_download_wifi_) {
       ScheduleDownloadEntry(url);
     }
   }
 }