Add a DistilledContentStore (and an in-memory impl)

components/dom_distiller/core/task_tracker.cc

 // Copyright 2013 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 "components/dom_distiller/core/task_tracker.h"
 
 #include "base/auto_reset.h"
 #include "base/message_loop/message_loop.h"
+#include "components/dom_distiller/core/distilled_content_store.h"
 #include "components/dom_distiller/core/proto/distilled_article.pb.h"
 #include "components/dom_distiller/core/proto/distilled_page.pb.h"
 
 namespace dom_distiller {
 
 ViewerHandle::ViewerHandle(CancelCallback callback)
     : cancel_callback_(callback) {}
 
 ViewerHandle::~ViewerHandle() {
   if (!cancel_callback_.is_null()) {
     cancel_callback_.Run();
   }
 }
 
-TaskTracker::TaskTracker(const ArticleEntry& entry, CancelCallback callback)
+TaskTracker::TaskTracker(const ArticleEntry& entry,
+                         CancelCallback callback,
+                         DistilledContentStore* content_store)
     : cancel_callback_(callback),
+      content_store_(content_store),
       entry_(entry),
       distilled_article_(),
       content_ready_(false),
       destruction_allowed_(true),
       weak_ptr_factory_(this) {}
 
 TaskTracker::~TaskTracker() {
   DCHECK(destruction_allowed_);
   DCHECK(viewers_.empty());
 }
 
 void TaskTracker::StartDistiller(DistillerFactory* factory) {
   if (distiller_) {
     return;
   }
   if (entry_.pages_size() == 0) {
     return;
   }
-
   GURL url(entry_.pages(0).url());
   DCHECK(url.is_valid());
 
   distiller_ = factory->CreateDistiller();
   distiller_->DistillPage(url,
                           base::Bind(&TaskTracker::OnDistillerFinished,
                                      weak_ptr_factory_.GetWeakPtr()),
                           base::Bind(&TaskTracker::OnArticleDistillationUpdated,
                                      weak_ptr_factory_.GetWeakPtr()));
 }
 
 void TaskTracker::StartBlobFetcher() {
-  // TODO(cjhopman): There needs to be some local storage for the distilled
-  // blob. When that happens, this should start some task to fetch the blob for
-  // |entry_| and asynchronously notify |this| when it is done.
+  if (content_store_) {
+    content_store_->LoadContent(entry_,
+                                base::Bind(&TaskTracker::OnBlobFetched,
+                                           weak_ptr_factory_.GetWeakPtr()));
+  }
 }
 
 void TaskTracker::AddSaveCallback(const SaveCallback& callback) {
   DCHECK(!callback.is_null());
   save_callbacks_.push_back(callback);
   if (content_ready_) {
     // Distillation for this task has already completed, and so it can be
     // immediately saved.
     ScheduleSaveCallbacks(true);
   }
@@ skipping 70 matching lines @@
   }
 }
 
 void TaskTracker::NotifyViewer(ViewRequestDelegate* delegate) {
   DCHECK(content_ready_);
   delegate->OnArticleReady(distilled_article_.get());
 }
 
 void TaskTracker::OnDistillerFinished(
     scoped_ptr<DistilledArticleProto> distilled_article) {
-  OnDistilledArticleReady(distilled_article.Pass());
+  if (!content_ready_) {
+    OnDistilledArticleReady(distilled_article.Pass());
+  }
+}
+
+void TaskTracker::OnBlobFetched(
+    bool success,
+    scoped_ptr<DistilledArticleProto> distilled_article) {
+  if (!content_ready_ && success) {
+    OnDistilledArticleReady(distilled_article.Pass());
+  }
 }
 
 void TaskTracker::OnDistilledArticleReady(
     scoped_ptr<DistilledArticleProto> distilled_article) {
+  DCHECK(!content_ready_);
+  content_ready_ = true;
   distilled_article_ = distilled_article.Pass();
   bool distillation_successful = false;

[nyquist, 2014/04/10 19:00:13]

how about bool distillation_successful = distilled_article_->pages_size() > 0;

And then maybe even merge this and the block below?
Or have an additional if for the success if you do something along the lines of
what shashi mentioned inside the block.

[cjhopman, 2014/04/11 22:37:21]

Done.

   if (distilled_article_->pages_size() > 0) {
     distillation_successful = true;
     entry_.set_title(distilled_article_->title());
     // Reset the pages.
     entry_.clear_pages();
     for (int i = 0; i < distilled_article_->pages_size(); ++i) {
       sync_pb::ArticlePage* page = entry_.add_pages();
       page->set_url(distilled_article_->pages(i).url());
     }
   }
 
-  content_ready_ = true;
+  if (distillation_successful) {

[shashi, 2014/03/18 21:14:57]

Can OnDistillerFinished determine if the distillation was successful:
(distilled_article->pages_size() > 0)  and pass that to
OnDistilledArticleReady(scoped_ptr<> distilled_article, bool success)

This was both onBlobFetched and OnDistilledFinished call OnDistilledArticleReady
with additional success parameter and only OnDistillerFinished adds the content
to the store.

[nyquist, 2014/04/10 19:00:13]

Or how about just adding a param |store_if_valid| to this method and only pass
true in OnDistillerFinished?

[cjhopman, 2014/04/11 22:37:21]

So this should be more correct now.

+    // TODO(cjhopman): If this article came from the blob fetcher, there is no
+    // reason to store it back there.
+    AddDistilledContentToStore();
+  }
 
   for (size_t i = 0; i < viewers_.size(); ++i) {
     NotifyViewer(viewers_[i]);
   }
 
   // Already inside a callback run SaveCallbacks directly.
   DoSaveCallbacks(distillation_successful);
 }
 
 void TaskTracker::OnArticleDistillationUpdated(
     const ArticleDistillationUpdate& article_update) {
   for (size_t i = 0; i < viewers_.size(); ++i) {
     viewers_[i]->OnArticleUpdated(article_update);
   }
 }
 
+void TaskTracker::AddDistilledContentToStore() {
+  if (content_store_) {
+    content_store_->SaveContent(
+        entry_, *distilled_article_, DistilledContentStore::SaveCallback());
+  }
+}
+
+
 }  // namespace dom_distiller