Supervised user blacklist: Don't download the blacklist file on every startup

chrome/browser/supervised_user/supervised_user_service.cc

 // Copyright 2014 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 "chrome/browser/supervised_user/supervised_user_service.h"
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
+#include "base/files/file_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/path_service.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/version.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/component_updater/supervised_user_whitelist_installer.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ skipping 594 matching lines @@
   url_filter_context_.LoadWhitelists(site_lists);
 }
 
 void SupervisedUserService::OnSiteListUpdated() {
   FOR_EACH_OBSERVER(
       SupervisedUserServiceObserver, observer_list_, OnURLFilterChanged());
 }
 
 void SupervisedUserService::LoadBlacklist(const base::FilePath& path,
                                           const GURL& url) {
-  // TODO(treib): Don't re-download the blacklist if the local file exists!
-  if (!url.is_valid()) {
+  BrowserThread::PostTaskAndReplyWithResult(
+      BrowserThread::IO,

[Bernhard Bauer, 2015/04/16 13:58:06]

I don't think you want to do this on the IO thread (which, sadly, is not the
thread to do disk I/O on: it's only for non-blocking I/O, which in practice
means IPC...). You probably want BrowserThread::GetBlockingPool() (with
base::PostTaskAndReplyWithResult()).

[Marc Treib, 2015/04/16 14:04:16]

Argh, yeah, I ran into this before (the IO thread not being allowed to do IO ;)
)
I meant to use BrowserThread::FILE. Is GetBlockingPool preferable to that?

[Bernhard Bauer, 2015/04/16 14:09:16]

Yes, because it will use more than one thread, so there is less risk of
contention. It also doesn't block shutdown by default (I think).

[Marc Treib, 2015/04/16 14:40:49]

Got it, thanks! Done.

+      FROM_HERE,
+      base::Bind(&base::PathExists, path),
+      base::Bind(&SupervisedUserService::OnBlacklistFileChecked,
+                 weak_ptr_factory_.GetWeakPtr(), path, url));
+}
+
+void SupervisedUserService::OnBlacklistFileChecked(const base::FilePath& path,
+                                                   const GURL& url,
+                                                   bool file_exists) {
+  if (file_exists) {
     LoadBlacklistFromFile(path);
     return;
   }
 
   DCHECK(!blacklist_downloader_);
   blacklist_downloader_.reset(new SupervisedUserBlacklistDownloader(
       url,
       path,
       profile_->GetRequestContext(),
       base::Bind(&SupervisedUserService::OnBlacklistDownloadDone,
@@ skipping 313 matching lines @@
   // The active user can be NULL in unit tests.
   if (user_manager::UserManager::Get()->GetActiveUser()) {
     return UTF16ToUTF8(user_manager::UserManager::Get()->GetUserDisplayName(
         user_manager::UserManager::Get()->GetActiveUser()->GetUserID()));
   }
   return std::string();
 #else
   return profile_->GetPrefs()->GetString(prefs::kProfileName);
 #endif
 }