Change AddList to take a completion callback.

 AddList now takes a completion callback. This callback is invoked when
 distillation is complete with the status of distillation. Clients can
 add an item to the list and then use callback to update their state
 when distillation finishes or gets cancelled.

 BUG=288015

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=241768

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

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

13 files changed, 335 insertions, 86 deletions

diff --git a/components/dom_distiller/core/dom_distiller_service.cc b/components/dom_distiller/core/dom_distiller_service.cc
index fe840b0..65791c4f 100644
--- a/components/dom_distiller/core/dom_distiller_service.cc
+++ b/components/dom_distiller/core/dom_distiller_service.cc
@@ -24,6 +24,14 @@ ArticleEntry CreateSkeletonEntryForUrl(const GURL& url) {
   return skeleton;
 }
 
+void RunArticleAvailableCallback(
+    const DomDistillerService::ArticleAvailableCallback& article_cb,
+    const ArticleEntry& entry,
+    DistilledPageProto* proto,
+    bool distillation_succeeded) {
+  article_cb.Run(distillation_succeeded);
+}
+
 }  // namespace
 
 DomDistillerService::DomDistillerService(
@@ -37,14 +45,42 @@ syncer::SyncableService* DomDistillerService::GetSyncableService() const {
   return store_->GetSyncableService();
 }
 
-void DomDistillerService::AddToList(const GURL& url) {
-  if (store_->GetEntryByUrl(url, NULL)) {
-    return;
+const std::string DomDistillerService::AddToList(
+    const GURL& url,
+    const ArticleAvailableCallback& article_cb) {
+  ArticleEntry entry;
+  const bool is_already_added = store_->GetEntryByUrl(url, &entry);
+
+  TaskTracker* task_tracker;
+  if (is_already_added) {
+    task_tracker = GetTaskTrackerForEntry(entry);
+    if (task_tracker == NULL) {
+      // Entry is in the store but there is no task tracker. This could
+      // happen when distillation has already completed. For now just return
+      // true.
+      // TODO(shashishekhar): Change this to check if article is available,
+      // An article may not be available for a variety of reasons, e.g.
+      // distillation failure or blobs not available locally.
+      base::MessageLoop::current()->PostTask(FROM_HERE,
+                                             base::Bind(article_cb, true));
+      return entry.entry_id();
+    }
+  } else {
+    task_tracker = GetOrCreateTaskTrackerForUrl(url);
   }
-  TaskTracker* task_tracker = GetTaskTrackerForUrl(url);
-  task_tracker->SetSaveCallback(base::Bind(
-      &DomDistillerService::AddDistilledPageToList, base::Unretained(this)));
-  task_tracker->StartDistiller(distiller_factory_.get());
+
+  if (!article_cb.is_null()) {
+    task_tracker->AddSaveCallback(
+        base::Bind(&RunArticleAvailableCallback, article_cb));
+  }
+
+  if (!is_already_added) {
+    task_tracker->AddSaveCallback(base::Bind(
+        &DomDistillerService::AddDistilledPageToList, base::Unretained(this)));
+    task_tracker->StartDistiller(distiller_factory_.get());
+  }
+
+  return task_tracker->GetEntryId();
 }
 
 std::vector<ArticleEntry> DomDistillerService::GetEntries() const {
@@ -57,6 +93,12 @@ void DomDistillerService::RemoveEntry(
   if (!store_->GetEntryById(entry_id, &entry)) {
     return;
   }
+
+  TaskTracker* task_tracker = GetTaskTrackerForEntry(entry);
+  if (task_tracker != NULL) {
+    task_tracker->CancelSaveCallbacks();
+  }
+
   store_->RemoveEntry(entry);
 }
 
@@ -68,7 +110,7 @@ scoped_ptr<ViewerHandle> DomDistillerService::ViewEntry(
     return scoped_ptr<ViewerHandle>();
   }
 
-  TaskTracker* task_tracker = GetTaskTrackerForEntry(entry);
+  TaskTracker* task_tracker = GetOrCreateTaskTrackerForEntry(entry);
   scoped_ptr<ViewerHandle> viewer_handle = task_tracker->AddViewer(delegate);
   task_tracker->StartDistiller(distiller_factory_.get());
 
@@ -82,17 +124,18 @@ scoped_ptr<ViewerHandle> DomDistillerService::ViewUrl(
     return scoped_ptr<ViewerHandle>();
   }
 
-  TaskTracker* task_tracker = GetTaskTrackerForUrl(url);
+  TaskTracker* task_tracker = GetOrCreateTaskTrackerForUrl(url);
   scoped_ptr<ViewerHandle> viewer_handle = task_tracker->AddViewer(delegate);
   task_tracker->StartDistiller(distiller_factory_.get());
 
   return viewer_handle.Pass();
 }
 
-TaskTracker* DomDistillerService::GetTaskTrackerForUrl(const GURL& url) {
+TaskTracker* DomDistillerService::GetOrCreateTaskTrackerForUrl(
+    const GURL& url) {
   ArticleEntry entry;
   if (store_->GetEntryByUrl(url, &entry)) {
-    return GetTaskTrackerForEntry(entry);
+    return GetOrCreateTaskTrackerForEntry(entry);
   }
 
   for (TaskList::iterator it = tasks_.begin(); it != tasks_.end(); ++it) {
@@ -108,16 +151,22 @@ TaskTracker* DomDistillerService::GetTaskTrackerForUrl(const GURL& url) {
 }
 
 TaskTracker* DomDistillerService::GetTaskTrackerForEntry(
-    const ArticleEntry& entry) {
+    const ArticleEntry& entry) const {
   const std::string& entry_id = entry.entry_id();
-  for (TaskList::iterator it = tasks_.begin(); it != tasks_.end(); ++it) {
+  for (TaskList::const_iterator it = tasks_.begin(); it != tasks_.end(); ++it) {
     if ((*it)->HasEntryId(entry_id)) {
       return *it;
     }
   }
+  return NULL;
+}
 
-  TaskTracker* task_tracker = CreateTaskTracker(entry);
-
+TaskTracker* DomDistillerService::GetOrCreateTaskTrackerForEntry(
+    const ArticleEntry& entry) {
+  TaskTracker* task_tracker = GetTaskTrackerForEntry(entry);
+  if (task_tracker == NULL) {
+    task_tracker = CreateTaskTracker(entry);
+  }
   return task_tracker;
 }
 
@@ -138,11 +187,13 @@ void DomDistillerService::CancelTask(TaskTracker* task) {
 }
 
 void DomDistillerService::AddDistilledPageToList(const ArticleEntry& entry,
-                                                 DistilledPageProto* proto) {
+                                                 DistilledPageProto* proto,
+                                                 bool distillation_succeeded) {
   DCHECK(IsEntryValid(entry));
-  DCHECK(proto);
-
-  store_->UpdateEntry(entry);
+  if (distillation_succeeded) {
+    DCHECK(proto);
+    store_->UpdateEntry(entry);
+  }
 }
 
 void DomDistillerService::AddObserver(DomDistillerObserver* observer) {
diff --git a/components/dom_distiller/core/dom_distiller_service.h b/components/dom_distiller/core/dom_distiller_service.h
index ce4f266..d730cf6 100644
--- a/components/dom_distiller/core/dom_distiller_service.h
+++ b/components/dom_distiller/core/dom_distiller_service.h
@@ -8,6 +8,7 @@
 #include <string>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/memory/weak_ptr.h"
@@ -32,6 +33,8 @@ class ViewRequestDelegate;
 // Provide a view of the article list and ways of interacting with it.
 class DomDistillerService {
  public:
+  typedef base::Callback<void(bool)> ArticleAvailableCallback;
+
   DomDistillerService(scoped_ptr<DomDistillerStoreInterface> store,
                       scoped_ptr<DistillerFactory> distiller_factory);
   ~DomDistillerService();
@@ -39,8 +42,10 @@ class DomDistillerService {
   syncer::SyncableService* GetSyncableService() const;
 
   // Distill the article at |url| and add the resulting entry to the DOM
-  // distiller list.
-  void AddToList(const GURL& url);
+  // distiller list. |article_cb| is invoked with true if article is
+  // available offline.
+  const std::string AddToList(const GURL& url,
+                              const ArticleAvailableCallback& article_cb);
 
   // Gets the full list of entries.
   std::vector<ArticleEntry> GetEntries() const;
@@ -65,15 +70,18 @@ class DomDistillerService {
  private:
   void CancelTask(TaskTracker* task);
   void AddDistilledPageToList(const ArticleEntry& entry,
-                              DistilledPageProto* proto);
+                              DistilledPageProto* proto,
+                              bool distillation_succeeded);
 
   TaskTracker* CreateTaskTracker(const ArticleEntry& entry);
 
+  TaskTracker* GetTaskTrackerForEntry(const ArticleEntry& entry) const;
+
   // Gets the task tracker for the given |url| or |entry|. If no appropriate
   // tracker exists, this will create one, initialize it, and add it to
   // |tasks_|.
-  TaskTracker* GetTaskTrackerForUrl(const GURL& url);
-  TaskTracker* GetTaskTrackerForEntry(const ArticleEntry& entry);
+  TaskTracker* GetOrCreateTaskTrackerForUrl(const GURL& url);
+  TaskTracker* GetOrCreateTaskTrackerForEntry(const ArticleEntry& entry);
 
   scoped_ptr<DomDistillerStoreInterface> store_;
   scoped_ptr<DistillerFactory> distiller_factory_;
diff --git a/components/dom_distiller/core/dom_distiller_service_unittest.cc b/components/dom_distiller/core/dom_distiller_service_unittest.cc
index c2b8dbc..e010821 100644
--- a/components/dom_distiller/core/dom_distiller_service_unittest.cc
+++ b/components/dom_distiller/core/dom_distiller_service_unittest.cc
@@ -39,6 +39,23 @@ class MockDistillerObserver : public DomDistillerObserver {
   virtual ~MockDistillerObserver() {}
 };
 
+class MockArticleAvailableCallback {
+ public:
+  MOCK_METHOD1(DistillationCompleted, void(bool));
+};
+
+DomDistillerService::ArticleAvailableCallback ArticleCallback(
+    MockArticleAvailableCallback* callback) {
+  return base::Bind(&MockArticleAvailableCallback::DistillationCompleted,
+                    base::Unretained(callback));
+}
+
+void RunDistillerCallback(FakeDistiller* distiller,
+                          scoped_ptr<DistilledPageProto> proto) {
+  distiller->RunDistillerCallback(proto.Pass());
+  base::RunLoop().RunUntilIdle();
+}
+
 }  // namespace
 
 class DomDistillerServiceTest : public testing::Test {
@@ -94,7 +111,7 @@ TEST_F(DomDistillerServiceTest, TestViewEntry) {
   scoped_ptr<DistilledPageProto> proto(new DistilledPageProto);
   EXPECT_CALL(viewer_delegate, OnArticleReady(proto.get()));
 
-  distiller->RunDistillerCallback(proto.Pass());
+  RunDistillerCallback(distiller, proto.Pass());
 }
 
 TEST_F(DomDistillerServiceTest, TestViewUrl) {
@@ -112,7 +129,7 @@ TEST_F(DomDistillerServiceTest, TestViewUrl) {
   scoped_ptr<DistilledPageProto> proto(new DistilledPageProto);
   EXPECT_CALL(viewer_delegate, OnArticleReady(proto.get()));
 
-  distiller->RunDistillerCallback(proto.Pass());
+  RunDistillerCallback(distiller, proto.Pass());
 }
 
 TEST_F(DomDistillerServiceTest, TestMultipleViewUrl) {
@@ -137,7 +154,7 @@ TEST_F(DomDistillerServiceTest, TestMultipleViewUrl) {
   scoped_ptr<DistilledPageProto> proto(new DistilledPageProto);
   EXPECT_CALL(viewer_delegate, OnArticleReady(proto.get()));
 
-  distiller->RunDistillerCallback(proto.Pass());
+  RunDistillerCallback(distiller, proto.Pass());
 
   ASSERT_FALSE(distiller2->GetCallback().is_null());
   EXPECT_EQ(url2, distiller2->GetUrl());
@@ -145,7 +162,7 @@ TEST_F(DomDistillerServiceTest, TestMultipleViewUrl) {
   scoped_ptr<DistilledPageProto> proto2(new DistilledPageProto);
   EXPECT_CALL(viewer_delegate2, OnArticleReady(proto2.get()));
 
-  distiller2->RunDistillerCallback(proto2.Pass());
+  RunDistillerCallback(distiller2, proto2.Pass());
 }
 
 TEST_F(DomDistillerServiceTest, TestViewUrlCancelled) {
@@ -180,21 +197,30 @@ TEST_F(DomDistillerServiceTest, TestAddAndRemoveEntry) {
 
   GURL url("http://www.example.com/p1");
 
-  service_->AddToList(url);
+  MockArticleAvailableCallback article_cb;
+  EXPECT_CALL(article_cb, DistillationCompleted(true));
+
+  std::string entry_id = service_->AddToList(url, ArticleCallback(&article_cb));
+
+  ArticleEntry entry;
+  EXPECT_TRUE(store_->GetEntryByUrl(url, &entry));
+  EXPECT_EQ(entry.entry_id(), entry_id);
 
   ASSERT_FALSE(distiller->GetCallback().is_null());
   EXPECT_EQ(url, distiller->GetUrl());
 
-  ArticleEntry entry;
+  scoped_ptr<DistilledPageProto> proto(new DistilledPageProto);
+  RunDistillerCallback(distiller, proto.Pass());
+
   EXPECT_TRUE(store_->GetEntryByUrl(url, &entry));
   EXPECT_EQ(1u, store_->GetEntries().size());
-  store_->RemoveEntry(entry);
+  service_->RemoveEntry(entry_id);
+  base::RunLoop().RunUntilIdle();
   EXPECT_EQ(0u, store_->GetEntries().size());
 }
 
-TEST_F(DomDistillerServiceTest, TestObserverIntegration) {
+TEST_F(DomDistillerServiceTest, TestCancellation) {
   FakeDistiller* distiller = new FakeDistiller();
-
   MockDistillerObserver observer;
   service_->AddObserver(&observer);
 
@@ -202,24 +228,128 @@ TEST_F(DomDistillerServiceTest, TestObserverIntegration) {
       .WillOnce(Return(distiller));
   EXPECT_CALL(observer, ArticleEntriesUpdated(_));
 
+  MockArticleAvailableCallback article_cb;
+  EXPECT_CALL(article_cb, DistillationCompleted(false));
+
   GURL url("http://www.example.com/p1");
+  std::string entry_id = service_->AddToList(url, ArticleCallback(&article_cb));
 
-  service_->AddToList(url);
+  // Just remove the entry, there should only be a REMOVE update and no UPDATE
+  // update.
+  std::vector<DomDistillerObserver::ArticleUpdate> expected_updates;
+  DomDistillerObserver::ArticleUpdate update;
+  update.entry_id = entry_id;
+  update.update_type = DomDistillerObserver::ArticleUpdate::REMOVE;
+  expected_updates.push_back(update);
+  EXPECT_CALL(
+      observer,
+      ArticleEntriesUpdated(util::HasExpectedUpdates(expected_updates)));
 
-  ArticleEntry entry;
-  EXPECT_TRUE(store_->GetEntryByUrl(url, &entry));
+  // Remove entry will post a cancellation task.
+  service_->RemoveEntry(entry_id);
+  base::RunLoop().RunUntilIdle();
+}
+
+TEST_F(DomDistillerServiceTest, TestMultipleObservers) {
+  FakeDistiller* distiller = new FakeDistiller();
+  EXPECT_CALL(*distiller_factory_, CreateDistillerImpl())
+      .WillOnce(Return(distiller));
+
+  const int kObserverCount = 5;
+  MockDistillerObserver observers[kObserverCount];
+  for (int i = 0; i < kObserverCount; ++i) {
+    service_->AddObserver(&observers[i]);
+    EXPECT_CALL(observers[i], ArticleEntriesUpdated(_));
+  }
+
+  DomDistillerService::ArticleAvailableCallback article_cb;
+  GURL url("http://www.example.com/p1");
+  std::string entry_id = service_->AddToList(url, article_cb);
+
+  // Distillation should notify all observers that article is updated.
   std::vector<DomDistillerObserver::ArticleUpdate> expected_updates;
   DomDistillerObserver::ArticleUpdate update;
-  update.entry_id = entry.entry_id();
+  update.entry_id = entry_id;
   update.update_type = DomDistillerObserver::ArticleUpdate::UPDATE;
   expected_updates.push_back(update);
 
-  EXPECT_CALL(
-      observer,
-      ArticleEntriesUpdated(test::util::HasExpectedUpdates(expected_updates)));
+  for (int i = 0; i < kObserverCount; ++i) {
+    EXPECT_CALL(
+        observers[i],
+        ArticleEntriesUpdated(util::HasExpectedUpdates(expected_updates)));
+  }
 
   scoped_ptr<DistilledPageProto> proto(new DistilledPageProto);
-  distiller->RunDistillerCallback(proto.Pass());
+  RunDistillerCallback(distiller, proto.Pass());
+
+  // Remove should notify all observers that article is removed.
+  update.update_type = DomDistillerObserver::ArticleUpdate::REMOVE;
+  expected_updates.clear();
+  expected_updates.push_back(update);
+  for (int i = 0; i < kObserverCount; ++i) {
+    EXPECT_CALL(
+        observers[i],
+        ArticleEntriesUpdated(util::HasExpectedUpdates(expected_updates)));
+  }
+
+  service_->RemoveEntry(entry_id);
+  base::RunLoop().RunUntilIdle();
+}
+
+TEST_F(DomDistillerServiceTest, TestMultipleCallbacks) {
+  FakeDistiller* distiller = new FakeDistiller();
+  EXPECT_CALL(*distiller_factory_, CreateDistillerImpl())
+      .WillOnce(Return(distiller));
+
+  const int kClientsCount = 5;
+  MockArticleAvailableCallback article_cb[kClientsCount];
+  // Adding a URL and then distilling calls all clients.
+  GURL url("http://www.example.com/p1");
+  const std::string entry_id =
+      service_->AddToList(url, ArticleCallback(&article_cb[0]));
+  EXPECT_CALL(article_cb[0], DistillationCompleted(true));
+
+  for (int i = 1; i < kClientsCount; ++i) {
+    EXPECT_EQ(entry_id,
+              service_->AddToList(url, ArticleCallback(&article_cb[i])));
+    EXPECT_CALL(article_cb[i], DistillationCompleted(true));
+  }
+
+  scoped_ptr<DistilledPageProto> proto(new DistilledPageProto);
+  RunDistillerCallback(distiller, proto.Pass());
+
+  // Add the same url again, all callbacks should be called with true.
+  for (int i = 0; i < kClientsCount; ++i) {
+    EXPECT_CALL(article_cb[i], DistillationCompleted(true));
+    EXPECT_EQ(entry_id,
+              service_->AddToList(url, ArticleCallback(&article_cb[i])));
+  }
+
+  base::RunLoop().RunUntilIdle();
+}
+
+TEST_F(DomDistillerServiceTest, TestMultipleCallbacksOnRemove) {
+  FakeDistiller* distiller = new FakeDistiller();
+  EXPECT_CALL(*distiller_factory_, CreateDistillerImpl())
+      .WillOnce(Return(distiller));
+
+  const int kClientsCount = 5;
+  MockArticleAvailableCallback article_cb[kClientsCount];
+  // Adding a URL and remove the entry before distillation. Callback should be
+  // called with false.
+  GURL url("http://www.example.com/p1");
+  const std::string entry_id =
+      service_->AddToList(url, ArticleCallback(&article_cb[0]));
+
+  EXPECT_CALL(article_cb[0], DistillationCompleted(false));
+  for (int i = 1; i < kClientsCount; ++i) {
+    EXPECT_EQ(entry_id,
+              service_->AddToList(url, ArticleCallback(&article_cb[i])));
+    EXPECT_CALL(article_cb[i], DistillationCompleted(false));
+  }
+
+  service_->RemoveEntry(entry_id);
+  base::RunLoop().RunUntilIdle();
 }
 
 }  // namespace test
diff --git a/components/dom_distiller/core/dom_distiller_test_util.cc b/components/dom_distiller/core/dom_distiller_test_util.cc
index 55ede69..99ed3bc 100644
--- a/components/dom_distiller/core/dom_distiller_test_util.cc
+++ b/components/dom_distiller/core/dom_distiller_test_util.cc
@@ -62,7 +62,6 @@ void ObserverUpdatesMatcher::DescribeUpdates(std::ostream* os) const {
            expected_updates_.begin();
        i != expected_updates_.end();
        ++i) {
-
     if (start) {
       start = false;
     } else {
@@ -72,6 +71,7 @@ void ObserverUpdatesMatcher::DescribeUpdates(std::ostream* os) const {
         << " )";
   }
 }
+
 void ObserverUpdatesMatcher::DescribeTo(std::ostream* os) const {
   *os << " has updates: { ";
   DescribeUpdates(os);
diff --git a/components/dom_distiller/core/dom_distiller_test_util.h b/components/dom_distiller/core/dom_distiller_test_util.h
index bd9a1b9..e83a4fb 100644
--- a/components/dom_distiller/core/dom_distiller_test_util.h
+++ b/components/dom_distiller/core/dom_distiller_test_util.h
@@ -5,6 +5,8 @@
 #ifndef COMPONENTS_DOM_DISTILLER_CORE_DOM_DISTILLER_TEST_UTIL_H_
 #define COMPONENTS_DOM_DISTILLER_CORE_DOM_DISTILLER_TEST_UTIL_H_
 
+#include <vector>
+
 #include "components/dom_distiller/core/dom_distiller_observer.h"
 #include "components/dom_distiller/core/fake_db.h"
 #include "testing/gmock/include/gmock/gmock.h"
diff --git a/components/dom_distiller/core/fake_db.h b/components/dom_distiller/core/fake_db.h
index 8b7e9c7..8d03d98 100644
--- a/components/dom_distiller/core/fake_db.h
+++ b/components/dom_distiller/core/fake_db.h
@@ -5,6 +5,8 @@
 #ifndef COMPONENTS_DOM_DISTILLER_CORE_FAKE_DB_H_
 #define COMPONENTS_DOM_DISTILLER_CORE_FAKE_DB_H_
 
+#include <string>
+
 #include "components/dom_distiller/core/dom_distiller_database.h"
 
 namespace dom_distiller {
diff --git a/components/dom_distiller/core/fake_distiller.cc b/components/dom_distiller/core/fake_distiller.cc
index 027d297..93659b3 100644
--- a/components/dom_distiller/core/fake_distiller.cc
+++ b/components/dom_distiller/core/fake_distiller.cc
@@ -4,6 +4,10 @@
 
 #include "components/dom_distiller/core/fake_distiller.h"
 
+#include "base/bind.h"
+#include "base/message_loop/message_loop.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
 namespace dom_distiller {
 namespace test {
 
@@ -16,5 +20,20 @@ FakeDistiller::FakeDistiller() {
 
 FakeDistiller::~FakeDistiller() { Die(); }
 
+void FakeDistiller::RunDistillerCallback(scoped_ptr<DistilledPageProto> proto) {
+  base::MessageLoop::current()->PostTask(
+      FROM_HERE,
+      base::Bind(&FakeDistiller::RunDistillerCallbackInternal,
+                 base::Unretained(this),
+                 base::Passed(&proto)));
+}
+
+void FakeDistiller::RunDistillerCallbackInternal(
+    scoped_ptr<DistilledPageProto> proto) {
+  EXPECT_FALSE(callback_.is_null());
+  callback_.Run(proto.Pass());
+  callback_.Reset();
+}
+
 }  // namespace test
 }  // namespace dom_distiller
diff --git a/components/dom_distiller/core/fake_distiller.h b/components/dom_distiller/core/fake_distiller.h
index ce6cb12..ef9126c 100644
--- a/components/dom_distiller/core/fake_distiller.h
+++ b/components/dom_distiller/core/fake_distiller.h
@@ -8,7 +8,6 @@
 #include "components/dom_distiller/core/article_entry.h"
 #include "components/dom_distiller/core/distiller.h"
 #include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
 class GURL;
@@ -38,17 +37,15 @@ class FakeDistiller : public Distiller {
     callback_ = callback;
   }
 
-  void RunDistillerCallback(scoped_ptr<DistilledPageProto> proto) {
-    EXPECT_FALSE(callback_.is_null());
-    callback_.Run(proto.Pass());
-    callback_.Reset();
-  }
+  void RunDistillerCallback(scoped_ptr<DistilledPageProto> proto);
 
   GURL GetUrl() { return url_; }
 
   DistillerCallback GetCallback() { return callback_; }
 
  private:
+  void RunDistillerCallbackInternal(scoped_ptr<DistilledPageProto> proto);
+
   GURL url_;
   DistillerCallback callback_;
 };
diff --git a/components/dom_distiller/core/task_tracker.cc b/components/dom_distiller/core/task_tracker.cc
index faf6fc1..ccd5656 100644
--- a/components/dom_distiller/core/task_tracker.cc
+++ b/components/dom_distiller/core/task_tracker.cc
@@ -48,17 +48,13 @@ void TaskTracker::StartBlobFetcher() {
   // |entry_| and asynchronously notify |this| when it is done.
 }
 
-void TaskTracker::SetSaveCallback(SaveCallback callback) {
-  save_callback_ = callback;
-  if (save_callback_.is_null()) {
-    MaybeCancel();
-  } else if (distilled_page_) {
+void TaskTracker::AddSaveCallback(const SaveCallback& callback) {
+  DCHECK(!callback.is_null());
+  save_callbacks_.push_back(callback);
+  if (distilled_page_) {
     // Distillation for this task has already completed, and so it can be
     // immediately saved.
-    base::MessageLoop::current()->PostTask(
-        FROM_HERE,
-        base::Bind(&TaskTracker::DoSaveCallback,
-                   weak_ptr_factory_.GetWeakPtr()));
+    ScheduleSaveCallbacks(true);
   }
 }
 
@@ -77,6 +73,8 @@ scoped_ptr<ViewerHandle> TaskTracker::AddViewer(ViewRequestDelegate* delegate) {
       &TaskTracker::RemoveViewer, weak_ptr_factory_.GetWeakPtr(), delegate)));
 }
 
+const std::string& TaskTracker::GetEntryId() const { return entry_.entry_id(); }
+
 bool TaskTracker::HasEntryId(const std::string& entry_id) const {
   return entry_.entry_id() == entry_id;
 }
@@ -98,7 +96,7 @@ void TaskTracker::RemoveViewer(ViewRequestDelegate* delegate) {
 }
 
 void TaskTracker::MaybeCancel() {
-  if (!save_callback_.is_null() || !viewers_.empty()) {
+  if (!save_callbacks_.empty() || !viewers_.empty()) {
     // There's still work to be done.
     return;
   }
@@ -110,11 +108,27 @@ void TaskTracker::MaybeCancel() {
   DCHECK(self);
 }
 
-void TaskTracker::DoSaveCallback() {
-  DCHECK(distilled_page_);
-  if (!save_callback_.is_null()) {
-    save_callback_.Run(entry_, distilled_page_.get());
-    save_callback_.Reset();
+void TaskTracker::CancelSaveCallbacks() { ScheduleSaveCallbacks(false); }
+
+void TaskTracker::ScheduleSaveCallbacks(bool distillation_succeeded) {
+  base::MessageLoop::current()->PostTask(
+      FROM_HERE,
+      base::Bind(&TaskTracker::DoSaveCallbacks,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 distillation_succeeded));
+}
+
+void TaskTracker::DoSaveCallbacks(bool distillation_succeeded) {
+  if (!save_callbacks_.empty()) {
+    DistilledPageProto* distilled_proto =
+        distillation_succeeded ? distilled_page_.get() : NULL;
+
+    for (size_t i = 0; i < save_callbacks_.size(); ++i) {
+      DCHECK(!save_callbacks_[i].is_null());
+      save_callbacks_[i].Run(entry_, distilled_proto, distillation_succeeded);
+    }
+
+    save_callbacks_.clear();
     MaybeCancel();
   }
 }
@@ -126,12 +140,15 @@ void TaskTracker::NotifyViewer(ViewRequestDelegate* delegate) {
 
 void TaskTracker::OnDistilledDataReady(scoped_ptr<DistilledPageProto> proto) {
   distilled_page_ = proto.Pass();
+  DCHECK(distilled_page_);
 
   entry_.set_title(distilled_page_->title());
   for (size_t i = 0; i < viewers_.size(); ++i) {
     NotifyViewer(viewers_[i]);
   }
-  DoSaveCallback();
+
+  // Already inside a callback run SaveCallbacks directly.
+  DoSaveCallbacks(true);
 }
 
 }  // namespace dom_distiller
diff --git a/components/dom_distiller/core/task_tracker.h b/components/dom_distiller/core/task_tracker.h
index 9133103..f545dd9 100644
--- a/components/dom_distiller/core/task_tracker.h
+++ b/components/dom_distiller/core/task_tracker.h
@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/callback.h"
 #include "base/memory/weak_ptr.h"
 #include "components/dom_distiller/core/article_entry.h"
 #include "components/dom_distiller/core/distiller.h"
@@ -67,7 +68,7 @@ class ViewRequestDelegate {
 class TaskTracker {
  public:
   typedef base::Callback<void(TaskTracker*)> CancelCallback;
-  typedef base::Callback<void(const ArticleEntry&, DistilledPageProto*)>
+  typedef base::Callback<void(const ArticleEntry&, DistilledPageProto*, bool)>
       SaveCallback;
 
   TaskTracker(const ArticleEntry& entry, CancelCallback callback);
@@ -77,17 +78,25 @@ class TaskTracker {
   void StartDistiller(DistillerFactory* factory);
   void StartBlobFetcher();
 
-  void SetSaveCallback(SaveCallback callback);
+  void AddSaveCallback(const SaveCallback& callback);
+
+  void CancelSaveCallbacks();
 
   // The ViewerHandle should be destroyed before the ViewRequestDelegate.
   scoped_ptr<ViewerHandle> AddViewer(ViewRequestDelegate* delegate);
 
+  const std::string& GetEntryId() const;
   bool HasEntryId(const std::string& entry_id) const;
   bool HasUrl(const GURL& url) const;
 
  private:
   void OnDistilledDataReady(scoped_ptr<DistilledPageProto> distilled_page);
-  void DoSaveCallback();
+  // Posts a task to run DoSaveCallbacks with |distillation_succeeded|.
+  void ScheduleSaveCallbacks(bool distillation_succeeded);
+
+  // Runs all callbacks passing |distillation_succeeded| and clears them. Should
+  // be called through ScheduleSaveCallbacks.
+  void DoSaveCallbacks(bool distillation_succeeded);
 
   void RemoveViewer(ViewRequestDelegate* delegate);
   void NotifyViewer(ViewRequestDelegate* delegate);
@@ -95,7 +104,7 @@ class TaskTracker {
   void MaybeCancel();
 
   CancelCallback cancel_callback_;
-  SaveCallback save_callback_;
+  std::vector<SaveCallback> save_callbacks_;
 
   scoped_ptr<Distiller> distiller_;
 
diff --git a/components/dom_distiller/core/task_tracker_unittest.cc b/components/dom_distiller/core/task_tracker_unittest.cc
index d404bf0..2d3ad8a 100644
--- a/components/dom_distiller/core/task_tracker_unittest.cc
+++ b/components/dom_distiller/core/task_tracker_unittest.cc
@@ -36,7 +36,7 @@ class TestCancelCallback {
 
 class MockSaveCallback {
  public:
-  MOCK_METHOD2(Save, void(const ArticleEntry&, DistilledPageProto*));
+  MOCK_METHOD3(Save, void(const ArticleEntry&, DistilledPageProto*, bool));
 };
 
 class DomDistillerTaskTrackerTest : public testing::Test {
@@ -109,7 +109,7 @@ TEST_F(DomDistillerTaskTrackerTest, TestViewerCancelledWithSaveRequest) {
   EXPECT_FALSE(cancel_callback.Cancelled());
 
   MockSaveCallback save_callback;
-  task_tracker.SetSaveCallback(
+  task_tracker.AddSaveCallback(
       base::Bind(&MockSaveCallback::Save, base::Unretained(&save_callback)));
   handle.reset();
 
@@ -148,11 +148,11 @@ TEST_F(DomDistillerTaskTrackerTest,
   TaskTracker task_tracker(GetDefaultEntry(), cancel_callback.GetCallback());
 
   MockSaveCallback save_callback;
-  task_tracker.SetSaveCallback(
+  task_tracker.AddSaveCallback(
       base::Bind(&MockSaveCallback::Save, base::Unretained(&save_callback)));
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_CALL(save_callback, Save(_, _));
+  EXPECT_CALL(save_callback, Save(_, _, _));
 
   task_tracker.StartDistiller(&distiller_factory);
   distiller->RunDistillerCallback(make_scoped_ptr(new DistilledPageProto));
diff --git a/components/dom_distiller/webui/dom_distiller_handler.cc b/components/dom_distiller/webui/dom_distiller_handler.cc
index aaec2c1..9067b71 100644
--- a/components/dom_distiller/webui/dom_distiller_handler.cc
+++ b/components/dom_distiller/webui/dom_distiller_handler.cc
@@ -17,10 +17,7 @@ namespace dom_distiller {
 
 DomDistillerHandler::DomDistillerHandler(DomDistillerService* service,
                                          const std::string& scheme)
-    : weak_ptr_factory_(this),
-      service_(service) {
-  article_scheme_ = scheme;
-}
+    : service_(service), article_scheme_(scheme), weak_ptr_factory_(this) {}
 
 DomDistillerHandler::~DomDistillerHandler() {}
 
@@ -28,25 +25,29 @@ void DomDistillerHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback(
       "requestEntries",
       base::Bind(&DomDistillerHandler::HandleRequestEntries,
-      base::Unretained(this)));
+                 base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
       "addArticle",
       base::Bind(&DomDistillerHandler::HandleAddArticle,
-      base::Unretained(this)));
+                 base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
       "selectArticle",
       base::Bind(&DomDistillerHandler::HandleSelectArticle,
-      base::Unretained(this)));
+                 base::Unretained(this)));
 }
 
 void DomDistillerHandler::HandleAddArticle(const ListValue* args) {
   std::string url;
   args->GetString(0, &url);
   GURL gurl(url);
-  if (gurl.is_valid())
-    service_->AddToList(gurl);
-  else
+  if (gurl.is_valid()) {
+    service_->AddToList(
+        gurl,
+        base::Bind(base::Bind(&DomDistillerHandler::OnArticleAdded,
+                              base::Unretained(this))));
+  } else {
     web_ui()->CallJavascriptFunction("domDistiller.onArticleAddFailed");
+  }
 }
 
 void DomDistillerHandler::HandleSelectArticle(const ListValue* args) {
@@ -66,12 +67,22 @@ void DomDistillerHandler::HandleRequestEntries(const ListValue* args) {
     DCHECK(IsEntryValid(article));
     scoped_ptr<base::DictionaryValue> entry(new base::DictionaryValue());
     entry->SetString("entry_id", article.entry_id());
-    std::string title = (!article.has_title() || article.title().empty()) ?
-        article.entry_id() : article.title();
+    std::string title = (!article.has_title() || article.title().empty())
+                            ? article.entry_id()
+                            : article.title();
     entry->SetString("title", title);
     entries.Append(entry.release());
   }
   web_ui()->CallJavascriptFunction("domDistiller.onReceivedEntries", entries);
 }
 
+void DomDistillerHandler::OnArticleAdded(bool article_available) {
+  // TODO(nyquist): Update this function.
+  if (article_available) {
+    HandleRequestEntries(NULL);
+  } else {
+    web_ui()->CallJavascriptFunction("domDistiller.onArticleAddFailed");
+  }
+}
+
 }  // namespace dom_distiller
diff --git a/components/dom_distiller/webui/dom_distiller_handler.h b/components/dom_distiller/webui/dom_distiller_handler.h
index 512da06..e328633 100644
--- a/components/dom_distiller/webui/dom_distiller_handler.h
+++ b/components/dom_distiller/webui/dom_distiller_handler.h
@@ -41,8 +41,8 @@ class DomDistillerHandler : public content::WebUIMessageHandler {
   void HandleSelectArticle(const ListValue* args);
 
  private:
-  // Factory for the creating refs in callbacks.
-  base::WeakPtrFactory<DomDistillerHandler> weak_ptr_factory_;
+  // Callback from DomDistillerService when an article is available.
+  void OnArticleAdded(bool article_available);
 
   // The DomDistillerService.
   DomDistillerService* service_;
@@ -50,6 +50,9 @@ class DomDistillerHandler : public content::WebUIMessageHandler {
   // The scheme for DOM distiller articles.
   std::string article_scheme_;
 
+  // Factory for the creating refs in callbacks.
+  base::WeakPtrFactory<DomDistillerHandler> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(DomDistillerHandler);
 };