Remove custom Task implementations from VisitedLinkMaster and web_app.

Fairly straightforward conversion except for two things:
- VisitedLinkMaster tries to be really clever and used StackVector. I converted
  it to use a plain old std::string since I don't think the cleverness adds
  anything.
- web_app::CreateShortcut() sole caller doesn't even use the success callback, so it was removed.

BUG=none
TEST=trybots

Do not submit. It's entirely possible we can just delete all the CreateShortcut
related goop in web_app.


Review URL: http://codereview.chromium.org/8885007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@113662 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/views/create_application_shortcut_view.cc

@@ -376,9 +376,7 @@ bool CreateApplicationShortcutView::Accept() {
   shortcut_info_.create_in_quick_launch_bar = false;
 #endif
 
-  web_app::CreateShortcut(profile_->GetPath(),
-                          shortcut_info_,
-                          NULL);
+  web_app::CreateShortcut(profile_->GetPath(), shortcut_info_);
   return true;
 }
 

chrome/browser/visitedlink/visitedlink_master.cc

@@ -14,6 +14,7 @@
 #include <algorithm>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
@@ -62,86 +63,30 @@ void GenerateSalt(uint8 salt[LINK_SALT_LENGTH]) {
   memcpy(salt, &randval, 8);
 }
 
-// AsyncWriter ----------------------------------------------------------------
-
-// This task executes on a background thread and executes a write. This
-// prevents us from blocking the UI thread doing I/O.
-class AsyncWriter : public Task {
- public:
-  AsyncWriter(FILE* file, int32 offset, const void* data, size_t data_len)
-      : file_(file),
-        offset_(offset) {
-    data_->resize(data_len);
-    memcpy(&*data_->begin(), data, data_len);
-  }
-
-  virtual void Run() {
-    WriteToFile(file_, offset_,
-                &*data_->begin(), static_cast<int32>(data_->size()));
-  }
-
-  // Exposed as a static so it can be called directly from the Master to
-  // reduce the number of platform-specific I/O sites we have. Returns true if
-  // the write was complete.
-  static bool WriteToFile(FILE* file,
-                          off_t offset,
-                          const void* data,
-                          size_t data_len) {
-    if (fseek(file, offset, SEEK_SET) != 0)
-      return false;  // Don't write to an invalid part of the file.
-
-    size_t num_written = fwrite(data, 1, data_len, file);
-
-    // The write may not make it to the kernel (stdlib may buffer the write)
-    // until the next fseek/fclose call.  If we crash, it's easy for our used
-    // item count to be out of sync with the number of hashes we write.
-    // Protect against this by calling fflush.
-    int ret = fflush(file);
-    DCHECK_EQ(0, ret);
-    return num_written == data_len;
-  }
-
- private:
-  // The data to write and where to write it.
-  FILE* file_;
-  int32 offset_;  // Offset from the beginning of the file.
-
-  // Most writes are just a single fingerprint, so we reserve that much in this
-  // object to avoid mallocs in that case.
-  StackVector<char, sizeof(VisitedLinkCommon::Fingerprint)> data_;
-
-  DISALLOW_COPY_AND_ASSIGN(AsyncWriter);
-};
-
-// Used to asynchronously set the end of the file. This must be done on the
-// same thread as the writing to keep things synchronized.
-class AsyncSetEndOfFile : public Task {
- public:
-  explicit AsyncSetEndOfFile(FILE* file) : file_(file) {}
-
-  virtual void Run() {
-    TruncateFile(file_);
-  }
-
- private:
-  FILE* file_;
-  DISALLOW_COPY_AND_ASSIGN(AsyncSetEndOfFile);
-};
+// Returns true if the write was complete.
+static bool WriteToFile(FILE* file,
+                        off_t offset,
+                        const void* data,
+                        size_t data_len) {
+  if (fseek(file, offset, SEEK_SET) != 0)
+    return false;  // Don't write to an invalid part of the file.
 
-// Used to asynchronously close a file. This must be done on the same thread as
-// the writing to keep things synchronized.
-class AsyncCloseHandle : public Task {
- public:
-  explicit AsyncCloseHandle(FILE* file) : file_(file) {}
+  size_t num_written = fwrite(data, 1, data_len, file);
 
-  virtual void Run() {
-    fclose(file_);
-  }
+  // The write may not make it to the kernel (stdlib may buffer the write)
+  // until the next fseek/fclose call.  If we crash, it's easy for our used
+  // item count to be out of sync with the number of hashes we write.
+  // Protect against this by calling fflush.
+  int ret = fflush(file);
+  DCHECK_EQ(0, ret);
+  return num_written == data_len;
+}
 
- private:
-  FILE* file_;
-  DISALLOW_COPY_AND_ASSIGN(AsyncCloseHandle);
-};
+// This task executes on a background thread and executes a write. This
+// prevents us from blocking the UI thread doing I/O.
+void AsyncWrite(FILE* file, int32 offset, const std::string& data) {
+  WriteToFile(file, offset, data.data(), data.size());
+}
 
 }  // namespace
 
@@ -536,7 +481,9 @@ bool VisitedLinkMaster::WriteFullTable() {
 
   // The hash table may have shrunk, so make sure this is the end.
   BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE, new AsyncSetEndOfFile(file_));
+      BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(base::IgnoreResult(&TruncateFile), file_));
   return true;
 }
 
@@ -728,7 +675,9 @@ void VisitedLinkMaster::FreeURLTable() {
     return;
 
   BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE, new AsyncCloseHandle(file_));
+      BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(base::IgnoreResult(&fclose), file_));
 }
 
 bool VisitedLinkMaster::ResizeTableIfNecessary() {
@@ -914,7 +863,8 @@ void VisitedLinkMaster::WriteToFile(FILE* file,
 
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
-      new AsyncWriter(file, offset, data, data_size));
+      base::Bind(&AsyncWrite, file, offset,
+                 std::string(static_cast<const char*>(data), data_size)));
 }
 
 void VisitedLinkMaster::WriteUsedItemCountToFile() {

chrome/browser/web_applications/web_app.cc

@@ -8,6 +8,7 @@
 #include <shlobj.h>
 #endif  // defined(OS_WIN)
 
+#include "base/bind.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/i18n/file_util_icu.h"
@@ -148,79 +149,9 @@ bool ShouldUpdateIcon(const FilePath& icon_file, const SkBitmap& image) {
 
 #endif  // defined(OS_WIN)
 
-// Represents a task that creates web application shortcut. This runs on
-// file thread and schedules the callback (if any) on the calling thread
-// when finished (either success or failure).
-class CreateShortcutTask : public Task {
- public:
-  CreateShortcutTask(const FilePath& profile_path,
-                     const ShellIntegration::ShortcutInfo& shortcut_info,
-                     web_app::CreateShortcutCallback* callback);
-
- private:
-  class CreateShortcutCallbackTask : public Task {
-   public:
-    CreateShortcutCallbackTask(web_app::CreateShortcutCallback* callback,
-        bool success)
-        : callback_(callback),
-          success_(success) {
-    }
-
-    // Overridden from Task:
-    virtual void Run() {
-      callback_->Run(success_);
-    }
-
-   private:
-    web_app::CreateShortcutCallback* callback_;
-    bool success_;
-  };
-
-  // Overridden from Task:
-  virtual void Run();
-
-  // Returns true if shortcut is created successfully.
-  bool CreateShortcut();
-
-  // Path to store persisted data for web app.
-  FilePath web_app_path_;
-
-  // Out copy of profile path.
-  FilePath profile_path_;
-
-  // Our copy of short cut data.
-  ShellIntegration::ShortcutInfo shortcut_info_;
-
-  // Callback when task is finished.
-  web_app::CreateShortcutCallback* callback_;
-  MessageLoop* message_loop_;
-
-  DISALLOW_COPY_AND_ASSIGN(CreateShortcutTask);
-};
-
-CreateShortcutTask::CreateShortcutTask(
-    const FilePath& profile_path,
-    const ShellIntegration::ShortcutInfo& shortcut_info,
-    web_app::CreateShortcutCallback* callback)
-    : web_app_path_(web_app::internals::GetWebAppDataDirectory(
-        web_app::GetDataDir(profile_path),
-        shortcut_info)),
-      profile_path_(profile_path),
-      shortcut_info_(shortcut_info),
-      callback_(callback),
-      message_loop_(MessageLoop::current()) {
-  DCHECK(message_loop_ != NULL);
-}
-
-void CreateShortcutTask::Run() {
-  bool success = CreateShortcut();
-
-  if (callback_ != NULL)
-    message_loop_->PostTask(FROM_HERE,
-      new CreateShortcutCallbackTask(callback_, success));
-}
-
-bool CreateShortcutTask::CreateShortcut() {
+void CreateShortcutTask(const FilePath& web_app_path,
+                        const FilePath& profile_path,
+                        const ShellIntegration::ShortcutInfo& shortcut_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX)
@@ -229,10 +160,10 @@ bool CreateShortcutTask::CreateShortcut() {
   std::string shortcut_template;
   if (!ShellIntegration::GetDesktopShortcutTemplate(env.get(),
                                                     &shortcut_template)) {
-    return false;
+    return;
   }
-  ShellIntegration::CreateDesktopShortcut(shortcut_info_, shortcut_template);
-  return true;  // assuming always success.
+  ShellIntegration::CreateDesktopShortcut(shortcut_info, shortcut_template);
+  return;  // assuming always success.
 #elif defined(OS_WIN)
   // Shortcut paths under which to create shortcuts.
   std::vector<FilePath> shortcut_paths;
@@ -244,15 +175,15 @@ bool CreateShortcutTask::CreateShortcut() {
     const wchar_t* sub_dir;
   } locations[] = {
     {
-      shortcut_info_.create_on_desktop,
+      shortcut_info.create_on_desktop,
       chrome::DIR_USER_DESKTOP,
       NULL
     }, {
-      shortcut_info_.create_in_applications_menu,
+      shortcut_info.create_in_applications_menu,
       base::DIR_START_MENU,
       NULL
     }, {
-      shortcut_info_.create_in_quick_launch_bar,
+      shortcut_info.create_in_quick_launch_bar,
       // For Win7, create_in_quick_launch_bar means pinning to taskbar. Use
       // base::PATH_START as a flag for this case.
       (base::win::GetVersion() >= base::win::VERSION_WIN7) ?
@@ -272,8 +203,7 @@ bool CreateShortcutTask::CreateShortcut() {
         continue;
 
       if (!PathService::Get(locations[i].location_id, &path)) {
-        NOTREACHED();
-        return false;
+        return;
       }
 
       if (locations[i].sub_dir != NULL)
@@ -284,69 +214,66 @@ bool CreateShortcutTask::CreateShortcut() {
   }
 
   bool pin_to_taskbar =
-      shortcut_info_.create_in_quick_launch_bar &&
+      shortcut_info.create_in_quick_launch_bar &&
       (base::win::GetVersion() >= base::win::VERSION_WIN7);
 
   // For Win7's pinning support, any shortcut could be used. So we only create
   // the shortcut file when there is no shortcut file will be created. That is,
   // user only selects "Pin to taskbar".
   if (pin_to_taskbar && shortcut_paths.empty()) {
-    // Creates the shortcut in web_app_path_ in this case.
-    shortcut_paths.push_back(web_app_path_);
+    // Creates the shortcut in web_app_path in this case.
+    shortcut_paths.push_back(web_app_path);
   }
 
   if (shortcut_paths.empty()) {
-    NOTREACHED();
-    return false;
+    return;
   }
 
-  // Ensure web_app_path_ exists.
-  if (!file_util::PathExists(web_app_path_) &&
-      !file_util::CreateDirectory(web_app_path_)) {
-    NOTREACHED();
-    return false;
+  // Ensure web_app_path exists.
+  if (!file_util::PathExists(web_app_path) &&
+      !file_util::CreateDirectory(web_app_path)) {
+    return;
   }
 
   // Generates file name to use with persisted ico and shortcut file.
   FilePath file_name =
-      web_app::internals::GetSanitizedFileName(shortcut_info_.title);
+      web_app::internals::GetSanitizedFileName(shortcut_info.title);
 
   // Creates an ico file to use with shortcut.
-  FilePath icon_file = web_app_path_.Append(file_name).ReplaceExtension(
+  FilePath icon_file = web_app_path.Append(file_name).ReplaceExtension(
       FILE_PATH_LITERAL(".ico"));
   if (!web_app::internals::CheckAndSaveIcon(icon_file,
-                                            shortcut_info_.favicon)) {
-    NOTREACHED();
-    return false;
+                                            shortcut_info.favicon)) {
+    return;
   }
 
   FilePath chrome_exe;
   if (!PathService::Get(base::FILE_EXE, &chrome_exe)) {
-    NOTREACHED();
-    return false;
+    return;
   }
 
   // Working directory.
   FilePath chrome_folder = chrome_exe.DirName();
 
   CommandLine cmd_line =
-     ShellIntegration::CommandLineArgsForLauncher(shortcut_info_.url,
-                                                  shortcut_info_.extension_id);
+     ShellIntegration::CommandLineArgsForLauncher(shortcut_info.url,
+                                                  shortcut_info.extension_id);
   // TODO(evan): we rely on the fact that command_line_string() is
   // properly quoted for a Windows command line.  The method on
   // CommandLine should probably be renamed to better reflect that
   // fact.
-  std::wstring wide_switches(cmd_line.GetCommandLineString());
+  string16 wide_switches(cmd_line.GetCommandLineString());
 
   // Sanitize description
-  if (shortcut_info_.description.length() >= MAX_PATH)
-    shortcut_info_.description.resize(MAX_PATH - 1);
+  string16 description = shortcut_info.description;
+  if (description.length() >= MAX_PATH)
+    description.resize(MAX_PATH - 1);
 
   // Generates app id from web app url and profile path.
   std::string app_name =
-      web_app::GenerateApplicationNameFromInfo(shortcut_info_);
+      web_app::GenerateApplicationNameFromInfo(shortcut_info);
   std::wstring app_id = ShellIntegration::GetAppId(
-      UTF8ToWide(app_name), profile_path_);
+      UTF8ToWide(app_name), profile_path);
 
   FilePath shortcut_to_pin;
 
@@ -367,7 +294,7 @@ bool CreateShortcutTask::CreateShortcut() {
         shortcut_file.value().c_str(),
         chrome_folder.value().c_str(),
         wide_switches.c_str(),
-        shortcut_info_.description.c_str(),
+        description.c_str(),
         icon_file.value().c_str(),
         0,
         app_id.c_str());
@@ -382,15 +309,11 @@ bool CreateShortcutTask::CreateShortcut() {
       success &= file_util::TaskbarPinShortcutLink(
           shortcut_to_pin.value().c_str());
     } else {
-      NOTREACHED();
       success = false;
     }
   }
-
-  return success;
 #else
   NOTIMPLEMENTED();
-  return false;
 #endif
 }
 
@@ -486,10 +409,16 @@ std::string GetExtensionIdFromApplicationName(const std::string& app_name) {
 
 void CreateShortcut(
     const FilePath& data_dir,
-    const ShellIntegration::ShortcutInfo& shortcut_info,
-    CreateShortcutCallback* callback) {
-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-      new CreateShortcutTask(data_dir, shortcut_info, callback));
+    const ShellIntegration::ShortcutInfo& shortcut_info) {
+  BrowserThread::PostTask(
+      BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(&CreateShortcutTask,
+                 web_app::internals::GetWebAppDataDirectory(
+                    web_app::GetDataDir(data_dir),
+                    shortcut_info),
+                 data_dir,
+                 shortcut_info));
 }
 
 bool IsValidUrl(const GURL& url) {

chrome/browser/web_applications/web_app.h

@@ -9,7 +9,6 @@
 #include <string>
 #include <vector>
 
-#include "base/callback_old.h"
 #include "base/file_path.h"
 #include "build/build_config.h"
 #include "chrome/browser/shell_integration.h"
@@ -32,10 +31,6 @@ std::string GenerateApplicationNameFromExtensionId(const std::string& id);
 // Extracts the extension id from the app name.
 std::string GetExtensionIdFromApplicationName(const std::string& app_name);
 
-// Callback after user dismisses CreateShortcutView. "true" indicates
-// shortcut is created successfully. Otherwise, it is false.
-typedef Callback1<bool>::Type CreateShortcutCallback;
-
 // Creates a shortcut for web application based on given shortcut data.
 // |profile_path| is used as root directory for persisted data such as icon.
 // Directory layout is similar to what Gears has, i.e. an web application's
@@ -43,8 +38,7 @@ typedef Callback1<bool>::Type CreateShortcutCallback;
 // |root_dir|.  A crx based app uses a directory named _crx_<app id>.
 void CreateShortcut(
     const FilePath& profile_path,
-    const ShellIntegration::ShortcutInfo& shortcut_info,
-    CreateShortcutCallback* callback);
+    const ShellIntegration::ShortcutInfo& shortcut_info);
 
 // Returns true if given url is a valid web app url.
 bool IsValidUrl(const GURL& url);