Add incremental updates for multipage distillation.

Add support for providing incremental updates for multipage
distillation. Updates are now broadcasted to the viewers, whenever a new
page in the article finishes distillation. This allows viewer to show
data to the user, before the distillation of the entire article is
completed.
This change required changing management of pages in distiller to be
refcounted.

BUG=288015
NOTRY=true
R=cjhopman@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@255708 0039d316-1c4b-4281-b951-d872f2087c98
parent 978e9010
......@@ -50,6 +50,8 @@
'sources': [
'dom_distiller/android/component_jni_registrar.cc',
'dom_distiller/android/component_jni_registrar.h',
'dom_distiller/core/article_distillation_update.cc',
'dom_distiller/core/article_distillation_update.h',
'dom_distiller/core/article_entry.cc',
'dom_distiller/core/article_entry.h',
'dom_distiller/core/distiller.cc',
......
......@@ -59,6 +59,9 @@ class DomDistillerViewerSource::RequestViewerHandle
virtual void OnArticleReady(const DistilledArticleProto* article_proto)
OVERRIDE;
virtual void OnArticleUpdated(ArticleDistillationUpdate article_update)
OVERRIDE;
void TakeViewerHandle(scoped_ptr<ViewerHandle> viewer_handle);
private:
......@@ -102,6 +105,11 @@ void DomDistillerViewerSource::RequestViewerHandle::OnArticleReady(
base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
}
void DomDistillerViewerSource::RequestViewerHandle::OnArticleUpdated(
ArticleDistillationUpdate article_update) {
// TODO(nyquist): Add support for displaying pages incrementally.
}
void DomDistillerViewerSource::RequestViewerHandle::TakeViewerHandle(
scoped_ptr<ViewerHandle> viewer_handle) {
viewer_handle_ = viewer_handle.Pass();
......@@ -179,7 +187,7 @@ bool DomDistillerViewerSource::ShouldServiceRequest(
// TODO(nyquist): Start tracking requests using this method.
void DomDistillerViewerSource::WillServiceRequest(
const net::URLRequest* request,
std::string* path) const {};
std::string* path) const {}
std::string DomDistillerViewerSource::GetContentSecurityPolicyObjectSrc()
const {
......
......@@ -5,6 +5,8 @@
#ifndef COMPONENTS_DOM_DISTILLER_CONTENT_DOM_DISTILLER_VIEWER_SOURCE_H_
#define COMPONENTS_DOM_DISTILLER_CONTENT_DOM_DISTILLER_VIEWER_SOURCE_H_
#include <string>
#include "base/basictypes.h"
#include "base/compiler_specific.h"
#include "content/public/browser/url_data_source.h"
......
......@@ -4,7 +4,10 @@
#include "components/dom_distiller/content/dom_distiller_viewer_source.h"
#include <vector>
#include "base/callback.h"
#include "components/dom_distiller/core/article_distillation_update.h"
#include "components/dom_distiller/core/article_entry.h"
#include "components/dom_distiller/core/dom_distiller_service.h"
#include "components/dom_distiller/core/dom_distiller_store.h"
......@@ -23,6 +26,8 @@ class FakeViewRequestDelegate : public ViewRequestDelegate {
public:
virtual ~FakeViewRequestDelegate() {}
MOCK_METHOD1(OnArticleReady, void(const DistilledArticleProto* proto));
MOCK_METHOD1(OnArticleUpdated,
void(ArticleDistillationUpdate article_update));
};
class TestDomDistillerService : public DomDistillerServiceInterface {
......
// 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 "components/dom_distiller/core/article_distillation_update.h"
#include "base/logging.h"
namespace dom_distiller {
ArticleDistillationUpdate::ArticleDistillationUpdate(
const std::vector<scoped_refptr<RefCountedPageProto> >& pages,
bool has_next_page,
bool has_prev_page)
: has_next_page_(has_next_page),
has_prev_page_(has_prev_page),
pages_(pages) {}
ArticleDistillationUpdate::~ArticleDistillationUpdate() {}
const DistilledPageProto& ArticleDistillationUpdate::GetDistilledPage(
size_t index) const {
DCHECK_GT(pages_.size(), index);
return pages_[index]->data;
}
} // namespace dom_distiller
// 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.
#ifndef COMPONENTS_DOM_DISTILLER_CORE_ARTICLE_DISTILLATION_UPDATE_H_
#define COMPONENTS_DOM_DISTILLER_CORE_ARTICLE_DISTILLATION_UPDATE_H_
#include <vector>
#include "base/memory/ref_counted.h"
#include "components/dom_distiller/core/proto/distilled_page.pb.h"
namespace dom_distiller {
// Update about an article that is currently under distillation.
class ArticleDistillationUpdate {
public:
typedef base::RefCountedData<DistilledPageProto> RefCountedPageProto;
ArticleDistillationUpdate(
const std::vector<scoped_refptr<RefCountedPageProto> >& pages,
bool has_next_page,
bool has_prev_page);
~ArticleDistillationUpdate();
// Returns the distilled page at |index|.
const DistilledPageProto& GetDistilledPage(size_t index) const;
// Returns the size of distilled pages in this update.
size_t GetPagesSize() const { return pages_.size(); }
// Returns true, if article has a next page that is currently under
// distillation and that is not part of the distilled pages included in this
// update.
bool HasNextPage() const { return has_next_page_; }
// Returns true if article has a previous page that is currently under
// distillation and that is not part of the distilled pages included in this
// update.
bool HasPrevPage() const { return has_prev_page_; }
private:
bool has_next_page_;
bool has_prev_page_;
// Currently available pages.
std::vector<scoped_refptr<RefCountedPageProto> > pages_;
};
} // namespace dom_distiller
#endif // COMPONENTS_DOM_DISTILLER_CORE_ARTICLE_DISTILLATION_UPDATE_H_
......@@ -5,6 +5,7 @@
#include "components/dom_distiller/core/distiller.h"
#include <map>
#include <vector>
#include "base/bind.h"
#include "base/callback.h"
......@@ -96,9 +97,11 @@ DistillerImpl::DistilledPageData* DistillerImpl::GetPageAtIndex(size_t index)
}
void DistillerImpl::DistillPage(const GURL& url,
const DistillerCallback& distillation_cb) {
const DistillationFinishedCallback& finished_cb,
const DistillationUpdateCallback& update_cb) {
DCHECK(AreAllPagesFinished());
distillation_cb_ = distillation_cb;
finished_cb_ = finished_cb;
update_cb_ = update_cb;
AddToDistillationQueue(0, url);
DistillNextPage();
......@@ -136,13 +139,13 @@ void DistillerImpl::OnPageDistillationFinished(
if (distillation_successful) {
DistilledPageData* page_data =
GetPageAtIndex(started_pages_index_[page_num]);
DistilledPageProto* current_page = new DistilledPageProto();
page_data->proto.reset(current_page);
page_data->distilled_page_proto =
new base::RefCountedData<DistilledPageProto>();
page_data->page_num = page_num;
page_data->title = distilled_page->title;
current_page->set_url(page_url.spec());
current_page->set_html(distilled_page->html);
page_data->distilled_page_proto->data.set_url(page_url.spec());
page_data->distilled_page_proto->data.set_html(distilled_page->html);
GURL next_page_url(distilled_page->next_page_url);
if (next_page_url.is_valid()) {
......@@ -195,7 +198,7 @@ void DistillerImpl::OnFetchImageDone(int page_num,
const std::string& response) {
DCHECK(started_pages_index_.find(page_num) != started_pages_index_.end());
DistilledPageData* page_data = GetPageAtIndex(started_pages_index_[page_num]);
DCHECK(page_data->proto);
DCHECK(page_data->distilled_page_proto);
DCHECK(url_fetcher);
ScopedVector<DistillerURLFetcher>::iterator fetcher_it =
std::find(page_data->image_fetchers_.begin(),
......@@ -208,7 +211,8 @@ void DistillerImpl::OnFetchImageDone(int page_num,
page_data->image_fetchers_.weak_erase(fetcher_it);
base::MessageLoop::current()->DeleteSoon(FROM_HERE, url_fetcher);
DistilledPageProto_Image* image = page_data->proto->add_image();
DistilledPageProto_Image* image =
page_data->distilled_page_proto->data.add_image();
image->set_name(id);
image->set_data(response);
......@@ -222,12 +226,37 @@ void DistillerImpl::AddPageIfDone(int page_num) {
if (page_data->image_fetchers_.empty()) {
finished_pages_index_[page_num] = started_pages_index_[page_num];
started_pages_index_.erase(page_num);
const ArticleDistillationUpdate& article_update =
CreateDistillationUpdate();
DCHECK_EQ(article_update.GetPagesSize(), finished_pages_index_.size());
update_cb_.Run(article_update);
RunDistillerCallbackIfDone();
}
}
const ArticleDistillationUpdate DistillerImpl::CreateDistillationUpdate()
const {
bool has_prev_page = false;
bool has_next_page = false;
if (!finished_pages_index_.empty()) {
int prev_page_num = finished_pages_index_.begin()->first - 1;
int next_page_num = finished_pages_index_.rbegin()->first + 1;
has_prev_page = IsPageNumberInUse(prev_page_num);
has_next_page = IsPageNumberInUse(next_page_num);
}
std::vector<scoped_refptr<ArticleDistillationUpdate::RefCountedPageProto> >
update_pages;
for (std::map<int, size_t>::const_iterator it = finished_pages_index_.begin();
it != finished_pages_index_.end();
++it) {
update_pages.push_back(pages_[it->second]->distilled_page_proto);
}
return ArticleDistillationUpdate(update_pages, has_next_page, has_prev_page);
}
void DistillerImpl::RunDistillerCallbackIfDone() {
DCHECK(!distillation_cb_.is_null());
DCHECK(!finished_cb_.is_null());
if (AreAllPagesFinished()) {
bool first_page = true;
scoped_ptr<DistilledArticleProto> article_proto(
......@@ -236,7 +265,7 @@ void DistillerImpl::RunDistillerCallbackIfDone() {
for (std::map<int, size_t>::iterator it = finished_pages_index_.begin();
it != finished_pages_index_.end();) {
DistilledPageData* page_data = GetPageAtIndex(it->second);
*(article_proto->add_pages()) = *(page_data->proto);
*(article_proto->add_pages()) = page_data->distilled_page_proto->data;
if (first_page) {
article_proto->set_title(page_data->title);
......@@ -252,8 +281,8 @@ void DistillerImpl::RunDistillerCallbackIfDone() {
DCHECK(pages_.empty());
DCHECK(finished_pages_index_.empty());
distillation_cb_.Run(article_proto.Pass());
distillation_cb_.Reset();
finished_cb_.Run(article_proto.Pass());
finished_cb_.Reset();
}
}
......
......@@ -5,12 +5,15 @@
#ifndef COMPONENTS_DOM_DISTILLER_CORE_DISTILLER_H_
#define COMPONENTS_DOM_DISTILLER_CORE_DISTILLER_H_
#include <map>
#include <string>
#include "base/callback.h"
#include "base/containers/hash_tables.h"
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/scoped_vector.h"
#include "components/dom_distiller/core/article_distillation_update.h"
#include "components/dom_distiller/core/distiller_url_fetcher.h"
#include "components/dom_distiller/core/page_distiller.h"
#include "components/dom_distiller/core/proto/distilled_article.pb.h"
......@@ -24,13 +27,21 @@ class DistillerImpl;
class Distiller {
public:
typedef base::Callback<void(scoped_ptr<DistilledArticleProto>)>
DistillerCallback;
DistillationFinishedCallback;
typedef base::Callback<void(const ArticleDistillationUpdate&)>
DistillationUpdateCallback;
virtual ~Distiller() {}
// Distills a page, and asynchrounously returns the article HTML to the
// supplied callback.
// Distills a page, and asynchronously returns the article HTML to the
// supplied |finished_cb| callback. |update_cb| is invoked whenever article
// under distillation is updated with more data.
// E.g. when distilling a 2 page article, |update_cb| may be invoked each time
// a distilled page is added and |finished_cb| will be invoked once
// distillation is completed.
virtual void DistillPage(const GURL& url,
const DistillerCallback& callback) = 0;
const DistillationFinishedCallback& finished_cb,
const DistillationUpdateCallback& update_cb) = 0;
};
class DistillerFactory {
......@@ -66,7 +77,9 @@ class DistillerImpl : public Distiller {
virtual void Init();
virtual void DistillPage(const GURL& url,
const DistillerCallback& callback) OVERRIDE;
const DistillationFinishedCallback& finished_cb,
const DistillationUpdateCallback& update_cb)
OVERRIDE;
void SetMaxNumPagesInArticle(size_t max_num_pages);
......@@ -84,7 +97,8 @@ class DistillerImpl : public Distiller {
int page_num;
std::string title;
ScopedVector<DistillerURLFetcher> image_fetchers_;
scoped_ptr<DistilledPageProto> proto;
scoped_refptr<base::RefCountedData<DistilledPageProto> >
distilled_page_proto;
private:
DISALLOW_COPY_AND_ASSIGN(DistilledPageData);
......@@ -122,7 +136,7 @@ class DistillerImpl : public Distiller {
// includes pages that are pending distillation.
size_t TotalPageCount() const;
// Runs |distillation_cb_| if all distillation callbacks and image fetches are
// Runs |finished_cb_| if all distillation callbacks and image fetches are
// complete.
void RunDistillerCallbackIfDone();
......@@ -132,9 +146,14 @@ class DistillerImpl : public Distiller {
DistilledPageData* GetPageAtIndex(size_t index) const;
// Create an ArticleDistillationUpdate for the current distillation
// state.
const ArticleDistillationUpdate CreateDistillationUpdate() const;
const DistillerURLFetcherFactory& distiller_url_fetcher_factory_;
scoped_ptr<PageDistiller> page_distiller_;
DistillerCallback distillation_cb_;
DistillationFinishedCallback finished_cb_;
DistillationUpdateCallback update_cb_;
// Set of pages that are under distillation or have finished distillation.
// |started_pages_index_| and |finished_pages_index_| maintains the mapping
......
......@@ -33,6 +33,8 @@ class FakeViewRequestDelegate : public ViewRequestDelegate {
public:
virtual ~FakeViewRequestDelegate() {}
MOCK_METHOD1(OnArticleReady, void(const DistilledArticleProto* proto));
MOCK_METHOD1(OnArticleUpdated,
void(ArticleDistillationUpdate article_update));
};
class MockDistillerObserver : public DomDistillerObserver {
......@@ -120,7 +122,7 @@ TEST_F(DomDistillerServiceTest, TestViewEntry) {
scoped_ptr<ViewerHandle> handle =
service_->ViewEntry(&viewer_delegate, entry_id);
ASSERT_FALSE(distiller->GetCallback().is_null());
ASSERT_FALSE(distiller->GetArticleCallback().is_null());
scoped_ptr<DistilledArticleProto> proto = CreateDefaultArticle();
EXPECT_CALL(viewer_delegate, OnArticleReady(proto.get()));
......@@ -137,7 +139,7 @@ TEST_F(DomDistillerServiceTest, TestViewUrl) {
GURL url("http://www.example.com/p1");
scoped_ptr<ViewerHandle> handle = service_->ViewUrl(&viewer_delegate, url);
ASSERT_FALSE(distiller->GetCallback().is_null());
ASSERT_FALSE(distiller->GetArticleCallback().is_null());
EXPECT_EQ(url, distiller->GetUrl());
scoped_ptr<DistilledArticleProto> proto = CreateDefaultArticle();
......@@ -162,7 +164,7 @@ TEST_F(DomDistillerServiceTest, TestMultipleViewUrl) {
scoped_ptr<ViewerHandle> handle = service_->ViewUrl(&viewer_delegate, url);
scoped_ptr<ViewerHandle> handle2 = service_->ViewUrl(&viewer_delegate2, url2);
ASSERT_FALSE(distiller->GetCallback().is_null());
ASSERT_FALSE(distiller->GetArticleCallback().is_null());
EXPECT_EQ(url, distiller->GetUrl());
scoped_ptr<DistilledArticleProto> proto = CreateDefaultArticle();
......@@ -170,7 +172,7 @@ TEST_F(DomDistillerServiceTest, TestMultipleViewUrl) {
RunDistillerCallback(distiller, proto.Pass());
ASSERT_FALSE(distiller2->GetCallback().is_null());
ASSERT_FALSE(distiller2->GetArticleCallback().is_null());
EXPECT_EQ(url2, distiller2->GetUrl());
scoped_ptr<DistilledArticleProto> proto2 = CreateDefaultArticle();
......@@ -192,7 +194,7 @@ TEST_F(DomDistillerServiceTest, TestViewUrlCancelled) {
GURL url("http://www.example.com/p1");
scoped_ptr<ViewerHandle> handle = service_->ViewUrl(&viewer_delegate, url);
ASSERT_FALSE(distiller->GetCallback().is_null());
ASSERT_FALSE(distiller->GetArticleCallback().is_null());
EXPECT_EQ(url, distiller->GetUrl());
EXPECT_CALL(viewer_delegate, OnArticleReady(_)).Times(0);
......@@ -234,7 +236,7 @@ TEST_F(DomDistillerServiceTest, TestAddAndRemoveEntry) {
std::string entry_id = service_->AddToList(url, ArticleCallback(&article_cb));
ASSERT_FALSE(distiller->GetCallback().is_null());
ASSERT_FALSE(distiller->GetArticleCallback().is_null());
EXPECT_EQ(url, distiller->GetUrl());
scoped_ptr<DistilledArticleProto> proto = CreateArticleWithURL(url.spec());
......@@ -389,7 +391,7 @@ TEST_F(DomDistillerServiceTest, TestMultiplePageArticle) {
service_->AddToList(pages_url[0], ArticleCallback(&article_cb));
ArticleEntry entry;
ASSERT_FALSE(distiller->GetCallback().is_null());
ASSERT_FALSE(distiller->GetArticleCallback().is_null());
EXPECT_EQ(pages_url[0], distiller->GetUrl());
// Create the article with pages to pass to the distiller.
......
......@@ -21,10 +21,13 @@ FakeDistiller::FakeDistiller(bool execute_callback)
FakeDistiller::~FakeDistiller() { Die(); }
void FakeDistiller::DistillPage(const GURL& url,
const DistillerCallback& callback) {
void FakeDistiller::DistillPage(
const GURL& url,
const DistillationFinishedCallback& article_callback,
const DistillationUpdateCallback& page_callback) {
url_ = url;
callback_ = callback;
article_callback_ = article_callback;
page_callback_ = page_callback;
if (execute_callback_) {
scoped_ptr<DistilledArticleProto> proto(new DistilledArticleProto);
proto->add_pages()->set_url(url_.spec());
......@@ -43,9 +46,9 @@ void FakeDistiller::RunDistillerCallback(
void FakeDistiller::RunDistillerCallbackInternal(
scoped_ptr<DistilledArticleProto> proto) {
EXPECT_FALSE(callback_.is_null());
callback_.Run(proto.Pass());
callback_.Reset();
EXPECT_FALSE(article_callback_.is_null());
article_callback_.Run(proto.Pass());
article_callback_.Reset();
}
} // namespace test
......
......@@ -5,6 +5,7 @@
#ifndef COMPONENTS_DOM_DISTILLER_CORE_FAKE_DISTILLER_H_
#define COMPONENTS_DOM_DISTILLER_CORE_FAKE_DISTILLER_H_
#include "components/dom_distiller/core/article_distillation_update.h"
#include "components/dom_distiller/core/article_entry.h"
#include "components/dom_distiller/core/distiller.h"
#include "testing/gmock/include/gmock/gmock.h"
......@@ -32,20 +33,25 @@ class FakeDistiller : public Distiller {
MOCK_METHOD0(Die, void());
virtual void DistillPage(const GURL& url,
const DistillerCallback& callback) OVERRIDE;
const DistillationFinishedCallback& article_callback,
const DistillationUpdateCallback& page_callback)
OVERRIDE;
void RunDistillerCallback(scoped_ptr<DistilledArticleProto> proto);
GURL GetUrl() { return url_; }
DistillerCallback GetCallback() { return callback_; }
DistillationFinishedCallback GetArticleCallback() {
return article_callback_;
}
private:
void RunDistillerCallbackInternal(scoped_ptr<DistilledArticleProto> proto);
bool execute_callback_;
GURL url_;
DistillerCallback callback_;
DistillationFinishedCallback article_callback_;
DistillationUpdateCallback page_callback_;
};
} // namespace test
......
......@@ -41,7 +41,9 @@ void TaskTracker::StartDistiller(DistillerFactory* factory) {
distiller_ = factory->CreateDistiller();
distiller_->DistillPage(url,
base::Bind(&TaskTracker::OnDistilledDataReady,
base::Bind(&TaskTracker::OnDistilledArticleReady,
weak_ptr_factory_.GetWeakPtr()),
base::Bind(&TaskTracker::OnArticleDistillationUpdated,
weak_ptr_factory_.GetWeakPtr()));
}
......@@ -139,7 +141,7 @@ void TaskTracker::NotifyViewer(ViewRequestDelegate* delegate) {
delegate->OnArticleReady(distilled_article_.get());
}
void TaskTracker::OnDistilledDataReady(
void TaskTracker::OnDistilledArticleReady(
scoped_ptr<DistilledArticleProto> distilled_article) {
distilled_article_ = distilled_article.Pass();
bool distillation_successful = false;
......@@ -164,4 +166,11 @@ void TaskTracker::OnDistilledDataReady(
DoSaveCallbacks(distillation_successful);
}
void TaskTracker::OnArticleDistillationUpdated(
const ArticleDistillationUpdate& article_update) {
for (size_t i = 0; i < viewers_.size(); ++i) {
viewers_[i]->OnArticleUpdated(article_update);
}
}
} // namespace dom_distiller
......@@ -11,6 +11,7 @@
#include "base/bind.h"
#include "base/callback.h"
#include "base/memory/weak_ptr.h"
#include "components/dom_distiller/core/article_distillation_update.h"
#include "components/dom_distiller/core/article_entry.h"
#include "components/dom_distiller/core/distiller.h"
#include "components/dom_distiller/core/proto/distilled_page.pb.h"
......@@ -44,6 +45,9 @@ class ViewRequestDelegate {
// when the corresponding ViewerHandle is destroyed (or when the
// DomDistillerService is destroyed).
virtual void OnArticleReady(const DistilledArticleProto* article_proto) = 0;
// Called when an article that is currently under distillation is updated.
virtual void OnArticleUpdated(ArticleDistillationUpdate article_update) = 0;
};
// A TaskTracker manages the various tasks related to viewing, saving,
......@@ -90,8 +94,10 @@ class TaskTracker {
bool HasUrl(const GURL& url) const;
private:
void OnDistilledDataReady(
void OnDistilledArticleReady(
scoped_ptr<DistilledArticleProto> distilled_article);
void OnArticleDistillationUpdated(
const ArticleDistillationUpdate& article_update);
// Posts a task to run DoSaveCallbacks with |distillation_succeeded|.
void ScheduleSaveCallbacks(bool distillation_succeeded);
......
......@@ -5,6 +5,7 @@
#include "components/dom_distiller/core/task_tracker.h"
#include "base/run_loop.h"
#include "components/dom_distiller/core/article_distillation_update.h"
#include "components/dom_distiller/core/article_entry.h"
#include "components/dom_distiller/core/fake_distiller.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -20,6 +21,8 @@ class FakeViewRequestDelegate : public ViewRequestDelegate {
virtual ~FakeViewRequestDelegate() {}
MOCK_METHOD1(OnArticleReady,
void(const DistilledArticleProto* article_proto));
MOCK_METHOD1(OnArticleUpdated,
void(ArticleDistillationUpdate article_update));
};
class TestCancelCallback {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment