Commit 2ba6b280 authored by kuan@chromium.org's avatar kuan@chromium.org

DomDistiller: fix 0 document width

bug:
When distilling a page with new WebContents, document.offsetWidth in
domdistiller.js is always 0, because the newly created RenderView doesn't have a
size.

fix:
DistillerPageWebContents extends WebContentsDelegate and implements
GetNewSizeForRenderView() to return the size for the new RenderView.  This size
is the container bounds of the WebContents from where the distillation is
triggered.  The size is plumbed through the pipeline from the following sources
all the way down to DistillerPageWebContents.
1) WebUI: WebContents of chrome:://dom-distiller
3) ReadingList: WebContents currently associated with the extension
2) standalone ContentExtractor: WebContents of content::Shell
As for the reused WebContents (via "Distill page" in wrench menu), it already
has a size, so we just use it.

BUG=368941,367254
TBR=danakj (ui/gfx/size.h)

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@284094 0039d316-1c4b-4281-b951-d872f2087c98
parent db16bf7f
...@@ -76,8 +76,9 @@ scoped_ptr<ViewerHandle> LazyDomDistillerService::ViewUrl( ...@@ -76,8 +76,9 @@ scoped_ptr<ViewerHandle> LazyDomDistillerService::ViewUrl(
} }
scoped_ptr<DistillerPage> scoped_ptr<DistillerPage>
LazyDomDistillerService::CreateDefaultDistillerPage() { LazyDomDistillerService::CreateDefaultDistillerPage(
return instance()->CreateDefaultDistillerPage(); const gfx::Size& render_view_size) {
return instance()->CreateDefaultDistillerPage(render_view_size);
} }
scoped_ptr<DistillerPage> scoped_ptr<DistillerPage>
......
...@@ -49,7 +49,8 @@ class LazyDomDistillerService : public DomDistillerServiceInterface, ...@@ -49,7 +49,8 @@ class LazyDomDistillerService : public DomDistillerServiceInterface,
ViewRequestDelegate* delegate, ViewRequestDelegate* delegate,
scoped_ptr<DistillerPage> distiller_page, scoped_ptr<DistillerPage> distiller_page,
const GURL& url) OVERRIDE; const GURL& url) OVERRIDE;
virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPage() OVERRIDE; virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPage(
const gfx::Size& render_view_size) OVERRIDE;
virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPageWithHandle( virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPageWithHandle(
scoped_ptr<SourcePageHandle> handle) OVERRIDE; scoped_ptr<SourcePageHandle> handle) OVERRIDE;
virtual void AddObserver(DomDistillerObserver* observer) OVERRIDE; virtual void AddObserver(DomDistillerObserver* observer) OVERRIDE;
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "chrome/common/extensions/api/reading_list_private.h" #include "chrome/common/extensions/api/reading_list_private.h"
#include "components/dom_distiller/core/article_entry.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_service.h"
#include "content/public/browser/web_contents.h"
namespace extensions { namespace extensions {
...@@ -37,9 +38,13 @@ bool ReadingListPrivateAddEntryFunction::RunAsync() { ...@@ -37,9 +38,13 @@ bool ReadingListPrivateAddEntryFunction::RunAsync() {
DomDistillerService* service = DomDistillerService* service =
DomDistillerServiceFactory::GetForBrowserContext(GetProfile()); DomDistillerServiceFactory::GetForBrowserContext(GetProfile());
gfx::Size render_view_size;
content::WebContents* web_contents = GetAssociatedWebContents();
if (web_contents)
render_view_size = web_contents->GetContainerBounds().size();
const std::string& id = service->AddToList( const std::string& id = service->AddToList(
url_to_add, url_to_add,
service->CreateDefaultDistillerPage().Pass(), service->CreateDefaultDistillerPage(render_view_size).Pass(),
base::Bind(&ReadingListPrivateAddEntryFunction::SendResponse, this)); base::Bind(&ReadingListPrivateAddEntryFunction::SendResponse, this));
Entry new_entry; Entry new_entry;
new_entry.id = id; new_entry.id = id;
......
...@@ -12,6 +12,7 @@ include_rules = [ ...@@ -12,6 +12,7 @@ include_rules = [
"+net/url_request", "+net/url_request",
"+ui/base/l10n", "+ui/base/l10n",
"+ui/base/resource", "+ui/base/resource",
"+ui/gfx",
# The dom distiller is a layered component; subdirectories must explicitly # The dom distiller is a layered component; subdirectories must explicitly
# introduce the ability to use the content layer as appropriate. # introduce the ability to use the content layer as appropriate.
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "content/public/browser/render_view_host.h" #include "content/public/browser/render_view_host.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_observer.h" #include "content/public/browser/web_contents_observer.h"
#include "ui/gfx/screen.h"
#include "url/gurl.h" #include "url/gurl.h"
namespace dom_distiller { namespace dom_distiller {
...@@ -33,11 +34,12 @@ scoped_ptr<content::WebContents> SourcePageHandleWebContents::GetWebContents() { ...@@ -33,11 +34,12 @@ scoped_ptr<content::WebContents> SourcePageHandleWebContents::GetWebContents() {
return web_contents_.Pass(); return web_contents_.Pass();
} }
scoped_ptr<DistillerPage> DistillerPageWebContentsFactory::CreateDistillerPage() scoped_ptr<DistillerPage> DistillerPageWebContentsFactory::CreateDistillerPage(
const { const gfx::Size& render_view_size) const {
DCHECK(browser_context_); DCHECK(browser_context_);
return scoped_ptr<DistillerPage>(new DistillerPageWebContents( return scoped_ptr<DistillerPage>(new DistillerPageWebContents(
browser_context_, scoped_ptr<SourcePageHandleWebContents>())); browser_context_, render_view_size,
scoped_ptr<SourcePageHandleWebContents>()));
} }
scoped_ptr<DistillerPage> scoped_ptr<DistillerPage>
...@@ -48,19 +50,25 @@ DistillerPageWebContentsFactory::CreateDistillerPageWithHandle( ...@@ -48,19 +50,25 @@ DistillerPageWebContentsFactory::CreateDistillerPageWithHandle(
scoped_ptr<SourcePageHandleWebContents>( scoped_ptr<SourcePageHandleWebContents>(
static_cast<SourcePageHandleWebContents*>(handle.release())); static_cast<SourcePageHandleWebContents*>(handle.release()));
return scoped_ptr<DistillerPage>(new DistillerPageWebContents( return scoped_ptr<DistillerPage>(new DistillerPageWebContents(
browser_context_, web_contents_handle.Pass())); browser_context_, gfx::Size(), web_contents_handle.Pass()));
} }
DistillerPageWebContents::DistillerPageWebContents( DistillerPageWebContents::DistillerPageWebContents(
content::BrowserContext* browser_context, content::BrowserContext* browser_context,
const gfx::Size& render_view_size,
scoped_ptr<SourcePageHandleWebContents> optional_web_contents_handle) scoped_ptr<SourcePageHandleWebContents> optional_web_contents_handle)
: state_(IDLE), browser_context_(browser_context) { : state_(IDLE), browser_context_(browser_context),
render_view_size_(render_view_size) {
if (optional_web_contents_handle) { if (optional_web_contents_handle) {
web_contents_ = optional_web_contents_handle->GetWebContents().Pass(); web_contents_ = optional_web_contents_handle->GetWebContents().Pass();
if (render_view_size.IsEmpty())
render_view_size_ = web_contents_->GetContainerBounds().size();
} }
} }
DistillerPageWebContents::~DistillerPageWebContents() { DistillerPageWebContents::~DistillerPageWebContents() {
if (web_contents_)
web_contents_->SetDelegate(NULL);
} }
void DistillerPageWebContents::DistillPageImpl(const GURL& url, void DistillerPageWebContents::DistillPageImpl(const GURL& url,
...@@ -101,12 +109,28 @@ void DistillerPageWebContents::CreateNewWebContents(const GURL& url) { ...@@ -101,12 +109,28 @@ void DistillerPageWebContents::CreateNewWebContents(const GURL& url) {
web_contents_.reset(content::WebContents::Create(create_params)); web_contents_.reset(content::WebContents::Create(create_params));
DCHECK(web_contents_.get()); DCHECK(web_contents_.get());
web_contents_->SetDelegate(this);
// Start observing WebContents and load the requested URL. // Start observing WebContents and load the requested URL.
content::WebContentsObserver::Observe(web_contents_.get()); content::WebContentsObserver::Observe(web_contents_.get());
content::NavigationController::LoadURLParams params(url); content::NavigationController::LoadURLParams params(url);
web_contents_->GetController().LoadURLWithParams(params); web_contents_->GetController().LoadURLWithParams(params);
} }
gfx::Size DistillerPageWebContents::GetSizeForNewRenderView(
content::WebContents* web_contents) const {
gfx::Size size(render_view_size_);
if (size.IsEmpty())
size = web_contents->GetContainerBounds().size();
// If size is still empty, set it to fullscreen so that document.offsetWidth
// in the executed domdistiller.js won't be 0.
if (size.IsEmpty()) {
DVLOG(1) << "Using fullscreen as default RenderView size";
size = gfx::Screen::GetNativeScreen()->GetPrimaryDisplay().size();
}
return size;
}
void DistillerPageWebContents::DocumentLoadedInFrame( void DistillerPageWebContents::DocumentLoadedInFrame(
content::RenderFrameHost* render_frame_host) { content::RenderFrameHost* render_frame_host) {
if (render_frame_host == web_contents_->GetMainFrame()) { if (render_frame_host == web_contents_->GetMainFrame()) {
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/memory/scoped_ptr.h" #include "base/memory/scoped_ptr.h"
#include "components/dom_distiller/core/distiller_page.h" #include "components/dom_distiller/core/distiller_page.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_delegate.h"
#include "content/public/browser/web_contents_observer.h" #include "content/public/browser/web_contents_observer.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -35,7 +36,8 @@ class DistillerPageWebContentsFactory : public DistillerPageFactory { ...@@ -35,7 +36,8 @@ class DistillerPageWebContentsFactory : public DistillerPageFactory {
: DistillerPageFactory(), browser_context_(browser_context) {} : DistillerPageFactory(), browser_context_(browser_context) {}
virtual ~DistillerPageWebContentsFactory() {} virtual ~DistillerPageWebContentsFactory() {}
virtual scoped_ptr<DistillerPage> CreateDistillerPage() const OVERRIDE; virtual scoped_ptr<DistillerPage> CreateDistillerPage(
const gfx::Size& render_view_size) const OVERRIDE;
virtual scoped_ptr<DistillerPage> CreateDistillerPageWithHandle( virtual scoped_ptr<DistillerPage> CreateDistillerPageWithHandle(
scoped_ptr<SourcePageHandle> handle) const OVERRIDE; scoped_ptr<SourcePageHandle> handle) const OVERRIDE;
...@@ -44,13 +46,19 @@ class DistillerPageWebContentsFactory : public DistillerPageFactory { ...@@ -44,13 +46,19 @@ class DistillerPageWebContentsFactory : public DistillerPageFactory {
}; };
class DistillerPageWebContents : public DistillerPage, class DistillerPageWebContents : public DistillerPage,
public content::WebContentsDelegate,
public content::WebContentsObserver { public content::WebContentsObserver {
public: public:
DistillerPageWebContents( DistillerPageWebContents(
content::BrowserContext* browser_context, content::BrowserContext* browser_context,
const gfx::Size& render_view_size,
scoped_ptr<SourcePageHandleWebContents> optional_web_contents_handle); scoped_ptr<SourcePageHandleWebContents> optional_web_contents_handle);
virtual ~DistillerPageWebContents(); virtual ~DistillerPageWebContents();
// content::WebContentsDelegate implementation.
virtual gfx::Size GetSizeForNewRenderView(
content::WebContents* web_contents) const OVERRIDE;
// content::WebContentsObserver implementation. // content::WebContentsObserver implementation.
virtual void DocumentLoadedInFrame( virtual void DocumentLoadedInFrame(
content::RenderFrameHost* render_frame_host) OVERRIDE; content::RenderFrameHost* render_frame_host) OVERRIDE;
...@@ -99,6 +107,7 @@ class DistillerPageWebContents : public DistillerPage, ...@@ -99,6 +107,7 @@ class DistillerPageWebContents : public DistillerPage,
scoped_ptr<content::WebContents> web_contents_; scoped_ptr<content::WebContents> web_contents_;
content::BrowserContext* browser_context_; content::BrowserContext* browser_context_;
gfx::Size render_view_size_;
DISALLOW_COPY_AND_ASSIGN(DistillerPageWebContents); DISALLOW_COPY_AND_ASSIGN(DistillerPageWebContents);
}; };
......
...@@ -89,9 +89,10 @@ class TestDistillerPageWebContents : public DistillerPageWebContents { ...@@ -89,9 +89,10 @@ class TestDistillerPageWebContents : public DistillerPageWebContents {
public: public:
TestDistillerPageWebContents( TestDistillerPageWebContents(
content::BrowserContext* browser_context, content::BrowserContext* browser_context,
const gfx::Size& render_view_size,
scoped_ptr<SourcePageHandleWebContents> optional_web_contents_handle, scoped_ptr<SourcePageHandleWebContents> optional_web_contents_handle,
bool expect_new_web_contents) bool expect_new_web_contents)
: DistillerPageWebContents(browser_context, : DistillerPageWebContents(browser_context, render_view_size,
optional_web_contents_handle.Pass()), optional_web_contents_handle.Pass()),
expect_new_web_contents_(expect_new_web_contents), expect_new_web_contents_(expect_new_web_contents),
new_web_contents_created_(false) {} new_web_contents_created_(false) {}
...@@ -161,6 +162,7 @@ class WebContentsMainFrameHelper : public content::WebContentsObserver { ...@@ -161,6 +162,7 @@ class WebContentsMainFrameHelper : public content::WebContentsObserver {
IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, BasicDistillationWorks) { IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, BasicDistillationWorks) {
DistillerPageWebContents distiller_page( DistillerPageWebContents distiller_page(
shell()->web_contents()->GetBrowserContext(), shell()->web_contents()->GetBrowserContext(),
shell()->web_contents()->GetContainerBounds().size(),
scoped_ptr<SourcePageHandleWebContents>()); scoped_ptr<SourcePageHandleWebContents>());
distiller_page_ = &distiller_page; distiller_page_ = &distiller_page;
...@@ -178,6 +180,7 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, BasicDistillationWorks) { ...@@ -178,6 +180,7 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, BasicDistillationWorks) {
IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeLinks) { IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeLinks) {
DistillerPageWebContents distiller_page( DistillerPageWebContents distiller_page(
shell()->web_contents()->GetBrowserContext(), shell()->web_contents()->GetBrowserContext(),
shell()->web_contents()->GetContainerBounds().size(),
scoped_ptr<SourcePageHandleWebContents>()); scoped_ptr<SourcePageHandleWebContents>());
distiller_page_ = &distiller_page; distiller_page_ = &distiller_page;
...@@ -195,6 +198,7 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeLinks) { ...@@ -195,6 +198,7 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeLinks) {
IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeImages) { IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeImages) {
DistillerPageWebContents distiller_page( DistillerPageWebContents distiller_page(
shell()->web_contents()->GetBrowserContext(), shell()->web_contents()->GetBrowserContext(),
shell()->web_contents()->GetContainerBounds().size(),
scoped_ptr<SourcePageHandleWebContents>()); scoped_ptr<SourcePageHandleWebContents>());
distiller_page_ = &distiller_page; distiller_page_ = &distiller_page;
...@@ -213,6 +217,7 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeImages) { ...@@ -213,6 +217,7 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeImages) {
IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeVideos) { IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeVideos) {
DistillerPageWebContents distiller_page( DistillerPageWebContents distiller_page(
shell()->web_contents()->GetBrowserContext(), shell()->web_contents()->GetBrowserContext(),
shell()->web_contents()->GetContainerBounds().size(),
scoped_ptr<SourcePageHandleWebContents>()); scoped_ptr<SourcePageHandleWebContents>());
distiller_page_ = &distiller_page; distiller_page_ = &distiller_page;
...@@ -238,6 +243,7 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeVideos) { ...@@ -238,6 +243,7 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeVideos) {
IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, VisibilityDetection) { IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, VisibilityDetection) {
DistillerPageWebContents distiller_page( DistillerPageWebContents distiller_page(
shell()->web_contents()->GetBrowserContext(), shell()->web_contents()->GetBrowserContext(),
shell()->web_contents()->GetContainerBounds().size(),
scoped_ptr<SourcePageHandleWebContents>()); scoped_ptr<SourcePageHandleWebContents>());
distiller_page_ = &distiller_page; distiller_page_ = &distiller_page;
...@@ -334,6 +340,7 @@ void DistillerPageWebContentsTest::RunUseCurrentWebContentsTest( ...@@ -334,6 +340,7 @@ void DistillerPageWebContentsTest::RunUseCurrentWebContentsTest(
TestDistillerPageWebContents distiller_page( TestDistillerPageWebContents distiller_page(
shell()->web_contents()->GetBrowserContext(), shell()->web_contents()->GetBrowserContext(),
shell()->web_contents()->GetContainerBounds().size(),
source_page_handle.Pass(), source_page_handle.Pass(),
expect_new_web_contents); expect_new_web_contents);
distiller_page_ = &distiller_page; distiller_page_ = &distiller_page;
......
...@@ -285,7 +285,8 @@ void DomDistillerViewerSource::StartDataRequest( ...@@ -285,7 +285,8 @@ void DomDistillerViewerSource::StartDataRequest(
web_contents, scheme_, path_after_query_separator, callback, web_contents, scheme_, path_after_query_separator, callback,
dom_distiller_service_->GetDistilledPagePrefs()); dom_distiller_service_->GetDistilledPagePrefs());
scoped_ptr<ViewerHandle> viewer_handle = viewer::CreateViewRequest( scoped_ptr<ViewerHandle> viewer_handle = viewer::CreateViewRequest(
dom_distiller_service_, path, request_viewer_handle); dom_distiller_service_, path, request_viewer_handle,
web_contents->GetContainerBounds().size());
if (viewer_handle) { if (viewer_handle) {
// The service returned a |ViewerHandle| and guarantees it will call // The service returned a |ViewerHandle| and guarantees it will call
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/values.h" #include "base/values.h"
#include "third_party/dom_distiller_js/dom_distiller.pb.h" #include "third_party/dom_distiller_js/dom_distiller.pb.h"
#include "ui/gfx/size.h"
#include "url/gurl.h" #include "url/gurl.h"
namespace dom_distiller { namespace dom_distiller {
...@@ -81,7 +82,8 @@ class DistillerPageFactory { ...@@ -81,7 +82,8 @@ class DistillerPageFactory {
// Constructs and returns a new DistillerPage. The implementation of this // Constructs and returns a new DistillerPage. The implementation of this
// should be very cheap, since the pages can be thrown away without being // should be very cheap, since the pages can be thrown away without being
// used. // used.
virtual scoped_ptr<DistillerPage> CreateDistillerPage() const = 0; virtual scoped_ptr<DistillerPage> CreateDistillerPage(
const gfx::Size& render_view_size) const = 0;
virtual scoped_ptr<DistillerPage> CreateDistillerPageWithHandle( virtual scoped_ptr<DistillerPage> CreateDistillerPageWithHandle(
scoped_ptr<SourcePageHandle> handle) const = 0; scoped_ptr<SourcePageHandle> handle) const = 0;
}; };
......
...@@ -55,8 +55,9 @@ syncer::SyncableService* DomDistillerService::GetSyncableService() const { ...@@ -55,8 +55,9 @@ syncer::SyncableService* DomDistillerService::GetSyncableService() const {
return store_->GetSyncableService(); return store_->GetSyncableService();
} }
scoped_ptr<DistillerPage> DomDistillerService::CreateDefaultDistillerPage() { scoped_ptr<DistillerPage> DomDistillerService::CreateDefaultDistillerPage(
return distiller_page_factory_->CreateDistillerPage().Pass(); const gfx::Size& render_view_size) {
return distiller_page_factory_->CreateDistillerPage(render_view_size).Pass();
} }
scoped_ptr<DistillerPage> scoped_ptr<DistillerPage>
......
...@@ -84,7 +84,8 @@ class DomDistillerServiceInterface { ...@@ -84,7 +84,8 @@ class DomDistillerServiceInterface {
const GURL& url) = 0; const GURL& url) = 0;
// Creates a default DistillerPage. // Creates a default DistillerPage.
virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPage() = 0; virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPage(
const gfx::Size& render_view_size) = 0;
virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPageWithHandle( virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPageWithHandle(
scoped_ptr<SourcePageHandle> handle) = 0; scoped_ptr<SourcePageHandle> handle) = 0;
...@@ -128,7 +129,8 @@ class DomDistillerService : public DomDistillerServiceInterface { ...@@ -128,7 +129,8 @@ class DomDistillerService : public DomDistillerServiceInterface {
ViewRequestDelegate* delegate, ViewRequestDelegate* delegate,
scoped_ptr<DistillerPage> distiller_page, scoped_ptr<DistillerPage> distiller_page,
const GURL& url) OVERRIDE; const GURL& url) OVERRIDE;
virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPage() OVERRIDE; virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPage(
const gfx::Size& render_view_size) OVERRIDE;
virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPageWithHandle( virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPageWithHandle(
scoped_ptr<SourcePageHandle> handle) OVERRIDE; scoped_ptr<SourcePageHandle> handle) OVERRIDE;
virtual void AddObserver(DomDistillerObserver* observer) OVERRIDE; virtual void AddObserver(DomDistillerObserver* observer) OVERRIDE;
......
...@@ -128,7 +128,8 @@ TEST_F(DomDistillerServiceTest, TestViewEntry) { ...@@ -128,7 +128,8 @@ TEST_F(DomDistillerServiceTest, TestViewEntry) {
FakeViewRequestDelegate viewer_delegate; FakeViewRequestDelegate viewer_delegate;
scoped_ptr<ViewerHandle> handle = service_->ViewEntry( scoped_ptr<ViewerHandle> handle = service_->ViewEntry(
&viewer_delegate, service_->CreateDefaultDistillerPage(), entry_id); &viewer_delegate, service_->CreateDefaultDistillerPage(gfx::Size()),
entry_id);
ASSERT_FALSE(distiller->GetArticleCallback().is_null()); ASSERT_FALSE(distiller->GetArticleCallback().is_null());
...@@ -146,7 +147,7 @@ TEST_F(DomDistillerServiceTest, TestViewUrl) { ...@@ -146,7 +147,7 @@ TEST_F(DomDistillerServiceTest, TestViewUrl) {
FakeViewRequestDelegate viewer_delegate; FakeViewRequestDelegate viewer_delegate;
GURL url("http://www.example.com/p1"); GURL url("http://www.example.com/p1");
scoped_ptr<ViewerHandle> handle = service_->ViewUrl( scoped_ptr<ViewerHandle> handle = service_->ViewUrl(
&viewer_delegate, service_->CreateDefaultDistillerPage(), url); &viewer_delegate, service_->CreateDefaultDistillerPage(gfx::Size()), url);
ASSERT_FALSE(distiller->GetArticleCallback().is_null()); ASSERT_FALSE(distiller->GetArticleCallback().is_null());
EXPECT_EQ(url, distiller->GetUrl()); EXPECT_EQ(url, distiller->GetUrl());
...@@ -171,9 +172,10 @@ TEST_F(DomDistillerServiceTest, TestMultipleViewUrl) { ...@@ -171,9 +172,10 @@ TEST_F(DomDistillerServiceTest, TestMultipleViewUrl) {
GURL url2("http://www.example.com/a/p1"); GURL url2("http://www.example.com/a/p1");
scoped_ptr<ViewerHandle> handle = service_->ViewUrl( scoped_ptr<ViewerHandle> handle = service_->ViewUrl(
&viewer_delegate, service_->CreateDefaultDistillerPage(), url); &viewer_delegate, service_->CreateDefaultDistillerPage(gfx::Size()), url);
scoped_ptr<ViewerHandle> handle2 = service_->ViewUrl( scoped_ptr<ViewerHandle> handle2 = service_->ViewUrl(
&viewer_delegate2, service_->CreateDefaultDistillerPage(), url2); &viewer_delegate2, service_->CreateDefaultDistillerPage(gfx::Size()),
url2);
ASSERT_FALSE(distiller->GetArticleCallback().is_null()); ASSERT_FALSE(distiller->GetArticleCallback().is_null());
EXPECT_EQ(url, distiller->GetUrl()); EXPECT_EQ(url, distiller->GetUrl());
...@@ -204,7 +206,7 @@ TEST_F(DomDistillerServiceTest, TestViewUrlCancelled) { ...@@ -204,7 +206,7 @@ TEST_F(DomDistillerServiceTest, TestViewUrlCancelled) {
FakeViewRequestDelegate viewer_delegate; FakeViewRequestDelegate viewer_delegate;
GURL url("http://www.example.com/p1"); GURL url("http://www.example.com/p1");
scoped_ptr<ViewerHandle> handle = service_->ViewUrl( scoped_ptr<ViewerHandle> handle = service_->ViewUrl(
&viewer_delegate, service_->CreateDefaultDistillerPage(), url); &viewer_delegate, service_->CreateDefaultDistillerPage(gfx::Size()), url);
ASSERT_FALSE(distiller->GetArticleCallback().is_null()); ASSERT_FALSE(distiller->GetArticleCallback().is_null());
EXPECT_EQ(url, distiller->GetUrl()); EXPECT_EQ(url, distiller->GetUrl());
...@@ -226,7 +228,7 @@ TEST_F(DomDistillerServiceTest, TestViewUrlDoesNotAddEntry) { ...@@ -226,7 +228,7 @@ TEST_F(DomDistillerServiceTest, TestViewUrlDoesNotAddEntry) {
FakeViewRequestDelegate viewer_delegate; FakeViewRequestDelegate viewer_delegate;
GURL url("http://www.example.com/p1"); GURL url("http://www.example.com/p1");
scoped_ptr<ViewerHandle> handle = service_->ViewUrl( scoped_ptr<ViewerHandle> handle = service_->ViewUrl(
&viewer_delegate, service_->CreateDefaultDistillerPage(), url); &viewer_delegate, service_->CreateDefaultDistillerPage(gfx::Size()), url);
scoped_ptr<DistilledArticleProto> proto = CreateArticleWithURL(url.spec()); scoped_ptr<DistilledArticleProto> proto = CreateArticleWithURL(url.spec());
EXPECT_CALL(viewer_delegate, OnArticleReady(proto.get())); EXPECT_CALL(viewer_delegate, OnArticleReady(proto.get()));
...@@ -248,8 +250,9 @@ TEST_F(DomDistillerServiceTest, TestAddAndRemoveEntry) { ...@@ -248,8 +250,9 @@ TEST_F(DomDistillerServiceTest, TestAddAndRemoveEntry) {
EXPECT_CALL(article_cb, DistillationCompleted(true)); EXPECT_CALL(article_cb, DistillationCompleted(true));
std::string entry_id = std::string entry_id =
service_->AddToList(url, service_->CreateDefaultDistillerPage().Pass(), service_->AddToList(url,
ArticleCallback(&article_cb)); service_->CreateDefaultDistillerPage(gfx::Size()).Pass(),
ArticleCallback(&article_cb));
ASSERT_FALSE(distiller->GetArticleCallback().is_null()); ASSERT_FALSE(distiller->GetArticleCallback().is_null());
EXPECT_EQ(url, distiller->GetUrl()); EXPECT_EQ(url, distiller->GetUrl());
...@@ -279,8 +282,9 @@ TEST_F(DomDistillerServiceTest, TestCancellation) { ...@@ -279,8 +282,9 @@ TEST_F(DomDistillerServiceTest, TestCancellation) {
GURL url("http://www.example.com/p1"); GURL url("http://www.example.com/p1");
std::string entry_id = std::string entry_id =
service_->AddToList(url, service_->CreateDefaultDistillerPage().Pass(), service_->AddToList(url,
ArticleCallback(&article_cb)); service_->CreateDefaultDistillerPage(gfx::Size()).Pass(),
ArticleCallback(&article_cb));
// Remove entry will cause the |article_cb| to be called with false value. // Remove entry will cause the |article_cb| to be called with false value.
service_->RemoveEntry(entry_id); service_->RemoveEntry(entry_id);
...@@ -301,7 +305,8 @@ TEST_F(DomDistillerServiceTest, TestMultipleObservers) { ...@@ -301,7 +305,8 @@ TEST_F(DomDistillerServiceTest, TestMultipleObservers) {
DomDistillerService::ArticleAvailableCallback article_cb; DomDistillerService::ArticleAvailableCallback article_cb;
GURL url("http://www.example.com/p1"); GURL url("http://www.example.com/p1");
std::string entry_id = service_->AddToList( std::string entry_id = service_->AddToList(
url, service_->CreateDefaultDistillerPage().Pass(), article_cb); url, service_->CreateDefaultDistillerPage(gfx::Size()).Pass(),
article_cb);
// Distillation should notify all observers that article is added. // Distillation should notify all observers that article is added.
std::vector<DomDistillerObserver::ArticleUpdate> expected_updates; std::vector<DomDistillerObserver::ArticleUpdate> expected_updates;
...@@ -341,14 +346,17 @@ TEST_F(DomDistillerServiceTest, TestMultipleCallbacks) { ...@@ -341,14 +346,17 @@ TEST_F(DomDistillerServiceTest, TestMultipleCallbacks) {
// Adding a URL and then distilling calls all clients. // Adding a URL and then distilling calls all clients.
GURL url("http://www.example.com/p1"); GURL url("http://www.example.com/p1");
const std::string entry_id = const std::string entry_id =
service_->AddToList(url, service_->CreateDefaultDistillerPage().Pass(), service_->AddToList(url,
ArticleCallback(&article_cb[0])); service_->CreateDefaultDistillerPage(gfx::Size()).Pass(),
ArticleCallback(&article_cb[0]));
EXPECT_CALL(article_cb[0], DistillationCompleted(true)); EXPECT_CALL(article_cb[0], DistillationCompleted(true));
for (int i = 1; i < kClientsCount; ++i) { for (int i = 1; i < kClientsCount; ++i) {
EXPECT_EQ(entry_id, service_->AddToList( EXPECT_EQ(entry_id,
url, service_->CreateDefaultDistillerPage().Pass(), service_->AddToList(
ArticleCallback(&article_cb[i]))); url,
service_->CreateDefaultDistillerPage(gfx::Size()).Pass(),
ArticleCallback(&article_cb[i])));
EXPECT_CALL(article_cb[i], DistillationCompleted(true)); EXPECT_CALL(article_cb[i], DistillationCompleted(true));
} }
...@@ -358,9 +366,11 @@ TEST_F(DomDistillerServiceTest, TestMultipleCallbacks) { ...@@ -358,9 +366,11 @@ TEST_F(DomDistillerServiceTest, TestMultipleCallbacks) {
// Add the same url again, all callbacks should be called with true. // Add the same url again, all callbacks should be called with true.
for (int i = 0; i < kClientsCount; ++i) { for (int i = 0; i < kClientsCount; ++i) {
EXPECT_CALL(article_cb[i], DistillationCompleted(true)); EXPECT_CALL(article_cb[i], DistillationCompleted(true));
EXPECT_EQ(entry_id, service_->AddToList( EXPECT_EQ(entry_id,
url, service_->CreateDefaultDistillerPage().Pass(), service_->AddToList(
ArticleCallback(&article_cb[i]))); url,
service_->CreateDefaultDistillerPage(gfx::Size()).Pass(),
ArticleCallback(&article_cb[i])));
} }
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
...@@ -377,14 +387,16 @@ TEST_F(DomDistillerServiceTest, TestMultipleCallbacksOnRemove) { ...@@ -377,14 +387,16 @@ TEST_F(DomDistillerServiceTest, TestMultipleCallbacksOnRemove) {
// called with false. // called with false.
GURL url("http://www.example.com/p1"); GURL url("http://www.example.com/p1");
const std::string entry_id = const std::string entry_id =
service_->AddToList(url, service_->CreateDefaultDistillerPage().Pass(), service_->AddToList(url,
ArticleCallback(&article_cb[0])); service_->CreateDefaultDistillerPage(gfx::Size()).Pass(),
ArticleCallback(&article_cb[0]));
EXPECT_CALL(article_cb[0], DistillationCompleted(false)); EXPECT_CALL(article_cb[0], DistillationCompleted(false));
for (int i = 1; i < kClientsCount; ++i) { for (int i = 1; i < kClientsCount; ++i) {
EXPECT_EQ(entry_id, service_->AddToList( EXPECT_EQ(entry_id,
url, service_->CreateDefaultDistillerPage().Pass(), service_->AddToList(
ArticleCallback(&article_cb[i]))); url, service_->CreateDefaultDistillerPage(gfx::Size()).Pass(),
ArticleCallback(&article_cb[i])));
EXPECT_CALL(article_cb[i], DistillationCompleted(false)); EXPECT_CALL(article_cb[i], DistillationCompleted(false));
} }
...@@ -409,7 +421,7 @@ TEST_F(DomDistillerServiceTest, TestMultiplePageArticle) { ...@@ -409,7 +421,7 @@ TEST_F(DomDistillerServiceTest, TestMultiplePageArticle) {
EXPECT_CALL(article_cb, DistillationCompleted(true)); EXPECT_CALL(article_cb, DistillationCompleted(true));
std::string entry_id = service_->AddToList( std::string entry_id = service_->AddToList(
pages_url[0], service_->CreateDefaultDistillerPage().Pass(), pages_url[0], service_->CreateDefaultDistillerPage(gfx::Size()).Pass(),
ArticleCallback(&article_cb)); ArticleCallback(&article_cb));
ArticleEntry entry; ArticleEntry entry;
......
...@@ -16,7 +16,8 @@ class MockDistillerPageFactory : public DistillerPageFactory { ...@@ -16,7 +16,8 @@ class MockDistillerPageFactory : public DistillerPageFactory {
MockDistillerPageFactory(); MockDistillerPageFactory();
virtual ~MockDistillerPageFactory(); virtual ~MockDistillerPageFactory();
MOCK_CONST_METHOD0(CreateDistillerPageImpl, DistillerPage*()); MOCK_CONST_METHOD0(CreateDistillerPageImpl, DistillerPage*());
virtual scoped_ptr<DistillerPage> CreateDistillerPage() const OVERRIDE { virtual scoped_ptr<DistillerPage> CreateDistillerPage(
const gfx::Size& render_view_size) const OVERRIDE {
return scoped_ptr<DistillerPage>(CreateDistillerPageImpl()); return scoped_ptr<DistillerPage>(CreateDistillerPageImpl());
} }
virtual scoped_ptr<DistillerPage> CreateDistillerPageWithHandle( virtual scoped_ptr<DistillerPage> CreateDistillerPageWithHandle(
......
...@@ -177,7 +177,8 @@ const std::string GetJavaScript() { ...@@ -177,7 +177,8 @@ const std::string GetJavaScript() {
scoped_ptr<ViewerHandle> CreateViewRequest( scoped_ptr<ViewerHandle> CreateViewRequest(
DomDistillerServiceInterface* dom_distiller_service, DomDistillerServiceInterface* dom_distiller_service,
const std::string& path, const std::string& path,
ViewRequestDelegate* view_request_delegate) { ViewRequestDelegate* view_request_delegate,
const gfx::Size& render_view_size) {
std::string entry_id = std::string entry_id =
url_utils::GetValueForKeyInUrlPathQuery(path, kEntryIdKey); url_utils::GetValueForKeyInUrlPathQuery(path, kEntryIdKey);
bool has_valid_entry_id = !entry_id.empty(); bool has_valid_entry_id = !entry_id.empty();
...@@ -195,15 +196,15 @@ scoped_ptr<ViewerHandle> CreateViewRequest( ...@@ -195,15 +196,15 @@ scoped_ptr<ViewerHandle> CreateViewRequest(
} }
if (has_valid_entry_id) { if (has_valid_entry_id) {
return dom_distiller_service->ViewEntry(view_request_delegate, return dom_distiller_service->ViewEntry(
dom_distiller_service view_request_delegate,
->CreateDefaultDistillerPage(), dom_distiller_service->CreateDefaultDistillerPage(render_view_size),
entry_id).Pass(); entry_id).Pass();
} else if (has_valid_url) { } else if (has_valid_url) {
return dom_distiller_service->ViewUrl(view_request_delegate, return dom_distiller_service->ViewUrl(
dom_distiller_service view_request_delegate,
->CreateDefaultDistillerPage(), dom_distiller_service->CreateDefaultDistillerPage(render_view_size),
requested_url).Pass(); requested_url).Pass();
} }
// It is invalid to not specify a query param for |kEntryIdKey| or |kUrlKey|. // It is invalid to not specify a query param for |kEntryIdKey| or |kUrlKey|.
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/memory/scoped_ptr.h" #include "base/memory/scoped_ptr.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "components/dom_distiller/core/distilled_page_prefs.h" #include "components/dom_distiller/core/distilled_page_prefs.h"
#include "ui/gfx/size.h"
namespace dom_distiller { namespace dom_distiller {
...@@ -67,7 +68,8 @@ const std::string GetJavaScript(); ...@@ -67,7 +68,8 @@ const std::string GetJavaScript();
scoped_ptr<ViewerHandle> CreateViewRequest( scoped_ptr<ViewerHandle> CreateViewRequest(
DomDistillerServiceInterface* dom_distiller_service, DomDistillerServiceInterface* dom_distiller_service,
const std::string& path, const std::string& path,
ViewRequestDelegate* view_request_delegate); ViewRequestDelegate* view_request_delegate,
const gfx::Size& render_view_size);
// Returns JavaScript corresponding to setting a specific theme. // Returns JavaScript corresponding to setting a specific theme.
const std::string GetDistilledPageThemeJs(DistilledPagePrefs::Theme theme); const std::string GetDistilledPageThemeJs(DistilledPagePrefs::Theme theme);
......
...@@ -60,7 +60,8 @@ class TestDomDistillerService : public DomDistillerServiceInterface { ...@@ -60,7 +60,8 @@ class TestDomDistillerService : public DomDistillerServiceInterface {
virtual scoped_ptr<ArticleEntry> RemoveEntry(const std::string&) { virtual scoped_ptr<ArticleEntry> RemoveEntry(const std::string&) {
return scoped_ptr<ArticleEntry>(RemoveEntryImpl()); return scoped_ptr<ArticleEntry>(RemoveEntryImpl());
} }
virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPage() { virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPage(
const gfx::Size& render_view_size) {
return scoped_ptr<DistillerPage>(); return scoped_ptr<DistillerPage>();
} }
virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPageWithHandle( virtual scoped_ptr<DistillerPage> CreateDefaultDistillerPageWithHandle(
...@@ -81,7 +82,7 @@ class DomDistillerViewerTest : public testing::Test { ...@@ -81,7 +82,7 @@ class DomDistillerViewerTest : public testing::Test {
const std::string& path, const std::string& path,
ViewRequestDelegate* view_request_delegate) { ViewRequestDelegate* view_request_delegate) {
return viewer::CreateViewRequest( return viewer::CreateViewRequest(
service_.get(), path, view_request_delegate); service_.get(), path, view_request_delegate, gfx::Size());
} }
scoped_ptr<TestDomDistillerService> service_; scoped_ptr<TestDomDistillerService> service_;
......
...@@ -151,10 +151,13 @@ std::string GetReadableArticleString( ...@@ -151,10 +151,13 @@ std::string GetReadableArticleString(
class ContentExtractionRequest : public ViewRequestDelegate { class ContentExtractionRequest : public ViewRequestDelegate {
public: public:
void Start(DomDistillerService* service, base::Closure finished_callback) { void Start(DomDistillerService* service, const gfx::Size& render_view_size,
base::Closure finished_callback) {
finished_callback_ = finished_callback; finished_callback_ = finished_callback;
viewer_handle_ = viewer_handle_ =
service->ViewUrl(this, service->CreateDefaultDistillerPage(), url_); service->ViewUrl(this,
service->CreateDefaultDistillerPage(render_view_size),
url_);
} }
DistilledArticleProto GetArticleCopy() { DistilledArticleProto GetArticleCopy() {
...@@ -252,6 +255,7 @@ class ContentExtractor : public ContentBrowserTest { ...@@ -252,6 +255,7 @@ class ContentExtractor : public ContentBrowserTest {
while (pending_tasks_ < max_tasks_ && next_request_ < requests_.size()) { while (pending_tasks_ < max_tasks_ && next_request_ < requests_.size()) {
requests_[next_request_]->Start( requests_[next_request_]->Start(
service_.get(), service_.get(),
shell()->web_contents()->GetContainerBounds().size(),
base::Bind(&ContentExtractor::FinishRequest, base::Unretained(this))); base::Bind(&ContentExtractor::FinishRequest, base::Unretained(this)));
++next_request_; ++next_request_;
++pending_tasks_; ++pending_tasks_;
......
...@@ -65,7 +65,8 @@ void DomDistillerHandler::HandleAddArticle(const base::ListValue* args) { ...@@ -65,7 +65,8 @@ void DomDistillerHandler::HandleAddArticle(const base::ListValue* args) {
if (gurl.is_valid()) { if (gurl.is_valid()) {
service_->AddToList( service_->AddToList(
gurl, gurl,
service_->CreateDefaultDistillerPage(), service_->CreateDefaultDistillerPage(
web_ui()->GetWebContents()->GetContainerBounds().size()),
base::Bind(base::Bind(&DomDistillerHandler::OnArticleAdded, base::Bind(base::Bind(&DomDistillerHandler::OnArticleAdded,
base::Unretained(this)))); base::Unretained(this))));
} else { } else {
......
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