Rework FileWatcher to avoid race condition upon deletion.

BUG=none
TEST=none

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

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

7 files changed, 191 insertions, 171 deletions

diff --git a/chrome/browser/file_watcher.h b/chrome/browser/file_watcher.h
index f24be2e..2d4417b 100644
--- a/chrome/browser/file_watcher.h
+++ b/chrome/browser/file_watcher.h
@@ -25,7 +25,10 @@ class FilePath;
 // 2s of the file having changed.
 class FileWatcher {
  public:
-  class Delegate {
+  // Declares the callback client code implements to receive notifications. Note
+  // that implementations of this interface should not keep a reference to the
+  // corresponding FileWatcher object to prevent a reference cycle.
+  class Delegate : public base::RefCountedThreadSafe<Delegate> {
    public:
     virtual ~Delegate() {}
     virtual void OnFileChanged(const FilePath& path) = 0;
diff --git a/chrome/browser/file_watcher_inotify.cc b/chrome/browser/file_watcher_inotify.cc
index c651408..20321d5 100644
--- a/chrome/browser/file_watcher_inotify.cc
+++ b/chrome/browser/file_watcher_inotify.cc
@@ -93,7 +93,7 @@ class FileWatcherImpl : public FileWatcher::PlatformDelegate {
 
  private:
   // Delegate to notify upon changes.
-  FileWatcher::Delegate* delegate_;
+  scoped_refptr<FileWatcher::Delegate> delegate_;
 
   // Watch returned by InotifyReader.
   InotifyReader::Watch watch_;
@@ -104,24 +104,6 @@ class FileWatcherImpl : public FileWatcher::PlatformDelegate {
   DISALLOW_COPY_AND_ASSIGN(FileWatcherImpl);
 };
 
-class FileWatcherImplNotifyTask : public Task {
- public:
-  FileWatcherImplNotifyTask(FileWatcher::Delegate* delegate,
-                            const FilePath& path)
-      : delegate_(delegate), path_(path) {
-  }
-
-  virtual void Run() {
-    delegate_->OnFileChanged(path_);
-  }
-
- private:
-  FileWatcher::Delegate* delegate_;
-  FilePath path_;
-
-  DISALLOW_COPY_AND_ASSIGN(FileWatcherImplNotifyTask);
-};
-
 class InotifyReaderTask : public Task {
  public:
   InotifyReaderTask(InotifyReader* reader, int inotify_fd, int shutdown_fd)
@@ -293,7 +275,8 @@ void FileWatcherImpl::OnInotifyEvent(const inotify_event* event) {
     return;
 
   ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
-      new FileWatcherImplNotifyTask(delegate_, path_));
+      NewRunnableMethod(delegate_.get(), &FileWatcher::Delegate::OnFileChanged,
+                        path_));
 }
 
 bool FileWatcherImpl::Watch(const FilePath& path,
diff --git a/chrome/browser/file_watcher_mac.cc b/chrome/browser/file_watcher_mac.cc
index 638654c..9bd6d2e2 100644
--- a/chrome/browser/file_watcher_mac.cc
+++ b/chrome/browser/file_watcher_mac.cc
@@ -76,7 +76,7 @@ class FileWatcherImpl : public FileWatcher::PlatformDelegate {
 
  private:
   // Delegate to notify upon changes.
-  FileWatcher::Delegate* delegate_;
+  scoped_refptr<FileWatcher::Delegate> delegate_;
 
   // Path we're watching (passed to delegate).
   FilePath path_;
diff --git a/chrome/browser/file_watcher_unittest.cc b/chrome/browser/file_watcher_unittest.cc
index c6fca6c..525e77a 100644
--- a/chrome/browser/file_watcher_unittest.cc
+++ b/chrome/browser/file_watcher_unittest.cc
@@ -15,8 +15,14 @@
 #include "base/scoped_temp_dir.h"
 #include "base/string_util.h"
 #include "base/thread.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using testing::_;
+using testing::AnyNumber;
+using testing::AtLeast;
+using testing::Mock;
+
 #if defined(OS_MACOSX)
 // TODO(tony): Tests are flaky on mac.  http://crbug.com/38188
 #define MAYBE(name) FLAKY_ ## name
@@ -32,21 +38,19 @@ const int kWaitForEventTime = 500;
 // Maximum amount of time to wait on a test.
 const int kMaxTestTimeMs = 10 * 1000;
 
+// A mock FileWatcher::Delegate for testing.
+class TestDelegate : public FileWatcher::Delegate {
+ public:
+  MOCK_METHOD1(OnFileChanged, void(const FilePath&));
+};
+
 class FileWatcherTest : public testing::Test {
  public:
   // Implementation of FileWatcher on Mac requires UI loop.
   FileWatcherTest()
       : loop_(MessageLoop::TYPE_UI),
         ui_thread_(ChromeThread::UI, &loop_),
-        file_thread_(ChromeThread::FILE, &loop_),
-        notified_delegates_(0),
-        expected_notified_delegates_(0) {
-  }
-
-  void OnTestDelegateFirstNotification() {
-    notified_delegates_++;
-    if (notified_delegates_ >= expected_notified_delegates_)
-      MessageLoop::current()->Quit();
+        file_thread_(ChromeThread::FILE, &loop_) {
   }
 
  protected:
@@ -59,6 +63,10 @@ class FileWatcherTest : public testing::Test {
                           kMaxTestTimeMs);
   }
 
+  virtual void TearDown() {
+    loop_.RunAllPending();
+  }
+
   FilePath test_file() {
     return temp_dir_->path().AppendASCII("FileWatcherTest");
   }
@@ -73,24 +81,12 @@ class FileWatcherTest : public testing::Test {
     return write_size == static_cast<int>(content.length());
   }
 
-  void SetExpectedNumberOfNotifiedDelegates(int n) {
-    notified_delegates_ = 0;
-    expected_notified_delegates_ = n;
-  }
-
-  void VerifyExpectedNumberOfNotifiedDelegates() {
+  void VerifyDelegate(TestDelegate* delegate) {
     // Check that we get at least the expected number of notified delegates.
-    if (expected_notified_delegates_ - notified_delegates_ > 0)
-      loop_.Run();
-    EXPECT_EQ(expected_notified_delegates_, notified_delegates_);
-  }
-
-  void VerifyNoExtraNotifications() {
-    // Check that we get no more than the expected number of notified delegates.
     loop_.PostDelayedTask(FROM_HERE, new MessageLoop::QuitTask,
                           kWaitForEventTime);
     loop_.Run();
-    EXPECT_EQ(expected_notified_delegates_, notified_delegates_);
+    Mock::VerifyAndClearExpectations(delegate);
   }
 
   // We need this function for reliable tests on Mac OS X. FSEvents API
@@ -107,54 +103,17 @@ class FileWatcherTest : public testing::Test {
   ChromeThread ui_thread_;
   ChromeThread file_thread_;
   scoped_ptr<ScopedTempDir> temp_dir_;
-
-  // The number of test delegates which received their notification.
-  int notified_delegates_;
-
-  // The number of notified test delegates after which we quit the message loop.
-  int expected_notified_delegates_;
 };
 
-class TestDelegate : public FileWatcher::Delegate {
- public:
-  explicit TestDelegate(FileWatcherTest* test)
-      : test_(test),
-        got_notification_(false) {
-  }
-
-  bool got_notification() const {
-    return got_notification_;
-  }
-
-  void reset() {
-    got_notification_ = false;
-  }
-
-  virtual void OnFileChanged(const FilePath& path) {
-    EXPECT_TRUE(ChromeThread::CurrentlyOn(ChromeThread::UI));
-    if (!got_notification_)
-      test_->OnTestDelegateFirstNotification();
-    got_notification_ = true;
-  }
-
- private:
-  // Hold a pointer to current test fixture to inform it on first notification.
-  FileWatcherTest* test_;
-
-  // Set to true after first notification.
-  bool got_notification_;
-};
-
-
 // Basic test: Create the file and verify that we notice.
 TEST_F(FileWatcherTest, MAYBE(NewFile)) {
   FileWatcher watcher;
-  TestDelegate delegate(this);
-  ASSERT_TRUE(watcher.Watch(test_file(), &delegate));
+  scoped_refptr<TestDelegate> delegate(new TestDelegate);
+  ASSERT_TRUE(watcher.Watch(test_file(), delegate.get()));
 
-  SetExpectedNumberOfNotifiedDelegates(1);
+  EXPECT_CALL(*delegate, OnFileChanged(_)).Times(AtLeast(1));
   ASSERT_TRUE(WriteTestFile("content"));
-  VerifyExpectedNumberOfNotifiedDelegates();
+  VerifyDelegate(delegate.get());
 }
 
 // Verify that modifying the file is caught.
@@ -162,48 +121,46 @@ TEST_F(FileWatcherTest, MAYBE(ModifiedFile)) {
   ASSERT_TRUE(WriteTestFile("content"));
 
   FileWatcher watcher;
-  TestDelegate delegate(this);
-  ASSERT_TRUE(watcher.Watch(test_file(), &delegate));
+  scoped_refptr<TestDelegate> delegate(new TestDelegate);
+  ASSERT_TRUE(watcher.Watch(test_file(), delegate.get()));
 
   // Now make sure we get notified if the file is modified.
-  SetExpectedNumberOfNotifiedDelegates(1);
+  EXPECT_CALL(*delegate, OnFileChanged(_)).Times(AtLeast(1));
   ASSERT_TRUE(WriteTestFile("new content"));
-  VerifyExpectedNumberOfNotifiedDelegates();
+  VerifyDelegate(delegate.get());
 }
 
 TEST_F(FileWatcherTest, MAYBE(DeletedFile)) {
   ASSERT_TRUE(WriteTestFile("content"));
 
   FileWatcher watcher;
-  TestDelegate delegate(this);
-  ASSERT_TRUE(watcher.Watch(test_file(), &delegate));
+  scoped_refptr<TestDelegate> delegate(new TestDelegate);
+  ASSERT_TRUE(watcher.Watch(test_file(), delegate.get()));
 
   // Now make sure we get notified if the file is deleted.
-  SetExpectedNumberOfNotifiedDelegates(1);
+  EXPECT_CALL(*delegate, OnFileChanged(_)).Times(AtLeast(1));
   file_util::Delete(test_file(), false);
   SyncIfPOSIX();
-  VerifyExpectedNumberOfNotifiedDelegates();
+  VerifyDelegate(delegate.get());
 }
 
 // Verify that letting the watcher go out of scope stops notifications.
 TEST_F(FileWatcherTest, MAYBE(Unregister)) {
-  TestDelegate delegate(this);
+  scoped_refptr<TestDelegate> delegate(new TestDelegate);
 
   {
     FileWatcher watcher;
-    ASSERT_TRUE(watcher.Watch(test_file(), &delegate));
+    ASSERT_TRUE(watcher.Watch(test_file(), delegate.get()));
 
     // And then let it fall out of scope, clearing its watch.
   }
 
   // Write a file to the test dir.
-  SetExpectedNumberOfNotifiedDelegates(0);
+  EXPECT_CALL(*delegate, OnFileChanged(_)).Times(0);
   ASSERT_TRUE(WriteTestFile("content"));
-  VerifyExpectedNumberOfNotifiedDelegates();
-  VerifyNoExtraNotifications();
+  VerifyDelegate(delegate.get());
 }
 
-
 namespace {
 // Used by the DeleteDuringNotify test below.
 // Deletes the FileWatcher when it's notified.
@@ -227,26 +184,52 @@ class Deleter : public FileWatcher::Delegate {
 // Verify that deleting a watcher during the callback doesn't crash.
 TEST_F(FileWatcherTest, MAYBE(DeleteDuringNotify)) {
   FileWatcher* watcher = new FileWatcher;
-  Deleter deleter(watcher, &loop_);  // Takes ownership of watcher.
-  ASSERT_TRUE(watcher->Watch(test_file(), &deleter));
+  // Takes ownership of watcher.
+  scoped_refptr<Deleter> deleter(new Deleter(watcher, &loop_));
+  ASSERT_TRUE(watcher->Watch(test_file(), deleter.get()));
 
   ASSERT_TRUE(WriteTestFile("content"));
   loop_.Run();
 
   // We win if we haven't crashed yet.
   // Might as well double-check it got deleted, too.
-  ASSERT_TRUE(deleter.watcher_.get() == NULL);
+  ASSERT_TRUE(deleter->watcher_.get() == NULL);
+}
+
+// Verify that deleting the watcher works even if there is a pending
+// notification.
+//
+// It's hard to test this, since both a change event and deletion of the file
+// watcher must happen before the task that runs the callback executes. The code
+// below only triggers the situation with the Linux implementation. Change
+// detection runs on a separate thread in the Linux implementation, so we can
+// schedule the FileWatcher deletion in advance. For Mac and Windows, the
+// DeleteTask runs before the message loop processes the platform-specific
+// change notifications, so the whole FileWatcher is destroyed before the
+// callback gets scheduled.
+TEST_F(FileWatcherTest, MAYBE(DestroyWithPendingNotification)) {
+  scoped_refptr<TestDelegate> delegate(new TestDelegate);
+  EXPECT_CALL(*delegate, OnFileChanged(_)).Times(AnyNumber());
+  FileWatcher* watcher = new FileWatcher;
+  ASSERT_TRUE(watcher->Watch(test_file(), delegate.get()));
+  ASSERT_TRUE(WriteTestFile("content"));
+  ChromeThread::DeleteSoon(ChromeThread::FILE, FROM_HERE, watcher);
+  // Success if there is no crash or DCHECK when running the callback.
+  VerifyDelegate(delegate.get());
 }
 
 TEST_F(FileWatcherTest, MAYBE(MultipleWatchersSingleFile)) {
   FileWatcher watcher1, watcher2;
-  TestDelegate delegate1(this), delegate2(this);
-  ASSERT_TRUE(watcher1.Watch(test_file(), &delegate1));
-  ASSERT_TRUE(watcher2.Watch(test_file(), &delegate2));
+  scoped_refptr<TestDelegate> delegate1(new TestDelegate);
+  scoped_refptr<TestDelegate> delegate2(new TestDelegate);
+  ASSERT_TRUE(watcher1.Watch(test_file(), delegate1.get()));
+  ASSERT_TRUE(watcher2.Watch(test_file(), delegate2.get()));
 
-  SetExpectedNumberOfNotifiedDelegates(2);
+  EXPECT_CALL(*delegate1, OnFileChanged(_)).Times(AtLeast(1));
+  EXPECT_CALL(*delegate2, OnFileChanged(_)).Times(AtLeast(1));
   ASSERT_TRUE(WriteTestFile("content"));
-  VerifyExpectedNumberOfNotifiedDelegates();
+  VerifyDelegate(delegate1.get());
+  VerifyDelegate(delegate2.get());
 }
 
 // Verify that watching a file who's parent directory doesn't exist
diff --git a/chrome/browser/file_watcher_win.cc b/chrome/browser/file_watcher_win.cc
index 7d19b32..d4ec0ed 100644
--- a/chrome/browser/file_watcher_win.cc
+++ b/chrome/browser/file_watcher_win.cc
@@ -27,7 +27,7 @@ class FileWatcherImpl : public FileWatcher::PlatformDelegate,
   virtual ~FileWatcherImpl();
 
   // Delegate to notify upon changes.
-  FileWatcher::Delegate* delegate_;
+  scoped_refptr<FileWatcher::Delegate> delegate_;
 
   // Path we're watching (passed to delegate).
   FilePath path_;
diff --git a/chrome/browser/user_style_sheet_watcher.cc b/chrome/browser/user_style_sheet_watcher.cc
index 2bc9568..3743820 100644
--- a/chrome/browser/user_style_sheet_watcher.cc
+++ b/chrome/browser/user_style_sheet_watcher.cc
@@ -18,58 +18,88 @@ const char kUserStyleSheetFile[] = "Custom.css";
 
 }  // namespace
 
-UserStyleSheetWatcher::UserStyleSheetWatcher(const FilePath& profile_path)
-    : profile_path_(profile_path),
-      has_loaded_(false) {
-  // Listen for when the first render view host is created.  If we load
-  // too fast, the first tab won't hear the notification and won't get
-  // the user style sheet.
-  registrar_.Add(this, NotificationType::RENDER_VIEW_HOST_CREATED_FOR_TAB,
-                 NotificationService::AllSources());
-}
+// UserStyleSheetLoader is responsible for loading  the user style sheet on the
+// file thread and sends a notification when the style sheet is loaded. It is
+// a helper to UserStyleSheetWatcher. The reference graph is as follows:
+//
+// .-----------------------.    owns    .-------------.
+// | UserStyleSheetWatcher |----------->| FileWatcher |
+// '-----------------------'            '-------------'
+//             |                               |
+//             V                               |
+//  .----------------------.                   |
+//  | UserStyleSheetLoader |<------------------'
+//  '----------------------'
+//
+// FileWatcher's reference to UserStyleSheetLoader is used for delivering the
+// change notifications. Since they happen asynchronously, UserStyleSheetWatcher
+// and its FileWatcher may be destroyed while a callback to UserStyleSheetLoader
+// is in progress, in which case the UserStyleSheetLoader object outlives the
+// watchers.
+class UserStyleSheetLoader : public FileWatcher::Delegate {
+ public:
+  UserStyleSheetLoader();
+  virtual ~UserStyleSheetLoader() {}
+
+  GURL user_style_sheet() const {
+    return user_style_sheet_;
+  }
 
-void UserStyleSheetWatcher::Observe(NotificationType type,
-    const NotificationSource& source, const NotificationDetails& details) {
-  DCHECK(type == NotificationType::RENDER_VIEW_HOST_CREATED_FOR_TAB);
+  // Load the user style sheet on the file thread and convert it to a
+  // base64 URL.  Posts the base64 URL back to the UI thread.
+  void LoadStyleSheet(const FilePath& style_sheet_file);
+
+  // Send out a notification if the stylesheet has already been loaded.
+  void NotifyLoaded();
+
+  // FileWatcher::Delegate interface
+  virtual void OnFileChanged(const FilePath& path);
 
+ private:
+  // Called on the UI thread after the stylesheet has loaded.
+  void SetStyleSheet(const GURL& url);
+
+  // The user style sheet as a base64 data:// URL.
+  GURL user_style_sheet_;
+
+  // Whether the stylesheet has been loaded.
+  bool has_loaded_;
+
+  DISALLOW_COPY_AND_ASSIGN(UserStyleSheetLoader);
+};
+
+UserStyleSheetLoader::UserStyleSheetLoader()
+    : has_loaded_(false) {
+}
+
+void UserStyleSheetLoader::NotifyLoaded() {
   if (has_loaded_) {
     NotificationService::current()->Notify(
         NotificationType::USER_STYLE_SHEET_UPDATED,
-        Source<UserStyleSheetWatcher>(this),
+        Source<UserStyleSheetLoader>(this),
         NotificationService::NoDetails());
   }
-
-  registrar_.RemoveAll();
-}
-
-void UserStyleSheetWatcher::OnFileChanged(const FilePath& path) {
-  ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
-      NewRunnableMethod(this, &UserStyleSheetWatcher::LoadStyleSheet,
-                        profile_path_));
 }
 
-void UserStyleSheetWatcher::Init() {
-  ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
-      NewRunnableMethod(this, &UserStyleSheetWatcher::LoadStyleSheet,
-                        profile_path_));
+void UserStyleSheetLoader::OnFileChanged(const FilePath& path) {
+  LoadStyleSheet(path);
 }
 
-void UserStyleSheetWatcher::LoadStyleSheet(const FilePath& profile_path) {
+void UserStyleSheetLoader::LoadStyleSheet(const FilePath& style_sheet_file) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
   // We keep the user style sheet in a subdir so we can watch for changes
   // to the file.
-  FilePath style_sheet_dir = profile_path.AppendASCII(kStyleSheetDir);
+  FilePath style_sheet_dir = style_sheet_file.DirName();
   if (!file_util::DirectoryExists(style_sheet_dir)) {
     if (!file_util::CreateDirectory(style_sheet_dir))
       return;
   }
   // Create the file if it doesn't exist.
-  FilePath css_file = style_sheet_dir.AppendASCII(kUserStyleSheetFile);
-  if (!file_util::PathExists(css_file))
-    file_util::WriteFile(css_file, "", 0);
+  if (!file_util::PathExists(style_sheet_file))
+    file_util::WriteFile(style_sheet_file, "", 0);
 
   std::string css;
-  bool rv = file_util::ReadFileToString(css_file, &css);
+  bool rv = file_util::ReadFileToString(style_sheet_file, &css);
   GURL style_sheet_url;
   if (rv && !css.empty()) {
     std::string css_base64;
@@ -81,23 +111,56 @@ void UserStyleSheetWatcher::LoadStyleSheet(const FilePath& profile_path) {
     }
   }
   ChromeThread::PostTask(ChromeThread::UI, FROM_HERE,
-      NewRunnableMethod(this, &UserStyleSheetWatcher::SetStyleSheet,
+      NewRunnableMethod(this, &UserStyleSheetLoader::SetStyleSheet,
                         style_sheet_url));
+}
+
+void UserStyleSheetLoader::SetStyleSheet(const GURL& url) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+
+  has_loaded_ = true;
+  user_style_sheet_ = url;
+  NotifyLoaded();
+}
+
+UserStyleSheetWatcher::UserStyleSheetWatcher(const FilePath& profile_path)
+    : profile_path_(profile_path),
+      loader_(new UserStyleSheetLoader) {
+  // Listen for when the first render view host is created.  If we load
+  // too fast, the first tab won't hear the notification and won't get
+  // the user style sheet.
+  registrar_.Add(this, NotificationType::RENDER_VIEW_HOST_CREATED_FOR_TAB,
+                 NotificationService::AllSources());
+}
+
+UserStyleSheetWatcher::~UserStyleSheetWatcher() {
+}
+
+void UserStyleSheetWatcher::Init() {
+  // Make sure we run on the file thread.
+  if (!ChromeThread::CurrentlyOn(ChromeThread::FILE)) {
+    ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
+        NewRunnableMethod(this, &UserStyleSheetWatcher::Init));
+    return;
+  }
 
   if (!file_watcher_.get()) {
     file_watcher_.reset(new FileWatcher);
     file_watcher_->Watch(profile_path_.AppendASCII(kStyleSheetDir)
-                         .AppendASCII(kUserStyleSheetFile), this);
+                         .AppendASCII(kUserStyleSheetFile), loader_.get());
+    FilePath style_sheet_file = profile_path_.AppendASCII(kStyleSheetDir)
+                                             .AppendASCII(kUserStyleSheetFile);
+    loader_->LoadStyleSheet(style_sheet_file);
   }
 }
 
-void UserStyleSheetWatcher::SetStyleSheet(const GURL& url) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+GURL UserStyleSheetWatcher::user_style_sheet() const {
+  return loader_->user_style_sheet();
+}
 
-  has_loaded_ = true;
-  user_style_sheet_ = url;
-  NotificationService::current()->Notify(
-      NotificationType::USER_STYLE_SHEET_UPDATED,
-      Source<UserStyleSheetWatcher>(this),
-      NotificationService::NoDetails());
+void UserStyleSheetWatcher::Observe(NotificationType type,
+    const NotificationSource& source, const NotificationDetails& details) {
+  DCHECK(type == NotificationType::RENDER_VIEW_HOST_CREATED_FOR_TAB);
+  loader_->NotifyLoaded();
+  registrar_.RemoveAll();
 }
diff --git a/chrome/browser/user_style_sheet_watcher.h b/chrome/browser/user_style_sheet_watcher.h
index d029b4f..c4974de 100644
--- a/chrome/browser/user_style_sheet_watcher.h
+++ b/chrome/browser/user_style_sheet_watcher.h
@@ -7,7 +7,6 @@
 #pragma once
 
 #include "base/file_path.h"
-#include "base/logging.h"
 #include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "chrome/browser/chrome_thread.h"
@@ -16,49 +15,38 @@
 #include "chrome/common/notification_registrar.h"
 #include "googleurl/src/gurl.h"
 
-// This loads the user style sheet on the file thread and sends a notification
-// when the style sheet is loaded.
+class UserStyleSheetLoader;
+
+// Watches the user style sheet file and triggers reloads on the file thread
+// whenever the file changes.
 class UserStyleSheetWatcher
     : public base::RefCountedThreadSafe<UserStyleSheetWatcher,
                                         ChromeThread::DeleteOnUIThread>,
-      public NotificationObserver,
-      public FileWatcher::Delegate {
+      public NotificationObserver {
  public:
   explicit UserStyleSheetWatcher(const FilePath& profile_path);
-  virtual ~UserStyleSheetWatcher() {}
+  virtual ~UserStyleSheetWatcher();
 
   void Init();
 
-  GURL user_style_sheet() const {
-    return user_style_sheet_;
-  }
+  GURL user_style_sheet() const;
 
   // NotificationObserver interface
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
-  // FileWatcher::Delegate interface
-  virtual void OnFileChanged(const FilePath& path);
-
  private:
-  // Load the user style sheet on the file thread and convert it to a
-  // base64 URL.  Posts the base64 URL back to the UI thread.
-  void LoadStyleSheet(const FilePath& profile_path);
-
-  void SetStyleSheet(const GURL& url);
-
   // The directory containing User StyleSheets/Custom.css.
   FilePath profile_path_;
 
-  // The user style sheet as a base64 data:// URL.
-  GURL user_style_sheet_;
+  // The loader object.
+  scoped_refptr<UserStyleSheetLoader> loader_;
 
   // Watches for changes to the css file so we can reload the style sheet.
   scoped_ptr<FileWatcher> file_watcher_;
 
   NotificationRegistrar registrar_;
-  bool has_loaded_;
 
   DISALLOW_COPY_AND_ASSIGN(UserStyleSheetWatcher);
 };