summaryrefslogtreecommitdiffstats
path: root/components/dom_distiller/core
diff options
context:
space:
mode:
Diffstat (limited to 'components/dom_distiller/core')
-rw-r--r--components/dom_distiller/core/distiller.cc6
-rw-r--r--components/dom_distiller/core/distiller.h4
-rw-r--r--components/dom_distiller/core/distiller_page.cc47
-rw-r--r--components/dom_distiller/core/distiller_page.h51
-rw-r--r--components/dom_distiller/core/distiller_unittest.cc48
5 files changed, 29 insertions, 127 deletions
diff --git a/components/dom_distiller/core/distiller.cc b/components/dom_distiller/core/distiller.cc
index acbb68e..d720a5b 100644
--- a/components/dom_distiller/core/distiller.cc
+++ b/components/dom_distiller/core/distiller.cc
@@ -39,7 +39,6 @@ DistillerFactoryImpl::~DistillerFactoryImpl() {}
scoped_ptr<Distiller> DistillerFactoryImpl::CreateDistiller() {
scoped_ptr<DistillerImpl> distiller(new DistillerImpl(
*distiller_page_factory_, *distiller_url_fetcher_factory_));
- distiller->Init();
return distiller.PassAs<Distiller>();
}
@@ -61,11 +60,6 @@ DistillerImpl::~DistillerImpl() {
DCHECK(destruction_allowed_);
}
-void DistillerImpl::Init() {
- DCHECK(AreAllPagesFinished());
- distiller_page_->Init();
-}
-
void DistillerImpl::SetMaxNumPagesInArticle(size_t max_num_pages) {
max_pages_in_article_ = max_num_pages;
}
diff --git a/components/dom_distiller/core/distiller.h b/components/dom_distiller/core/distiller.h
index 249a1b3..8196757 100644
--- a/components/dom_distiller/core/distiller.h
+++ b/components/dom_distiller/core/distiller.h
@@ -73,10 +73,6 @@ class DistillerImpl : public Distiller {
const DistillerURLFetcherFactory& distiller_url_fetcher_factory);
virtual ~DistillerImpl();
- // Creates an execution context. This must be called once before any calls are
- // made to distill the page.
- virtual void Init();
-
virtual void DistillPage(const GURL& url,
const DistillationFinishedCallback& finished_cb,
const DistillationUpdateCallback& update_cb)
diff --git a/components/dom_distiller/core/distiller_page.cc b/components/dom_distiller/core/distiller_page.cc
index eed6c2c..a935c7a 100644
--- a/components/dom_distiller/core/distiller_page.cc
+++ b/components/dom_distiller/core/distiller_page.cc
@@ -19,54 +19,27 @@ DistilledPageInfo::~DistilledPageInfo() {}
DistillerPageFactory::~DistillerPageFactory() {}
-DistillerPage::DistillerPage() : state_(NO_CONTEXT) {}
+DistillerPage::DistillerPage() : ready_(true) {}
DistillerPage::~DistillerPage() {}
-void DistillerPage::Init() {
- DCHECK_EQ(NO_CONTEXT, state_);
- InitImpl();
- state_ = IDLE;
-}
-
void DistillerPage::DistillPage(const GURL& gurl,
const DistillerPageCallback& callback) {
- DCHECK(state_ == IDLE || state_ == PAGE_AVAILABLE ||
- state_ == PAGELOAD_FAILED);
- state_ = LOADING_PAGE;
+ DCHECK(ready_);
+ // It is only possible to distill one page at a time. |ready_| is reset when
+ // the callback to OnDistillationDone happens.
+ ready_ = false;
distiller_page_callback_ = callback;
- LoadURLImpl(gurl);
-}
-
-void DistillerPage::ExecuteJavaScript() {
- DCHECK_EQ(PAGE_AVAILABLE, state_);
- state_ = EXECUTING_JAVASCRIPT;
- DVLOG(1) << "Beginning distillation";
std::string script = ResourceBundle::GetSharedInstance()
.GetRawDataResource(IDR_DISTILLER_JS)
.as_string();
- ExecuteJavaScriptImpl(script);
-}
-
-void DistillerPage::OnLoadURLDone() {
- DCHECK_EQ(LOADING_PAGE, state_);
- state_ = PAGE_AVAILABLE;
- ExecuteJavaScript();
-}
-
-void DistillerPage::OnLoadURLFailed() {
- state_ = PAGELOAD_FAILED;
- base::MessageLoop::current()->PostTask(
- FROM_HERE,
- base::Bind(distiller_page_callback_,
- base::Passed(scoped_ptr<DistilledPageInfo>()),
- false));
+ DistillPageImpl(gurl, script);
}
-void DistillerPage::OnExecuteJavaScriptDone(const GURL& page_url,
- const base::Value* value) {
- DCHECK_EQ(EXECUTING_JAVASCRIPT, state_);
- state_ = PAGE_AVAILABLE;
+void DistillerPage::OnDistillationDone(const GURL& page_url,
+ const base::Value* value) {
+ DCHECK(!ready_);
+ ready_ = true;
scoped_ptr<DistilledPageInfo> page_info(new DistilledPageInfo());
std::string result;
const base::ListValue* result_list = NULL;
diff --git a/components/dom_distiller/core/distiller_page.h b/components/dom_distiller/core/distiller_page.h
index a5c06cb..d0d8222 100644
--- a/components/dom_distiller/core/distiller_page.h
+++ b/components/dom_distiller/core/distiller_page.h
@@ -40,59 +40,26 @@ class DistillerPage {
DistillerPage();
virtual ~DistillerPage();
- // Initializes a |DistillerPage|. It must be called before any
- // other functions, and must only be called once.
- void Init();
-
- // Loads a URL. |OnLoadURLDone| is called when the load completes or fails.
- // May be called when the distiller is idle or a page is available.
+ // Loads a URL. |OnDistillationDone| is called when the load completes or
+ // fails. May be called when the distiller is idle.
void DistillPage(const GURL& url, const DistillerPageCallback& callback);
- virtual void OnLoadURLDone();
- virtual void OnLoadURLFailed();
-
- // Injects and executes JavaScript in the context of a loaded page. |LoadURL|
- // must complete before this function is called. May be called only when
- // a page is available.
- void ExecuteJavaScript();
// Called when the JavaScript execution completes. |page_url| is the url of
// the distilled page. |value| contains data returned by the script.
- virtual void OnExecuteJavaScriptDone(const GURL& page_url,
- const base::Value* value);
+ virtual void OnDistillationDone(const GURL& page_url,
+ const base::Value* value);
protected:
- enum State {
- // No context has yet been set in which to load or distill a page.
- NO_CONTEXT,
- // The page distiller has been initialized and is idle.
- IDLE,
- // A page is currently loading.
- LOADING_PAGE,
- // A page has loaded within the specified context.
- PAGE_AVAILABLE,
- // There was an error processing the page.
- PAGELOAD_FAILED,
- // JavaScript is executing within the context of the page. When the
- // JavaScript completes, the state will be returned to |PAGE_AVAILABLE|.
- EXECUTING_JAVASCRIPT
- };
-
- // Called by |Init| to do plaform-specific initialization work set up an
- // environment in which a page can be loaded.
- virtual void InitImpl() = 0;
-
- // Called by |LoadURL| to carry out platform-specific instructions to load a
- // page.
- virtual void LoadURLImpl(const GURL& gurl) = 0;
+ // Called by |DistillPage| to carry out platform-specific instructions to load
+ // a page.
+ virtual void DistillPageImpl(const GURL& gurl, const std::string& script) = 0;
// Called by |ExecuteJavaScript| to carry out platform-specific instructions
// to inject and execute JavaScript within the context of the loaded page.
- virtual void ExecuteJavaScriptImpl(const std::string& script) = 0;
-
- // The current state of the |DistillerPage|, initially |NO_CONTEXT|.
- State state_;
+ //virtual void ExecuteJavaScriptImpl() = 0;
private:
+ bool ready_;
DistillerPageCallback distiller_page_callback_;
DISALLOW_COPY_AND_ASSIGN(DistillerPage);
};
diff --git a/components/dom_distiller/core/distiller_unittest.cc b/components/dom_distiller/core/distiller_unittest.cc
index d4449d1..1590cfd 100644
--- a/components/dom_distiller/core/distiller_unittest.cc
+++ b/components/dom_distiller/core/distiller_unittest.cc
@@ -233,9 +233,7 @@ class MockDistillerURLFetcherFactory : public DistillerURLFetcherFactory {
class MockDistillerPage : public DistillerPage {
public:
- MOCK_METHOD0(InitImpl, void());
- MOCK_METHOD1(LoadURLImpl, void(const GURL& gurl));
- MOCK_METHOD1(ExecuteJavaScriptImpl, void(const string& script));
+ MOCK_METHOD2(DistillPageImpl, void(const GURL& gurl, const string& script));
MockDistillerPage() {}
};
@@ -276,18 +274,15 @@ class DistillerTest : public testing::Test {
TestDistillerURLFetcherFactory url_fetcher_factory_;
};
-ACTION_P3(DistillerPageOnExecuteJavaScriptDone, distiller_page, url, list) {
- distiller_page->OnExecuteJavaScriptDone(url, list);
+ACTION_P3(DistillerPageOnDistillationDone, distiller_page, url, list) {
+ distiller_page->OnDistillationDone(url, list);
}
ACTION_P2(CreateMockDistillerPage, list, kurl) {
MockDistillerPage* distiller_page = new MockDistillerPage();
- EXPECT_CALL(*distiller_page, InitImpl());
- EXPECT_CALL(*distiller_page, LoadURLImpl(kurl))
- .WillOnce(testing::InvokeWithoutArgs(distiller_page,
- &DistillerPage::OnLoadURLDone));
- EXPECT_CALL(*distiller_page, ExecuteJavaScriptImpl(_)).WillOnce(
- DistillerPageOnExecuteJavaScriptDone(distiller_page, kurl, list));
+ EXPECT_CALL(*distiller_page, DistillPageImpl(kurl, _))
+ .WillOnce(
+ DistillerPageOnDistillationDone(distiller_page, kurl, list));
return distiller_page;
}
@@ -296,11 +291,7 @@ ACTION_P2(CreateMockDistillerPageWithPendingJSCallback,
kurl) {
MockDistillerPage* distiller_page = new MockDistillerPage();
*distiller_page_ptr = distiller_page;
- EXPECT_CALL(*distiller_page, InitImpl());
- EXPECT_CALL(*distiller_page, LoadURLImpl(kurl))
- .WillOnce(testing::InvokeWithoutArgs(distiller_page,
- &DistillerPage::OnLoadURLDone));
- EXPECT_CALL(*distiller_page, ExecuteJavaScriptImpl(_));
+ EXPECT_CALL(*distiller_page, DistillPageImpl(kurl, _));
return distiller_page;
}
@@ -309,18 +300,14 @@ ACTION_P3(CreateMockDistillerPages,
pages_size,
start_page_num) {
MockDistillerPage* distiller_page = new MockDistillerPage();
- EXPECT_CALL(*distiller_page, InitImpl());
{
testing::InSequence s;
vector<int> page_nums = GetPagesInSequence(start_page_num, pages_size);
for (size_t page_num = 0; page_num < pages_size; ++page_num) {
int page = page_nums[page_num];
GURL url = GURL(distiller_data->page_urls[page]);
- EXPECT_CALL(*distiller_page, LoadURLImpl(url))
- .WillOnce(testing::InvokeWithoutArgs(distiller_page,
- &DistillerPage::OnLoadURLDone));
- EXPECT_CALL(*distiller_page, ExecuteJavaScriptImpl(_))
- .WillOnce(DistillerPageOnExecuteJavaScriptDone(
+ EXPECT_CALL(*distiller_page, DistillPageImpl(url, _))
+ .WillOnce(DistillerPageOnDistillationDone(
distiller_page, url, distiller_data->distilled_values[page]));
}
}
@@ -334,7 +321,6 @@ TEST_F(DistillerTest, DistillPage) {
EXPECT_CALL(page_factory_, CreateDistillerPageMock())
.WillOnce(CreateMockDistillerPage(list.get(), GURL(kURL)));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
- distiller_->Init();
DistillPage(kURL);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ(kTitle, article_proto_->title());
@@ -354,7 +340,6 @@ TEST_F(DistillerTest, DistillPageWithImages) {
EXPECT_CALL(page_factory_, CreateDistillerPageMock())
.WillOnce(CreateMockDistillerPage(list.get(), GURL(kURL)));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
- distiller_->Init();
DistillPage(kURL);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ(kTitle, article_proto_->title());
@@ -392,7 +377,6 @@ TEST_F(DistillerTest, DistillMultiplePages) {
.WillOnce(CreateMockDistillerPages(distiller_data.get(), kNumPages, 0));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
- distiller_->Init();
DistillPage(distiller_data->page_urls[0]);
base::MessageLoop::current()->RunUntilIdle();
VerifyArticleProtoMatchesMultipageData(
@@ -408,7 +392,6 @@ TEST_F(DistillerTest, DistillLinkLoop) {
EXPECT_CALL(page_factory_, CreateDistillerPageMock())
.WillOnce(CreateMockDistillerPage(list.get(), GURL(kURL)));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
- distiller_->Init();
DistillPage(kURL);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ(kTitle, article_proto_->title());
@@ -441,7 +424,6 @@ TEST_F(DistillerTest, CheckMaxPageLimitExtraPage) {
distiller_->SetMaxNumPagesInArticle(kMaxPagesInArticle);
- distiller_->Init();
DistillPage(distiller_data->page_urls[0]);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ(kTitle, article_proto_->title());
@@ -462,8 +444,6 @@ TEST_F(DistillerTest, CheckMaxPageLimitExactLimit) {
// Check if distilling an article with exactly the page limit works.
distiller_->SetMaxNumPagesInArticle(kMaxPagesInArticle);
- distiller_->Init();
-
DistillPage(distiller_data->page_urls[0]);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ(kTitle, article_proto_->title());
@@ -478,7 +458,6 @@ TEST_F(DistillerTest, SinglePageDistillationFailure) {
EXPECT_CALL(page_factory_, CreateDistillerPageMock())
.WillOnce(CreateMockDistillerPage(nullValue.get(), GURL(kURL)));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
- distiller_->Init();
DistillPage(kURL);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ("", article_proto_->title());
@@ -504,7 +483,6 @@ TEST_F(DistillerTest, MultiplePagesDistillationFailure) {
CreateMockDistillerPages(distiller_data.get(), failed_page_num + 1, 0));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
- distiller_->Init();
DistillPage(distiller_data->page_urls[0]);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ(kTitle, article_proto_->title());
@@ -526,7 +504,6 @@ TEST_F(DistillerTest, DistillPreviousPage) {
distiller_data.get(), kNumPages, start_page_num));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
- distiller_->Init();
DistillPage(distiller_data->page_urls[start_page_num]);
base::MessageLoop::current()->RunUntilIdle();
VerifyArticleProtoMatchesMultipageData(
@@ -547,7 +524,6 @@ TEST_F(DistillerTest, IncrementalUpdates) {
distiller_data.get(), kNumPages, start_page_num));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
- distiller_->Init();
DistillPage(distiller_data->page_urls[start_page_num]);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ(kTitle, article_proto_->title());
@@ -570,7 +546,6 @@ TEST_F(DistillerTest, IncrementalUpdatesDoNotDeleteFinalArticle) {
distiller_data.get(), kNumPages, start_page_num));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
- distiller_->Init();
DistillPage(distiller_data->page_urls[start_page_num]);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ(kNumPages, in_sequence_updates_.size());
@@ -595,7 +570,6 @@ TEST_F(DistillerTest, DeletingArticleDoesNotInterfereWithUpdates) {
distiller_data.get(), kNumPages, start_page_num));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
- distiller_->Init();
DistillPage(distiller_data->page_urls[start_page_num]);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ(kNumPages, in_sequence_updates_.size());
@@ -622,7 +596,6 @@ TEST_F(DistillerTest, CancelWithDelayedImageFetchCallback) {
EXPECT_CALL(url_fetcher_factory, CreateDistillerURLFetcher())
.WillOnce(Return(delayed_fetcher));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory));
- distiller_->Init();
DistillPage(kURL);
base::MessageLoop::current()->RunUntilIdle();
@@ -642,13 +615,12 @@ TEST_F(DistillerTest, CancelWithDelayedJSCallback) {
.WillOnce(CreateMockDistillerPageWithPendingJSCallback(&distiller_page,
GURL(kURL)));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
- distiller_->Init();
DistillPage(kURL);
base::MessageLoop::current()->RunUntilIdle();
ASSERT_TRUE(distiller_page);
// Post the task to execute javascript and then delete the distiller.
- distiller_page->OnExecuteJavaScriptDone(GURL(kURL), distilled_value.get());
+ distiller_page->OnDistillationDone(GURL(kURL), distilled_value.get());
distiller_.reset();
base::MessageLoop::current()->RunUntilIdle();