diff options
Diffstat (limited to 'components/dom_distiller/core')
-rw-r--r-- | components/dom_distiller/core/distiller.cc | 6 | ||||
-rw-r--r-- | components/dom_distiller/core/distiller.h | 4 | ||||
-rw-r--r-- | components/dom_distiller/core/distiller_page.cc | 47 | ||||
-rw-r--r-- | components/dom_distiller/core/distiller_page.h | 51 | ||||
-rw-r--r-- | components/dom_distiller/core/distiller_unittest.cc | 48 |
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(); |