From d289bd7f55ca3d3f647a0a2c6049115e0125b5df Mon Sep 17 00:00:00 2001 From: "nyquist@chromium.org" Date: Wed, 28 May 2014 01:24:12 +0000 Subject: [PATCH] Add support for distilling current WebContents This CL adds the utilities needed for using the current WebContents when distilling web pages. BUG=361939 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272611 Review URL: https://codereview.chromium.org/266073003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@273101 0039d316-1c4b-4281-b951-d872f2087c98 --- .../dom_distiller/DomDistillerTabUtils.java | 31 ++++ chrome/browser/android/chrome_jni_registrar.cc | 2 + .../dom_distiller/lazy_dom_distiller_service.cc | 7 + .../dom_distiller/lazy_dom_distiller_service.h | 2 + chrome/browser/dom_distiller/tab_utils.cc | 160 ++++++++++++++++ chrome/browser/dom_distiller/tab_utils.h | 17 ++ chrome/browser/dom_distiller/tab_utils_android.cc | 27 +++ chrome/browser/dom_distiller/tab_utils_android.h | 13 ++ .../browser/dom_distiller/tab_utils_browsertest.cc | 91 ++++++++++ chrome/browser/ui/tab_helpers.cc | 7 + chrome/chrome_browser.gypi | 5 + chrome/chrome_tests.gypi | 1 + chrome/test/data/dom_distiller/simple_article.html | 12 ++ components/dom_distiller.gypi | 2 + .../content/distiller_page_web_contents.cc | 71 +++++++- .../content/distiller_page_web_contents.h | 25 ++- .../distiller_page_web_contents_browsertest.cc | 201 ++++++++++++++++++++- .../content/web_contents_main_frame_observer.cc | 61 +++++++ .../content/web_contents_main_frame_observer.h | 63 +++++++ components/dom_distiller/core/distiller_page.h | 11 +- .../dom_distiller/core/dom_distiller_service.cc | 10 +- .../dom_distiller/core/dom_distiller_service.h | 4 + .../dom_distiller/core/fake_distiller_page.h | 4 + components/dom_distiller/core/viewer_unittest.cc | 4 + 24 files changed, 810 insertions(+), 21 deletions(-) create mode 100644 chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerTabUtils.java create mode 100644 chrome/browser/dom_distiller/tab_utils.cc create mode 100644 chrome/browser/dom_distiller/tab_utils.h create mode 100644 chrome/browser/dom_distiller/tab_utils_android.cc create mode 100644 chrome/browser/dom_distiller/tab_utils_android.h create mode 100644 chrome/browser/dom_distiller/tab_utils_browsertest.cc create mode 100644 chrome/test/data/dom_distiller/simple_article.html create mode 100644 components/dom_distiller/content/web_contents_main_frame_observer.cc create mode 100644 components/dom_distiller/content/web_contents_main_frame_observer.h diff --git a/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerTabUtils.java b/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerTabUtils.java new file mode 100644 index 000000000000..ed768127dff7 --- /dev/null +++ b/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerTabUtils.java @@ -0,0 +1,31 @@ +// 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. + +package org.chromium.chrome.browser.dom_distiller; + +import org.chromium.base.JNINamespace; +import org.chromium.content_public.browser.WebContents; + +/** + * A helper class for using the DOM Distiller. + */ +@JNINamespace("android") +public class DomDistillerTabUtils { + + private DomDistillerTabUtils() { + } + + /** + * Creates a new WebContents and navigates the {@link WebContents} to view the URL of the + * current page, while in the background starts distilling the current page. This method takes + * ownership over the old WebContents after swapping in the new one. + * + * @param webContents the WebContents to distill. + */ + public static void distillCurrentPageAndView(WebContents webContents) { + nativeDistillCurrentPageAndView(webContents); + } + + private static native void nativeDistillCurrentPageAndView(WebContents webContents); +} diff --git a/chrome/browser/android/chrome_jni_registrar.cc b/chrome/browser/android/chrome_jni_registrar.cc index 7515bff8bca2..36d3734dc1bf 100644 --- a/chrome/browser/android/chrome_jni_registrar.cc +++ b/chrome/browser/android/chrome_jni_registrar.cc @@ -38,6 +38,7 @@ #include "chrome/browser/android/url_utilities.h" #include "chrome/browser/android/voice_search_tab_helper.h" #include "chrome/browser/autofill/android/personal_data_manager_android.h" +#include "chrome/browser/dom_distiller/tab_utils_android.h" #include "chrome/browser/history/android/sqlite_cursor.h" #include "chrome/browser/invalidation/invalidation_controller_android.h" #include "chrome/browser/lifetime/application_lifetime_android.h" @@ -124,6 +125,7 @@ static base::android::RegistrationMethod kChromeRegisteredMethods[] = { { "ContextMenuHelper", RegisterContextMenuHelper }, { "DataReductionProxySettings", DataReductionProxySettingsAndroid::Register }, { "DevToolsServer", RegisterDevToolsServer }, + { "DomDistillerTabUtils", RegisterDomDistillerTabUtils }, { "ExternalPrerenderRequestHandler", prerender::ExternalPrerenderHandlerAndroid:: RegisterExternalPrerenderHandlerAndroid }, diff --git a/chrome/browser/dom_distiller/lazy_dom_distiller_service.cc b/chrome/browser/dom_distiller/lazy_dom_distiller_service.cc index bebd9f69392f..3e7e74573a3f 100644 --- a/chrome/browser/dom_distiller/lazy_dom_distiller_service.cc +++ b/chrome/browser/dom_distiller/lazy_dom_distiller_service.cc @@ -7,6 +7,7 @@ #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/dom_distiller/dom_distiller_service_factory.h" #include "chrome/browser/profiles/profile.h" +#include "components/dom_distiller/core/distiller_page.h" #include "components/dom_distiller/core/dom_distiller_service.h" #include "content/public/browser/notification_source.h" @@ -78,6 +79,12 @@ LazyDomDistillerService::CreateDefaultDistillerPage() { return instance()->CreateDefaultDistillerPage(); } +scoped_ptr +LazyDomDistillerService::CreateDefaultDistillerPageWithHandle( + scoped_ptr handle) { + return instance()->CreateDefaultDistillerPageWithHandle(handle.Pass()); +} + void LazyDomDistillerService::AddObserver(DomDistillerObserver* observer) { instance()->AddObserver(observer); } diff --git a/chrome/browser/dom_distiller/lazy_dom_distiller_service.h b/chrome/browser/dom_distiller/lazy_dom_distiller_service.h index 6fcab7c8d869..d3ed5f4499f6 100644 --- a/chrome/browser/dom_distiller/lazy_dom_distiller_service.h +++ b/chrome/browser/dom_distiller/lazy_dom_distiller_service.h @@ -50,6 +50,8 @@ class LazyDomDistillerService : public DomDistillerServiceInterface, scoped_ptr distiller_page, const GURL& url) OVERRIDE; virtual scoped_ptr CreateDefaultDistillerPage() OVERRIDE; + virtual scoped_ptr CreateDefaultDistillerPageWithHandle( + scoped_ptr handle) OVERRIDE; virtual void AddObserver(DomDistillerObserver* observer) OVERRIDE; virtual void RemoveObserver(DomDistillerObserver* observer) OVERRIDE; diff --git a/chrome/browser/dom_distiller/tab_utils.cc b/chrome/browser/dom_distiller/tab_utils.cc new file mode 100644 index 000000000000..1fbcd3f4775a --- /dev/null +++ b/chrome/browser/dom_distiller/tab_utils.cc @@ -0,0 +1,160 @@ +// 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 "chrome/browser/dom_distiller/tab_utils.h" + +#include "base/message_loop/message_loop.h" +#include "chrome/browser/dom_distiller/dom_distiller_service_factory.h" +#include "chrome/browser/ui/tab_contents/core_tab_helper.h" +#include "chrome/browser/ui/tab_contents/core_tab_helper_delegate.h" +#include "chrome/common/url_constants.h" +#include "components/dom_distiller/content/distiller_page_web_contents.h" +#include "components/dom_distiller/core/distiller_page.h" +#include "components/dom_distiller/core/dom_distiller_service.h" +#include "components/dom_distiller/core/task_tracker.h" +#include "components/dom_distiller/core/url_utils.h" +#include "content/public/browser/browser_context.h" +#include "content/public/browser/navigation_controller.h" +#include "content/public/browser/web_contents.h" +#include "content/public/browser/web_contents_observer.h" + +namespace { + +using dom_distiller::ViewRequestDelegate; +using dom_distiller::DistilledArticleProto; +using dom_distiller::ArticleDistillationUpdate; +using dom_distiller::ViewerHandle; +using dom_distiller::SourcePageHandleWebContents; +using dom_distiller::DomDistillerService; +using dom_distiller::DomDistillerServiceFactory; +using dom_distiller::DistillerPage; +using dom_distiller::SourcePageHandle; + +// An no-op ViewRequestDelegate which holds a ViewerHandle and deletes itself +// after the WebContents navigates or goes away. This class is a band-aid to +// keep a TaskTracker around until the distillation starts from the viewer. +class SelfDeletingRequestDelegate : public ViewRequestDelegate, + public content::WebContentsObserver { + public: + explicit SelfDeletingRequestDelegate(content::WebContents* web_contents); + virtual ~SelfDeletingRequestDelegate(); + + // ViewRequestDelegate implementation. + virtual void OnArticleReady( + const DistilledArticleProto* article_proto) OVERRIDE; + virtual void OnArticleUpdated( + ArticleDistillationUpdate article_update) OVERRIDE; + + // content::WebContentsObserver implementation. + virtual void DidNavigateMainFrame( + const content::LoadCommittedDetails& details, + const content::FrameNavigateParams& params) OVERRIDE; + virtual void RenderProcessGone(base::TerminationStatus status) OVERRIDE; + virtual void WebContentsDestroyed() OVERRIDE; + + // Takes ownership of the ViewerHandle to keep distillation alive until |this| + // is deleted. + void TakeViewerHandle(scoped_ptr viewer_handle); + + private: + // The handle to the view request towards the DomDistillerService. It + // needs to be kept around to ensure the distillation request finishes. + scoped_ptr viewer_handle_; + + // The WebContents this class is tracking. + content::WebContents* web_contents_; +}; + +void SelfDeletingRequestDelegate::DidNavigateMainFrame( + const content::LoadCommittedDetails& details, + const content::FrameNavigateParams& params) { + base::MessageLoop::current()->DeleteSoon(FROM_HERE, this); +} + +void SelfDeletingRequestDelegate::RenderProcessGone( + base::TerminationStatus status) { + base::MessageLoop::current()->DeleteSoon(FROM_HERE, this); +} + +void SelfDeletingRequestDelegate::WebContentsDestroyed() { + base::MessageLoop::current()->DeleteSoon(FROM_HERE, this); +} + +SelfDeletingRequestDelegate::SelfDeletingRequestDelegate( + content::WebContents* web_contents) + : web_contents_(web_contents) { + content::WebContentsObserver::Observe(web_contents_); +} + +SelfDeletingRequestDelegate::~SelfDeletingRequestDelegate() { + content::WebContentsObserver::Observe(NULL); +} + +void SelfDeletingRequestDelegate::OnArticleReady( + const DistilledArticleProto* article_proto) { +} + +void SelfDeletingRequestDelegate::OnArticleUpdated( + ArticleDistillationUpdate article_update) { +} + +void SelfDeletingRequestDelegate::TakeViewerHandle( + scoped_ptr viewer_handle) { + viewer_handle_ = viewer_handle.Pass(); +} + +// Start loading the viewer URL of the current page in |web_contents|. +void StartNavigationToDistillerViewer(content::WebContents* web_contents, + const GURL& url) { + GURL viewer_url = dom_distiller::url_utils::GetDistillerViewUrlFromUrl( + chrome::kDomDistillerScheme, url); + content::NavigationController::LoadURLParams params(viewer_url); + params.transition_type = content::PAGE_TRANSITION_AUTO_BOOKMARK; + web_contents->GetController().LoadURLWithParams(params); +} + +void StartDistillation(content::WebContents* web_contents) { + // Start distillation using |web_contents|, and ensure ViewerHandle stays + // around until the viewer requests distillation. + SelfDeletingRequestDelegate* view_request_delegate = + new SelfDeletingRequestDelegate(web_contents); + scoped_ptr old_web_contents_sptr(web_contents); + scoped_ptr source_page_handle( + new SourcePageHandleWebContents(old_web_contents_sptr.Pass())); + DomDistillerService* dom_distiller_service = + DomDistillerServiceFactory::GetForBrowserContext( + web_contents->GetBrowserContext()); + scoped_ptr distiller_page = + dom_distiller_service->CreateDefaultDistillerPageWithHandle( + source_page_handle.PassAs()) + .Pass(); + + const GURL& last_committed_url = web_contents->GetLastCommittedURL(); + scoped_ptr viewer_handle = dom_distiller_service->ViewUrl( + view_request_delegate, distiller_page.Pass(), last_committed_url); + view_request_delegate->TakeViewerHandle(viewer_handle.Pass()); +} + +} // namespace + +void DistillCurrentPageAndView(content::WebContents* old_web_contents) { + DCHECK(old_web_contents); + // Create new WebContents. + content::WebContents::CreateParams create_params( + old_web_contents->GetBrowserContext()); + content::WebContents* new_web_contents = + content::WebContents::Create(create_params); + DCHECK(new_web_contents); + + // Copy all navigation state from the old WebContents to the new one. + new_web_contents->GetController().CopyStateFrom( + old_web_contents->GetController()); + + CoreTabHelper::FromWebContents(old_web_contents)->delegate()->SwapTabContents( + old_web_contents, new_web_contents, false, false); + + StartNavigationToDistillerViewer(new_web_contents, + old_web_contents->GetLastCommittedURL()); + StartDistillation(old_web_contents); +} diff --git a/chrome/browser/dom_distiller/tab_utils.h b/chrome/browser/dom_distiller/tab_utils.h new file mode 100644 index 000000000000..d10a3c8d4b39 --- /dev/null +++ b/chrome/browser/dom_distiller/tab_utils.h @@ -0,0 +1,17 @@ +// 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 CHROME_BROWSER_DOM_DISTILLER_TAB_UTILS_H_ +#define CHROME_BROWSER_DOM_DISTILLER_TAB_UTILS_H_ + +namespace content { +class WebContents; +} // namespace content + +// Creates a new WebContents and navigates it to view the URL of the current +// page, while in the background starts distilling the current page. This method +// takes ownership over the old WebContents after swapping in the new one. +void DistillCurrentPageAndView(content::WebContents* old_web_contents); + +#endif // CHROME_BROWSER_DOM_DISTILLER_TAB_UTILS_H_ diff --git a/chrome/browser/dom_distiller/tab_utils_android.cc b/chrome/browser/dom_distiller/tab_utils_android.cc new file mode 100644 index 000000000000..805f7e3657fe --- /dev/null +++ b/chrome/browser/dom_distiller/tab_utils_android.cc @@ -0,0 +1,27 @@ +// 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 "chrome/browser/dom_distiller/tab_utils_android.h" + +#include + +#include "chrome/browser/dom_distiller/tab_utils.h" +#include "content/public/browser/web_contents.h" +#include "jni/DomDistillerTabUtils_jni.h" + +namespace android { + +void DistillCurrentPageAndView(JNIEnv* env, + jclass clazz, + jobject j_web_contents) { + content::WebContents* web_contents = + content::WebContents::FromJavaWebContents(j_web_contents); + ::DistillCurrentPageAndView(web_contents); +} + +} // namespace android + +bool RegisterDomDistillerTabUtils(JNIEnv* env) { + return android::RegisterNativesImpl(env); +} diff --git a/chrome/browser/dom_distiller/tab_utils_android.h b/chrome/browser/dom_distiller/tab_utils_android.h new file mode 100644 index 000000000000..cdd98eb481c0 --- /dev/null +++ b/chrome/browser/dom_distiller/tab_utils_android.h @@ -0,0 +1,13 @@ +// 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 CHROME_BROWSER_DOM_DISTILLER_TAB_UTILS_ANDROID_H_ +#define CHROME_BROWSER_DOM_DISTILLER_TAB_UTILS_ANDROID_H_ + +#include + +// Register JNI methods. +bool RegisterDomDistillerTabUtils(JNIEnv* env); + +#endif // CHROME_BROWSER_DOM_DISTILLER_TAB_UTILS_ANDROID_H_ diff --git a/chrome/browser/dom_distiller/tab_utils_browsertest.cc b/chrome/browser/dom_distiller/tab_utils_browsertest.cc new file mode 100644 index 000000000000..37bd4f293b60 --- /dev/null +++ b/chrome/browser/dom_distiller/tab_utils_browsertest.cc @@ -0,0 +1,91 @@ +// 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 + +#include "base/command_line.h" +#include "base/strings/utf_string_conversions.h" +#include "chrome/browser/dom_distiller/dom_distiller_service_factory.h" +#include "chrome/browser/dom_distiller/tab_utils.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/tabs/tab_strip_model.h" +#include "chrome/common/chrome_switches.h" +#include "chrome/common/url_constants.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "chrome/test/base/ui_test_utils.h" +#include "components/dom_distiller/content/web_contents_main_frame_observer.h" +#include "components/dom_distiller/core/dom_distiller_service.h" +#include "components/dom_distiller/core/task_tracker.h" +#include "components/dom_distiller/core/url_utils.h" +#include "content/public/browser/render_frame_host.h" +#include "content/public/browser/web_contents.h" +#include "content/public/browser/web_contents_observer.h" +#include "net/test/embedded_test_server/embedded_test_server.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace dom_distiller { + +namespace { +const char* kSimpleArticlePath = "/dom_distiller/simple_article.html"; +} // namespace + +class DomDistillerTabUtilsBrowserTest : public InProcessBrowserTest { + public: + virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE { + command_line->AppendSwitch(switches::kEnableDomDistiller); + } +}; + +class WebContentsMainFrameHelper : public content::WebContentsObserver { + public: + WebContentsMainFrameHelper(content::WebContents* web_contents, + const base::Closure& callback) + : callback_(callback) { + content::WebContentsObserver::Observe(web_contents); + } + + virtual void DidFinishLoad( + int64 frame_id, + const GURL& validated_url, + bool is_main_frame, + content::RenderViewHost* render_view_host) OVERRIDE { + if (is_main_frame && validated_url.scheme() == chrome::kDomDistillerScheme) + callback_.Run(); + } + + private: + base::Closure callback_; +}; + +IN_PROC_BROWSER_TEST_F(DomDistillerTabUtilsBrowserTest, TestSwapWebContents) { + ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady()); + + content::WebContents* initial_web_contents = + browser()->tab_strip_model()->GetActiveWebContents(); + const GURL& article_url = embedded_test_server()->GetURL(kSimpleArticlePath); + + // This blocks until the navigation has completely finished. + ui_test_utils::NavigateToURL(browser(), article_url); + + DistillCurrentPageAndView(initial_web_contents); + + // Wait until the new WebContents has fully navigated. + content::WebContents* after_web_contents = + browser()->tab_strip_model()->GetActiveWebContents(); + ASSERT_TRUE(after_web_contents != NULL); + base::RunLoop new_url_loaded_runner; + scoped_ptr distilled_page_loaded( + new WebContentsMainFrameHelper(after_web_contents, + new_url_loaded_runner.QuitClosure())); + new_url_loaded_runner.Run(); + + // Verify the new URL is showing distilled content in a new WebContents. + EXPECT_NE(initial_web_contents, after_web_contents); + EXPECT_TRUE(after_web_contents->GetLastCommittedURL().SchemeIs( + chrome::kDomDistillerScheme)); + EXPECT_EQ("Test Page Title", + base::UTF16ToUTF8(after_web_contents->GetTitle())); +} + +} // namespace dom_distiller diff --git a/chrome/browser/ui/tab_helpers.cc b/chrome/browser/ui/tab_helpers.cc index c4539c83f223..abdad8551ca9 100644 --- a/chrome/browser/ui/tab_helpers.cc +++ b/chrome/browser/ui/tab_helpers.cc @@ -29,6 +29,7 @@ #include "chrome/common/chrome_switches.h" #include "components/autofill/content/browser/content_autofill_driver.h" #include "components/autofill/core/browser/autofill_manager.h" +#include "components/dom_distiller/content/web_contents_main_frame_observer.h" #include "components/password_manager/core/browser/password_manager.h" #include "content/public/browser/web_contents.h" #include "extensions/browser/view_type_utils.h" @@ -189,6 +190,12 @@ void TabHelpers::AttachTabHelpers(WebContents* web_contents) { #endif // defined(ENABLE_FULL_PRINTING) #endif // defined(ENABLE_PRINTING) && !defined(OS_ANDROID) + if (CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableDomDistiller)) { + dom_distiller::WebContentsMainFrameObserver::CreateForWebContents( + web_contents); + } + #if defined(ENABLE_ONE_CLICK_SIGNIN) // If this is not an incognito window, setup to handle one-click login. // We don't want to check that the profile is already connected at this time diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 2c5c2884e96e..29822eaa20b5 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -540,6 +540,10 @@ 'browser/dom_distiller/dom_distiller_service_factory.h', 'browser/dom_distiller/lazy_dom_distiller_service.cc', 'browser/dom_distiller/lazy_dom_distiller_service.h', + 'browser/dom_distiller/tab_utils.cc', + 'browser/dom_distiller/tab_utils.h', + 'browser/dom_distiller/tab_utils_android.cc', + 'browser/dom_distiller/tab_utils_android.h', 'browser/download/all_download_item_notifier.cc', 'browser/download/all_download_item_notifier.h', 'browser/download/chrome_download_manager_delegate.cc', @@ -3625,6 +3629,7 @@ 'android/java/src/org/chromium/chrome/browser/DevToolsServer.java', 'android/java/src/org/chromium/chrome/browser/database/SQLiteCursor.java', 'android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerFeedbackReporter.java', + 'android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerTabUtils.java', 'android/java/src/org/chromium/chrome/browser/favicon/FaviconHelper.java', 'android/java/src/org/chromium/chrome/browser/FieldTrialHelper.java', 'android/java/src/org/chromium/chrome/browser/ForeignSessionHelper.java', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 77273a84a93a..6c0f235b5869 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1024,6 +1024,7 @@ 'browser/devtools/device/port_forwarding_browsertest.cc', 'browser/devtools/devtools_sanity_browsertest.cc', 'browser/dom_distiller/dom_distiller_viewer_source_browsertest.cc', + 'browser/dom_distiller/tab_utils_browsertest.cc', 'browser/do_not_track_browsertest.cc', 'browser/download/download_browsertest.cc', 'browser/download/download_browsertest.h', diff --git a/chrome/test/data/dom_distiller/simple_article.html b/chrome/test/data/dom_distiller/simple_article.html new file mode 100644 index 000000000000..4c0279f8fd49 --- /dev/null +++ b/chrome/test/data/dom_distiller/simple_article.html @@ -0,0 +1,12 @@ + +Test Page Title + +
+

Lorem ipsum dolor sit amet, at alia aliquip vel. Quas inani labore an vel. Sed an nemore minimum accusata. Sint inermis tacimates est ex, ad movet iracundia mei, delicata iracundia laboramus ei eos. Illud principes complectitur te nec, ius alienum insolens ea, cu quo oratio omnesque. + +

Lorem ipsum dolor sit amet, at alia aliquip vel. Quas inani labore an vel. Sed an nemore minimum accusata. Sint inermis tacimates est ex, ad movet iracundia mei, delicata iracundia laboramus ei eos. Illud principes complectitur te nec, ius alienum insolens ea, cu quo oratio omnesque. + +

Lorem ipsum dolor sit amet, at alia aliquip vel. Quas inani labore an vel. Sed an nemore minimum accusata. Sint inermis tacimates est ex, ad movet iracundia mei, delicata iracundia laboramus ei eos. Illud principes complectitur te nec, ius alienum insolens ea, cu quo oratio omnesque. +

+ + diff --git a/components/dom_distiller.gypi b/components/dom_distiller.gypi index 092a26abebfc..cc7bae5abdc5 100644 --- a/components/dom_distiller.gypi +++ b/components/dom_distiller.gypi @@ -156,6 +156,8 @@ 'dom_distiller/content/distiller_page_web_contents.h', 'dom_distiller/content/dom_distiller_viewer_source.cc', 'dom_distiller/content/dom_distiller_viewer_source.h', + 'dom_distiller/content/web_contents_main_frame_observer.cc', + 'dom_distiller/content/web_contents_main_frame_observer.h', ], }, ], diff --git a/components/dom_distiller/content/distiller_page_web_contents.cc b/components/dom_distiller/content/distiller_page_web_contents.cc index baa9977a9f16..527c0eb96de7 100644 --- a/components/dom_distiller/content/distiller_page_web_contents.cc +++ b/components/dom_distiller/content/distiller_page_web_contents.cc @@ -7,7 +7,9 @@ #include "base/callback.h" #include "base/memory/scoped_ptr.h" #include "base/strings/utf_string_conversions.h" +#include "components/dom_distiller/content/web_contents_main_frame_observer.h" #include "components/dom_distiller/core/distiller_page.h" +#include "components/dom_distiller/core/dom_distiller_service.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/navigation_controller.h" #include "content/public/browser/render_frame_host.h" @@ -18,18 +20,48 @@ namespace dom_distiller { +SourcePageHandleWebContents::SourcePageHandleWebContents( + scoped_ptr web_contents) + : web_contents_(web_contents.Pass()) { + DCHECK(web_contents_); +} + +SourcePageHandleWebContents::~SourcePageHandleWebContents() { +} + +scoped_ptr SourcePageHandleWebContents::GetWebContents() { + return web_contents_.Pass(); +} + scoped_ptr DistillerPageWebContentsFactory::CreateDistillerPage() const { DCHECK(browser_context_); - return scoped_ptr( - new DistillerPageWebContents(browser_context_)); + return scoped_ptr(new DistillerPageWebContents( + browser_context_, scoped_ptr())); +} + +scoped_ptr +DistillerPageWebContentsFactory::CreateDistillerPageWithHandle( + scoped_ptr handle) const { + DCHECK(browser_context_); + scoped_ptr web_contents_handle = + scoped_ptr( + static_cast(handle.release())); + return scoped_ptr(new DistillerPageWebContents( + browser_context_, web_contents_handle.Pass())); } DistillerPageWebContents::DistillerPageWebContents( - content::BrowserContext* browser_context) - : state_(IDLE), browser_context_(browser_context) {} + content::BrowserContext* browser_context, + scoped_ptr optional_web_contents_handle) + : state_(IDLE), browser_context_(browser_context) { + if (optional_web_contents_handle) { + web_contents_ = optional_web_contents_handle->GetWebContents().Pass(); + } +} -DistillerPageWebContents::~DistillerPageWebContents() {} +DistillerPageWebContents::~DistillerPageWebContents() { +} void DistillerPageWebContents::DistillPageImpl(const GURL& url, const std::string& script) { @@ -38,6 +70,31 @@ void DistillerPageWebContents::DistillPageImpl(const GURL& url, state_ = LOADING_PAGE; script_ = script; + if (web_contents_ && web_contents_->GetLastCommittedURL() == url) { + WebContentsMainFrameObserver* main_frame_observer = + WebContentsMainFrameObserver::FromWebContents(web_contents_.get()); + if (main_frame_observer && main_frame_observer->is_initialized()) { + if (main_frame_observer->is_document_loaded_in_main_frame()) { + // Main frame has already loaded for the current WebContents, so execute + // JavaScript immediately. + ExecuteJavaScript(); + } else { + // Main frame document has not loaded yet, so wait until it has before + // executing JavaScript. It will trigger after DocumentLoadedInFrame is + // called for the main frame. + content::WebContentsObserver::Observe(web_contents_.get()); + } + } else { + // The WebContentsMainFrameObserver has not been correctly initialized, + // so fall back to creating a new WebContents. + CreateNewWebContents(url); + } + } else { + CreateNewWebContents(url); + } +} + +void DistillerPageWebContents::CreateNewWebContents(const GURL& url) { // Create new WebContents to use for distilling the content. content::WebContents::CreateParams create_params(browser_context_); create_params.initially_hidden = true; @@ -54,8 +111,6 @@ void DistillerPageWebContents::DocumentLoadedInFrame( int64 frame_id, RenderViewHost* render_view_host) { if (frame_id == web_contents_->GetMainFrame()->GetRoutingID()) { - content::WebContentsObserver::Observe(NULL); - web_contents_->Stop(); ExecuteJavaScript(); } } @@ -81,6 +136,8 @@ void DistillerPageWebContents::ExecuteJavaScript() { DCHECK(frame); DCHECK_EQ(LOADING_PAGE, state_); state_ = EXECUTING_JAVASCRIPT; + content::WebContentsObserver::Observe(NULL); + web_contents_->Stop(); DVLOG(1) << "Beginning distillation"; frame->ExecuteJavaScript( base::UTF8ToUTF16(script_), diff --git a/components/dom_distiller/content/distiller_page_web_contents.h b/components/dom_distiller/content/distiller_page_web_contents.h index 52509f502804..aed9789cf552 100644 --- a/components/dom_distiller/content/distiller_page_web_contents.h +++ b/components/dom_distiller/content/distiller_page_web_contents.h @@ -21,7 +21,18 @@ using content::RenderViewHost; namespace dom_distiller { -class DistillerContext; +class SourcePageHandleWebContents : public SourcePageHandle { + public: + explicit SourcePageHandleWebContents( + scoped_ptr web_contents); + virtual ~SourcePageHandleWebContents(); + + scoped_ptr GetWebContents(); + + private: + // The WebContents this class owns. + scoped_ptr web_contents_; +}; class DistillerPageWebContentsFactory : public DistillerPageFactory { public: @@ -31,6 +42,8 @@ class DistillerPageWebContentsFactory : public DistillerPageFactory { virtual ~DistillerPageWebContentsFactory() {} virtual scoped_ptr CreateDistillerPage() const OVERRIDE; + virtual scoped_ptr CreateDistillerPageWithHandle( + scoped_ptr handle) const OVERRIDE; private: content::BrowserContext* browser_context_; @@ -39,7 +52,9 @@ class DistillerPageWebContentsFactory : public DistillerPageFactory { class DistillerPageWebContents : public DistillerPage, public content::WebContentsObserver { public: - DistillerPageWebContents(content::BrowserContext* browser_context); + DistillerPageWebContents( + content::BrowserContext* browser_context, + scoped_ptr optional_web_contents_handle); virtual ~DistillerPageWebContents(); // content::WebContentsObserver implementation. @@ -58,6 +73,8 @@ class DistillerPageWebContents : public DistillerPage, const std::string& script) OVERRIDE; private: + friend class TestDistillerPageWebContents; + enum State { // The page distiller is idle. IDLE, @@ -70,6 +87,10 @@ class DistillerPageWebContents : public DistillerPage, EXECUTING_JAVASCRIPT }; + // Creates a new WebContents, adds |this| as an observer, and loads the + // |url|. + virtual void CreateNewWebContents(const GURL& url); + // Injects and executes JavaScript in the context of a loaded page. This // must only be called after the page has successfully loaded. void ExecuteJavaScript(); diff --git a/components/dom_distiller/content/distiller_page_web_contents_browsertest.cc b/components/dom_distiller/content/distiller_page_web_contents_browsertest.cc index 5d1cd2ae1806..e319097bd419 100644 --- a/components/dom_distiller/content/distiller_page_web_contents_browsertest.cc +++ b/components/dom_distiller/content/distiller_page_web_contents_browsertest.cc @@ -7,8 +7,12 @@ #include "base/run_loop.h" #include "base/values.h" #include "components/dom_distiller/content/distiller_page_web_contents.h" +#include "components/dom_distiller/content/web_contents_main_frame_observer.h" #include "components/dom_distiller/core/distiller_page.h" #include "content/public/browser/browser_context.h" +#include "content/public/browser/navigation_controller.h" +#include "content/public/browser/render_frame_host.h" +#include "content/public/browser/web_contents_observer.h" #include "content/public/test/content_browser_test.h" #include "content/shell/browser/shell.h" #include "grit/component_resources.h" @@ -23,6 +27,8 @@ using testing::Not; namespace dom_distiller { +const char* kSimpleArticlePath = "/simple_article.html"; + class DistillerPageWebContentsTest : public ContentBrowserTest { public: // ContentBrowserTest: @@ -66,18 +72,108 @@ class DistillerPageWebContentsTest : public ContentBrowserTest { } protected: + void RunUseCurrentWebContentsTest(const std::string& url, + bool expect_new_web_contents, + bool setup_main_frame_observer, + bool wait_for_document_loaded); + DistillerPageWebContents* distiller_page_; base::Closure quit_closure_; scoped_ptr page_info_; }; +// Use this class to be able to leak the WebContents, which is needed for when +// the current WebContents is used for distillation. +class TestDistillerPageWebContents : public DistillerPageWebContents { + public: + TestDistillerPageWebContents( + content::BrowserContext* browser_context, + scoped_ptr optional_web_contents_handle, + bool expect_new_web_contents) + : DistillerPageWebContents(browser_context, + optional_web_contents_handle.Pass()), + expect_new_web_contents_(expect_new_web_contents), + new_web_contents_created_(false) {} + + virtual void CreateNewWebContents(const GURL& url) OVERRIDE { + ASSERT_EQ(true, expect_new_web_contents_); + new_web_contents_created_ = true; + // DistillerPageWebContents::CreateNewWebContents resets the scoped_ptr to + // the WebContents, so intentionally leak WebContents here, since it is + // owned by the shell. + content::WebContents* web_contents = web_contents_.release(); + web_contents->GetLastCommittedURL(); + DistillerPageWebContents::CreateNewWebContents(url); + } + + virtual ~TestDistillerPageWebContents() { + if (!expect_new_web_contents_) { + // Intentionally leaking WebContents, since it is owned by the shell. + content::WebContents* web_contents = web_contents_.release(); + web_contents->GetLastCommittedURL(); + } + } + + bool new_web_contents_created() { return new_web_contents_created_; } + + private: + bool expect_new_web_contents_; + bool new_web_contents_created_; +}; + +// Helper class to know how far in the loading process the current WebContents +// has come. It will call the callback either after +// DidCommitProvisionalLoadForFrame or DocumentLoadedInFrame is called for the +// main frame, based on the value of |wait_for_document_loaded|. +class WebContentsMainFrameHelper : public content::WebContentsObserver { + public: + WebContentsMainFrameHelper(content::WebContents* web_contents, + const base::Closure& callback, + bool wait_for_document_loaded) + : web_contents_(web_contents), + callback_(callback), + wait_for_document_loaded_(wait_for_document_loaded) { + content::WebContentsObserver::Observe(web_contents); + } + + virtual void DidCommitProvisionalLoadForFrame( + int64 frame_id, + const base::string16& frame_unique_name, + bool is_main_frame, + const GURL& url, + content::PageTransition transition_type, + content::RenderViewHost* render_view_host) OVERRIDE { + if (wait_for_document_loaded_) + return; + if (is_main_frame) + callback_.Run(); + } + + virtual void DocumentLoadedInFrame( + int64 frame_id, + content::RenderViewHost* render_view_host) OVERRIDE { + if (wait_for_document_loaded_) { + if (web_contents_ && + frame_id == web_contents_->GetMainFrame()->GetRoutingID()) { + callback_.Run(); + } + } + } + + private: + content::WebContents* web_contents_; + base::Closure callback_; + bool wait_for_document_loaded_; +}; + IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, BasicDistillationWorks) { DistillerPageWebContents distiller_page( - shell()->web_contents()->GetBrowserContext()); + shell()->web_contents()->GetBrowserContext(), + scoped_ptr()); distiller_page_ = &distiller_page; base::RunLoop run_loop; - DistillPage(run_loop.QuitClosure(), "/simple_article.html"); + DistillPage(run_loop.QuitClosure(), kSimpleArticlePath); run_loop.Run(); EXPECT_EQ("Test Page Title", page_info_.get()->title); @@ -89,11 +185,12 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, BasicDistillationWorks) { IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeLinks) { DistillerPageWebContents distiller_page( - shell()->web_contents()->GetBrowserContext()); + shell()->web_contents()->GetBrowserContext(), + scoped_ptr()); distiller_page_ = &distiller_page; base::RunLoop run_loop; - DistillPage(run_loop.QuitClosure(), "/simple_article.html"); + DistillPage(run_loop.QuitClosure(), kSimpleArticlePath); run_loop.Run(); // A relative link should've been updated. @@ -105,11 +202,12 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeLinks) { IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeImages) { DistillerPageWebContents distiller_page( - shell()->web_contents()->GetBrowserContext()); + shell()->web_contents()->GetBrowserContext(), + scoped_ptr()); distiller_page_ = &distiller_page; base::RunLoop run_loop; - DistillPage(run_loop.QuitClosure(), "/simple_article.html"); + DistillPage(run_loop.QuitClosure(), kSimpleArticlePath); run_loop.Run(); // A relative link should've been updated. @@ -121,7 +219,8 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeImages) { IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, VisibilityDetection) { DistillerPageWebContents distiller_page( - shell()->web_contents()->GetBrowserContext()); + shell()->web_contents()->GetBrowserContext(), + scoped_ptr()); distiller_page_ = &distiller_page; // visble_style.html and invisible_style.html only differ by the visibility @@ -142,4 +241,92 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, VisibilityDetection) { } } +IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, + UsingCurrentWebContentsWrongUrl) { + std::string url("/bogus"); + bool expect_new_web_contents = true; + bool setup_main_frame_observer = true; + bool wait_for_document_loaded = true; + RunUseCurrentWebContentsTest(url, + expect_new_web_contents, + setup_main_frame_observer, + wait_for_document_loaded); +} + +IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, + UsingCurrentWebContentsNoMainFrameObserver) { + std::string url(kSimpleArticlePath); + bool expect_new_web_contents = true; + bool setup_main_frame_observer = false; + bool wait_for_document_loaded = true; + RunUseCurrentWebContentsTest(url, + expect_new_web_contents, + setup_main_frame_observer, + wait_for_document_loaded); +} + +IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, + UsingCurrentWebContentsNotFinishedLoadingYet) { + std::string url(kSimpleArticlePath); + bool expect_new_web_contents = false; + bool setup_main_frame_observer = true; + bool wait_for_document_loaded = false; + RunUseCurrentWebContentsTest(url, + expect_new_web_contents, + setup_main_frame_observer, + wait_for_document_loaded); +} + +IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, + UsingCurrentWebContentsReadyForDistillation) { + std::string url(kSimpleArticlePath); + bool expect_new_web_contents = false; + bool setup_main_frame_observer = true; + bool wait_for_document_loaded = true; + RunUseCurrentWebContentsTest(url, + expect_new_web_contents, + setup_main_frame_observer, + wait_for_document_loaded); +} + +void DistillerPageWebContentsTest::RunUseCurrentWebContentsTest( + const std::string& url, + bool expect_new_web_contents, + bool setup_main_frame_observer, + bool wait_for_document_loaded) { + content::WebContents* current_web_contents = shell()->web_contents(); + if (setup_main_frame_observer) { + dom_distiller::WebContentsMainFrameObserver::CreateForWebContents( + current_web_contents); + } + base::RunLoop url_loaded_runner; + WebContentsMainFrameHelper main_frame_loaded(current_web_contents, + url_loaded_runner.QuitClosure(), + wait_for_document_loaded); + current_web_contents->GetController().LoadURL( + embedded_test_server()->GetURL(url), + content::Referrer(), + content::PAGE_TRANSITION_TYPED, + std::string()); + url_loaded_runner.Run(); + + scoped_ptr old_web_contents_sptr(current_web_contents); + scoped_ptr source_page_handle( + new SourcePageHandleWebContents(old_web_contents_sptr.Pass())); + + TestDistillerPageWebContents distiller_page( + shell()->web_contents()->GetBrowserContext(), + source_page_handle.Pass(), + expect_new_web_contents); + distiller_page_ = &distiller_page; + + base::RunLoop run_loop; + DistillPage(run_loop.QuitClosure(), kSimpleArticlePath); + run_loop.Run(); + + // Sanity check of distillation process. + EXPECT_EQ(expect_new_web_contents, distiller_page.new_web_contents_created()); + EXPECT_EQ("Test Page Title", page_info_.get()->title); +} + } // namespace dom_distiller diff --git a/components/dom_distiller/content/web_contents_main_frame_observer.cc b/components/dom_distiller/content/web_contents_main_frame_observer.cc new file mode 100644 index 000000000000..ed1bbab54913 --- /dev/null +++ b/components/dom_distiller/content/web_contents_main_frame_observer.cc @@ -0,0 +1,61 @@ +// 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/content/web_contents_main_frame_observer.h" + +#include "content/public/browser/navigation_details.h" +#include "content/public/browser/render_frame_host.h" +#include "content/public/browser/web_contents.h" +#include "content/public/browser/web_contents_observer.h" +#include "content/public/browser/web_contents_user_data.h" + +DEFINE_WEB_CONTENTS_USER_DATA_KEY(dom_distiller::WebContentsMainFrameObserver); + +namespace dom_distiller { + +WebContentsMainFrameObserver::WebContentsMainFrameObserver( + content::WebContents* web_contents) + : is_document_loaded_in_main_frame_(false), + is_initialized_(false), + web_contents_(web_contents) { + content::WebContentsObserver::Observe(web_contents); +} + +WebContentsMainFrameObserver::~WebContentsMainFrameObserver() { + CleanUp(); +} + +void WebContentsMainFrameObserver::DocumentLoadedInFrame( + int64 frame_id, + content::RenderViewHost* render_view_host) { + if (web_contents_ && + frame_id == web_contents_->GetMainFrame()->GetRoutingID()) { + is_document_loaded_in_main_frame_ = true; + } +} + +void WebContentsMainFrameObserver::DidNavigateMainFrame( + const content::LoadCommittedDetails& details, + const content::FrameNavigateParams& params) { + if (details.is_main_frame) { + is_document_loaded_in_main_frame_ = false; + is_initialized_ = true; + } +} + +void WebContentsMainFrameObserver::RenderProcessGone( + base::TerminationStatus status) { + CleanUp(); +} + +void WebContentsMainFrameObserver::WebContentsDestroyed() { + CleanUp(); +} + +void WebContentsMainFrameObserver::CleanUp() { + content::WebContentsObserver::Observe(NULL); + web_contents_ = NULL; +} + +} // namespace dom_distiller diff --git a/components/dom_distiller/content/web_contents_main_frame_observer.h b/components/dom_distiller/content/web_contents_main_frame_observer.h new file mode 100644 index 000000000000..7343dba79ccd --- /dev/null +++ b/components/dom_distiller/content/web_contents_main_frame_observer.h @@ -0,0 +1,63 @@ +// 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_CONTENT_WEB_CONTENTS_MAIN_FRAME_OBSERVER_H_ +#define COMPONENTS_DOM_DISTILLER_CONTENT_WEB_CONTENTS_MAIN_FRAME_OBSERVER_H_ + +#include "content/public/browser/web_contents.h" +#include "content/public/browser/web_contents_observer.h" +#include "content/public/browser/web_contents_user_data.h" + +namespace dom_distiller { + +// Tracks whether DocumentLoadedInFrame has been called for the main frame for +// the current main frame for the given WebContents. It removes itself as an +// observer if the WebContents is destroyed or the render process is gone. +class WebContentsMainFrameObserver + : public content::WebContentsObserver, + public content::WebContentsUserData { + public: + virtual ~WebContentsMainFrameObserver(); + + bool is_document_loaded_in_main_frame() { + return is_document_loaded_in_main_frame_; + } + + bool is_initialized() { return is_initialized_; } + + // content::WebContentsObserver implementation. + virtual void DocumentLoadedInFrame( + int64 frame_id, + content::RenderViewHost* render_view_host) OVERRIDE; + virtual void DidNavigateMainFrame( + const content::LoadCommittedDetails& details, + const content::FrameNavigateParams& params) OVERRIDE; + virtual void RenderProcessGone(base::TerminationStatus status) OVERRIDE; + virtual void WebContentsDestroyed() OVERRIDE; + + private: + explicit WebContentsMainFrameObserver(content::WebContents* web_contents); + friend class content::WebContentsUserData; + + // Removes the observer and clears the WebContents member. + void CleanUp(); + + // Whether DocumentLoadedInFrame has been called for the tracked WebContents + // for the current main frame. This is cleared when the main frame navigates, + // and set again when DocumentLoadedInFrame is called for the main frame. + bool is_document_loaded_in_main_frame_; + + // Whether this object has been correctly initialized. This is set as soon as + // at least one call to DidNavigateMainFrame has happened. + bool is_initialized_; + + // The WebContents this class is tracking. + content::WebContents* web_contents_; + + DISALLOW_COPY_AND_ASSIGN(WebContentsMainFrameObserver); +}; + +} // namespace dom_distiller + +#endif // COMPONENTS_DOM_DISTILLER_CONTENT_WEB_CONTENTS_MAIN_FRAME_OBSERVER_H_ diff --git a/components/dom_distiller/core/distiller_page.h b/components/dom_distiller/core/distiller_page.h index 429a676540ba..b950df7f1baf 100644 --- a/components/dom_distiller/core/distiller_page.h +++ b/components/dom_distiller/core/distiller_page.h @@ -29,6 +29,11 @@ struct DistilledPageInfo { DISALLOW_COPY_AND_ASSIGN(DistilledPageInfo); }; +class SourcePageHandle { + public: + virtual ~SourcePageHandle() {} +}; + // Injects JavaScript into a page, and uses it to extract and return long-form // content. The class can be reused to load and distill multiple pages, // following the state transitions described along with the class's states. @@ -62,10 +67,6 @@ class DistillerPage { // should be the same regardless of the DistillerPage implementation. virtual void DistillPageImpl(const GURL& url, 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() = 0; - private: bool ready_; DistillerPageCallback distiller_page_callback_; @@ -81,6 +82,8 @@ class DistillerPageFactory { // should be very cheap, since the pages can be thrown away without being // used. virtual scoped_ptr CreateDistillerPage() const = 0; + virtual scoped_ptr CreateDistillerPageWithHandle( + scoped_ptr handle) const = 0; }; } // namespace dom_distiller diff --git a/components/dom_distiller/core/dom_distiller_service.cc b/components/dom_distiller/core/dom_distiller_service.cc index 848adaa4a2aa..a3cbbda24747 100644 --- a/components/dom_distiller/core/dom_distiller_service.cc +++ b/components/dom_distiller/core/dom_distiller_service.cc @@ -46,7 +46,8 @@ DomDistillerService::DomDistillerService( distiller_page_factory_(distiller_page_factory.Pass()) { } -DomDistillerService::~DomDistillerService() {} +DomDistillerService::~DomDistillerService() { +} syncer::SyncableService* DomDistillerService::GetSyncableService() const { return store_->GetSyncableService(); @@ -56,6 +57,13 @@ scoped_ptr DomDistillerService::CreateDefaultDistillerPage() { return distiller_page_factory_->CreateDistillerPage().Pass(); } +scoped_ptr +DomDistillerService::CreateDefaultDistillerPageWithHandle( + scoped_ptr handle) { + return distiller_page_factory_->CreateDistillerPageWithHandle(handle.Pass()) + .Pass(); +} + const std::string DomDistillerService::AddToList( const GURL& url, scoped_ptr distiller_page, diff --git a/components/dom_distiller/core/dom_distiller_service.h b/components/dom_distiller/core/dom_distiller_service.h index b2698b9e1ac7..cd59950bd7fa 100644 --- a/components/dom_distiller/core/dom_distiller_service.h +++ b/components/dom_distiller/core/dom_distiller_service.h @@ -84,6 +84,8 @@ class DomDistillerServiceInterface { // Creates a default DistillerPage. virtual scoped_ptr CreateDefaultDistillerPage() = 0; + virtual scoped_ptr CreateDefaultDistillerPageWithHandle( + scoped_ptr handle) = 0; virtual void AddObserver(DomDistillerObserver* observer) = 0; virtual void RemoveObserver(DomDistillerObserver* observer) = 0; @@ -121,6 +123,8 @@ class DomDistillerService : public DomDistillerServiceInterface { scoped_ptr distiller_page, const GURL& url) OVERRIDE; virtual scoped_ptr CreateDefaultDistillerPage() OVERRIDE; + virtual scoped_ptr CreateDefaultDistillerPageWithHandle( + scoped_ptr handle) OVERRIDE; virtual void AddObserver(DomDistillerObserver* observer) OVERRIDE; virtual void RemoveObserver(DomDistillerObserver* observer) OVERRIDE; diff --git a/components/dom_distiller/core/fake_distiller_page.h b/components/dom_distiller/core/fake_distiller_page.h index 245d23eb8c32..2e88389c2e67 100644 --- a/components/dom_distiller/core/fake_distiller_page.h +++ b/components/dom_distiller/core/fake_distiller_page.h @@ -19,6 +19,10 @@ class MockDistillerPageFactory : public DistillerPageFactory { virtual scoped_ptr CreateDistillerPage() const OVERRIDE { return scoped_ptr(CreateDistillerPageImpl()); } + virtual scoped_ptr CreateDistillerPageWithHandle( + scoped_ptr handle) const OVERRIDE { + return scoped_ptr(CreateDistillerPageImpl()); + } }; class MockDistillerPage : public DistillerPage { diff --git a/components/dom_distiller/core/viewer_unittest.cc b/components/dom_distiller/core/viewer_unittest.cc index b0cea2a731e4..7c1b0752849a 100644 --- a/components/dom_distiller/core/viewer_unittest.cc +++ b/components/dom_distiller/core/viewer_unittest.cc @@ -62,6 +62,10 @@ class TestDomDistillerService : public DomDistillerServiceInterface { virtual scoped_ptr CreateDefaultDistillerPage() { return scoped_ptr(); } + virtual scoped_ptr CreateDefaultDistillerPageWithHandle( + scoped_ptr handle) { + return scoped_ptr(); + } }; class DomDistillerViewerTest : public testing::Test { -- 2.11.4.GIT