diff options
25 files changed, 490 insertions, 43 deletions
diff --git a/chrome/browser/dom_distiller/dom_distiller_service_factory.cc b/chrome/browser/dom_distiller/dom_distiller_service_factory.cc index 0d9d5e3..f249672 100644 --- a/chrome/browser/dom_distiller/dom_distiller_service_factory.cc +++ b/chrome/browser/dom_distiller/dom_distiller_service_factory.cc @@ -5,6 +5,7 @@ #include "chrome/browser/dom_distiller/dom_distiller_service_factory.h" #include "base/threading/sequenced_worker_pool.h" +#include "chrome/browser/profiles/profile.h" #include "components/dom_distiller/content/distiller_page_web_contents.h" #include "components/dom_distiller/core/article_entry.h" #include "components/dom_distiller/core/distiller.h" @@ -20,9 +21,13 @@ namespace dom_distiller { DomDistillerContextKeyedService::DomDistillerContextKeyedService( scoped_ptr<DomDistillerStoreInterface> store, scoped_ptr<DistillerFactory> distiller_factory, - scoped_ptr<DistillerPageFactory> distiller_page_factory) - : DomDistillerService(store.Pass(), distiller_factory.Pass(), - distiller_page_factory.Pass()) {} + scoped_ptr<DistillerPageFactory> distiller_page_factory, + scoped_ptr<DistilledPagePrefs> distilled_page_prefs) + : DomDistillerService(store.Pass(), + distiller_factory.Pass(), + distiller_page_factory.Pass(), + distilled_page_prefs.Pass()) { +} // static DomDistillerServiceFactory* DomDistillerServiceFactory::GetInstance() { @@ -72,11 +77,15 @@ KeyedService* DomDistillerServiceFactory::BuildServiceInstanceFor( } scoped_ptr<DistillerFactory> distiller_factory( new DistillerFactoryImpl(distiller_url_fetcher_factory.Pass(), options)); + scoped_ptr<DistilledPagePrefs> distilled_page_prefs( + new DistilledPagePrefs(Profile::FromBrowserContext(profile)->GetPrefs())); DomDistillerContextKeyedService* service = new DomDistillerContextKeyedService( dom_distiller_store.PassAs<DomDistillerStoreInterface>(), - distiller_factory.Pass(), distiller_page_factory.Pass()); + distiller_factory.Pass(), + distiller_page_factory.Pass(), + distilled_page_prefs.Pass()); return service; } diff --git a/chrome/browser/dom_distiller/dom_distiller_service_factory.h b/chrome/browser/dom_distiller/dom_distiller_service_factory.h index 78644b3..ee37659 100644 --- a/chrome/browser/dom_distiller/dom_distiller_service_factory.h +++ b/chrome/browser/dom_distiller/dom_distiller_service_factory.h @@ -6,6 +6,7 @@ #define CHROME_BROWSER_DOM_DISTILLER_DOM_DISTILLER_SERVICE_FACTORY_H_ #include "base/memory/singleton.h" +#include "components/dom_distiller/core/distilled_page_prefs.h" #include "components/dom_distiller/core/dom_distiller_service.h" #include "components/keyed_service/content/browser_context_keyed_service_factory.h" @@ -23,7 +24,8 @@ class DomDistillerContextKeyedService : public KeyedService, DomDistillerContextKeyedService( scoped_ptr<DomDistillerStoreInterface> store, scoped_ptr<DistillerFactory> distiller_factory, - scoped_ptr<DistillerPageFactory> distiller_page_factory); + scoped_ptr<DistillerPageFactory> distiller_page_factory, + scoped_ptr<DistilledPagePrefs> distilled_page_prefs); virtual ~DomDistillerContextKeyedService() {} private: diff --git a/chrome/browser/dom_distiller/dom_distiller_viewer_source_browsertest.cc b/chrome/browser/dom_distiller/dom_distiller_viewer_source_browsertest.cc index d36e45b..4640bc9 100644 --- a/chrome/browser/dom_distiller/dom_distiller_viewer_source_browsertest.cc +++ b/chrome/browser/dom_distiller/dom_distiller_viewer_source_browsertest.cc @@ -18,6 +18,7 @@ #include "chrome/test/base/ui_test_utils.h" #include "components/dom_distiller/content/dom_distiller_viewer_source.h" #include "components/dom_distiller/core/article_entry.h" +#include "components/dom_distiller/core/distilled_page_prefs.h" #include "components/dom_distiller/core/distiller.h" #include "components/dom_distiller/core/dom_distiller_service.h" #include "components/dom_distiller/core/dom_distiller_store.h" @@ -57,6 +58,10 @@ const char kGetContent[] = "window.domAutomationController.send(" "document.getElementById('content').innerHTML)"; +const char kGetBodyClass[] = + "window.domAutomationController.send(" + "document.body.className)"; + void AddEntry(const ArticleEntry& e, FakeDB<ArticleEntry>::EntryMap* map) { (*map)[e.entry_id()] = e; } @@ -99,7 +104,11 @@ class DomDistillerViewerSourceBrowserTest : public InProcessBrowserTest { CreateStoreWithFakeDB(fake_db, FakeDB<ArticleEntry>::EntryMap())), scoped_ptr<DistillerFactory>(distiller_factory_), - scoped_ptr<DistillerPageFactory>(distiller_page_factory_)); + scoped_ptr<DistillerPageFactory>(distiller_page_factory_), + scoped_ptr<DistilledPagePrefs>( + new DistilledPagePrefs( + Profile::FromBrowserContext( + context)->GetPrefs()))); fake_db->InitCallback(true); fake_db->LoadCallback(true); if (expect_distillation_) { @@ -321,4 +330,31 @@ IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, EXPECT_THAT(result, HasSubstr("Page 2 content")); } +IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, PrefChange) { + expect_distillation_ = true; + expect_distiller_page_ = true; + GURL view_url("http://www.example.com/1"); + content::WebContents* contents = + browser()->tab_strip_model()->GetActiveWebContents(); + const GURL url = url_utils::GetDistillerViewUrlFromUrl( + chrome::kDomDistillerScheme, view_url); + ViewSingleDistilledPage(url, "text/html"); + content::WaitForLoadStop(contents); + std::string result; + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + contents, kGetBodyClass, &result)); + EXPECT_EQ("light", result); + + // Getting DistilledPagePrefs instance. + DistilledPagePrefs* distilled_page_prefs = + DomDistillerServiceFactory::GetForBrowserContext( + browser()->profile())->GetDistilledPagePrefs(); + + distilled_page_prefs->SetTheme(DistilledPagePrefs::DARK); + base::RunLoop().RunUntilIdle(); + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + contents, kGetBodyClass, &result)); + EXPECT_EQ("dark", result); +} + } // namespace dom_distiller diff --git a/chrome/browser/dom_distiller/lazy_dom_distiller_service.cc b/chrome/browser/dom_distiller/lazy_dom_distiller_service.cc index 3e7e745..22c3c66 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/distilled_page_prefs.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" @@ -93,4 +94,8 @@ void LazyDomDistillerService::RemoveObserver(DomDistillerObserver* observer) { instance()->RemoveObserver(observer); } +DistilledPagePrefs* LazyDomDistillerService::GetDistilledPagePrefs() { + return instance()->GetDistilledPagePrefs(); +} + } // namespace dom_distiller diff --git a/chrome/browser/dom_distiller/lazy_dom_distiller_service.h b/chrome/browser/dom_distiller/lazy_dom_distiller_service.h index d3ed5f4..d63e9f4 100644 --- a/chrome/browser/dom_distiller/lazy_dom_distiller_service.h +++ b/chrome/browser/dom_distiller/lazy_dom_distiller_service.h @@ -54,6 +54,7 @@ class LazyDomDistillerService : public DomDistillerServiceInterface, scoped_ptr<SourcePageHandle> handle) OVERRIDE; virtual void AddObserver(DomDistillerObserver* observer) OVERRIDE; virtual void RemoveObserver(DomDistillerObserver* observer) OVERRIDE; + virtual DistilledPagePrefs* GetDistilledPagePrefs() OVERRIDE; private: // Accessor method for the backing service instance. diff --git a/chrome/browser/extensions/api/reading_list_private/reading_list_private_apitest.cc b/chrome/browser/extensions/api/reading_list_private/reading_list_private_apitest.cc index fac874c..375ff99 100644 --- a/chrome/browser/extensions/api/reading_list_private/reading_list_private_apitest.cc +++ b/chrome/browser/extensions/api/reading_list_private/reading_list_private_apitest.cc @@ -7,12 +7,14 @@ #include "chrome/browser/extensions/extension_apitest.h" #include "chrome/browser/profiles/profile.h" #include "components/dom_distiller/core/article_entry.h" +#include "components/dom_distiller/core/distilled_page_prefs.h" #include "components/dom_distiller/core/dom_distiller_service.h" #include "components/dom_distiller/core/dom_distiller_store.h" #include "components/dom_distiller/core/dom_distiller_test_util.h" #include "components/dom_distiller/core/fake_distiller.h" #include "components/dom_distiller/core/fake_distiller_page.h" #include "components/leveldb_proto/testing/fake_db.h" +#include "components/pref_registry/testing_pref_service_syncable.h" using dom_distiller::ArticleEntry; using dom_distiller::test::FakeDistiller; @@ -37,13 +39,22 @@ class ReadingListPrivateApiTest : public ExtensionApiTest { MockDistillerFactory* distiller_factory = new MockDistillerFactory(); MockDistillerPageFactory* distiller_page_factory = new MockDistillerPageFactory(); + + // Setting up pref service for DistilledPagePrefs. + user_prefs::TestingPrefServiceSyncable* pref_service = + new user_prefs::TestingPrefServiceSyncable(); + dom_distiller::DistilledPagePrefs::RegisterProfilePrefs( + pref_service->registry()); + DomDistillerContextKeyedService* service = new DomDistillerContextKeyedService( scoped_ptr<DomDistillerStoreInterface>( CreateStoreWithFakeDB(fake_db, FakeDB<ArticleEntry>::EntryMap())), scoped_ptr<DistillerFactory>(distiller_factory), - scoped_ptr<DistillerPageFactory>(distiller_page_factory)); + scoped_ptr<DistillerPageFactory>(distiller_page_factory), + scoped_ptr<dom_distiller::DistilledPagePrefs>( + new dom_distiller::DistilledPagePrefs(pref_service))); fake_db->InitCallback(true); fake_db->LoadCallback(true); EXPECT_CALL(*distiller_factory, CreateDistillerImpl()) diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc index 357ae0f..73277e2 100644 --- a/chrome/browser/prefs/browser_prefs.cc +++ b/chrome/browser/prefs/browser_prefs.cc @@ -85,6 +85,7 @@ #include "chrome/common/pref_names.h" #include "components/autofill/core/browser/autofill_manager.h" #include "components/bookmarks/browser/bookmark_utils.h" +#include "components/dom_distiller/core/distilled_page_prefs.h" #include "components/google/core/browser/google_pref_names.h" #include "components/google/core/browser/google_url_tracker.h" #include "components/network_time/network_time_tracker.h" @@ -358,6 +359,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) { chrome_browser_net::Predictor::RegisterProfilePrefs(registry); chrome_browser_net::RegisterPredictionOptionsProfilePrefs(registry); chrome_prefs::RegisterProfilePrefs(registry); + dom_distiller::DistilledPagePrefs::RegisterProfilePrefs(registry); DownloadPrefs::RegisterProfilePrefs(registry); easy_unlock::RegisterProfilePrefs(registry); gcm::GCMProfileService::RegisterProfilePrefs(registry); diff --git a/components/components_tests.gyp b/components/components_tests.gyp index 61d2ffb..b33019a 100644 --- a/components/components_tests.gyp +++ b/components/components_tests.gyp @@ -83,6 +83,7 @@ 'data_reduction_proxy/common/data_reduction_proxy_headers_unittest.cc', 'dom_distiller/core/article_entry_unittest.cc', 'dom_distiller/core/distilled_content_store_unittest.cc', + 'dom_distiller/core/distilled_page_prefs_unittests.cc', 'dom_distiller/core/distiller_unittest.cc', 'dom_distiller/core/distiller_url_fetcher_unittest.cc', 'dom_distiller/core/dom_distiller_model_unittest.cc', @@ -743,6 +744,7 @@ 'components.gyp:autofill_content_browser', 'components.gyp:dom_distiller_content', 'components.gyp:dom_distiller_core', + 'components.gyp:pref_registry_test_support', 'components_resources.gyp:components_resources', '../content/content.gyp:content_common', '../content/content.gyp:content_gpu', diff --git a/components/dom_distiller.gypi b/components/dom_distiller.gypi index 5f8c4bb..fc88cd2 100644 --- a/components/dom_distiller.gypi +++ b/components/dom_distiller.gypi @@ -60,6 +60,8 @@ 'dom_distiller/core/article_entry.h', 'dom_distiller/core/distilled_content_store.cc', 'dom_distiller/core/distilled_content_store.h', + 'dom_distiller/core/distilled_page_prefs.cc', + 'dom_distiller/core/distilled_page_prefs.h', 'dom_distiller/core/distiller.cc', 'dom_distiller/core/distiller.h', 'dom_distiller/core/distiller_page.cc', diff --git a/components/dom_distiller/DEPS b/components/dom_distiller/DEPS index cb7ec79..f21df2f 100644 --- a/components/dom_distiller/DEPS +++ b/components/dom_distiller/DEPS @@ -1,5 +1,6 @@ include_rules = [ "+components/leveldb_proto", + "+components/pref_registry", "+google", # For third_party/protobuf. "+grit", # For generated headers. "+jni", diff --git a/components/dom_distiller/content/dom_distiller_viewer_source.cc b/components/dom_distiller/content/dom_distiller_viewer_source.cc index 8f475cc..72e2db2 100644 --- a/components/dom_distiller/content/dom_distiller_viewer_source.cc +++ b/components/dom_distiller/content/dom_distiller_viewer_source.cc @@ -12,6 +12,8 @@ #include "base/memory/scoped_ptr.h" #include "base/message_loop/message_loop.h" #include "base/strings/utf_string_conversions.h" +#include "components/dom_distiller/core/distilled_page_prefs.h" +#include "components/dom_distiller/core/dom_distiller_service.h" #include "components/dom_distiller/core/task_tracker.h" #include "components/dom_distiller/core/url_constants.h" #include "components/dom_distiller/core/viewer.h" @@ -31,16 +33,18 @@ namespace dom_distiller { // the current main frame's page in the Viewer instance. class DomDistillerViewerSource::RequestViewerHandle : public ViewRequestDelegate, - public content::WebContentsObserver { + public content::WebContentsObserver, + public DistilledPagePrefs::Observer { public: explicit RequestViewerHandle( content::WebContents* web_contents, const std::string& expected_scheme, const std::string& expected_request_path, - const content::URLDataSource::GotDataCallback& callback); + const content::URLDataSource::GotDataCallback& callback, + DistilledPagePrefs* distilled_page_prefs); virtual ~RequestViewerHandle(); - // ViewRequestDelegate implementation. + // ViewRequestDelegate implementation: virtual void OnArticleReady( const DistilledArticleProto* article_proto) OVERRIDE; @@ -49,7 +53,7 @@ class DomDistillerViewerSource::RequestViewerHandle void TakeViewerHandle(scoped_ptr<ViewerHandle> viewer_handle); - // WebContentsObserver: + // content::WebContentsObserver implementation: virtual void DidNavigateMainFrame( const content::LoadCommittedDetails& details, const content::FrameNavigateParams& params) OVERRIDE; @@ -68,6 +72,9 @@ class DomDistillerViewerSource::RequestViewerHandle // cancelled. void Cancel(); + // DistilledPagePrefs::Observer implementation: + virtual void OnChangeTheme(DistilledPagePrefs::Theme new_theme) OVERRIDE; + // The handle to the view request towards the DomDistillerService. It // needs to be kept around to ensure the distillation request finishes. scoped_ptr<ViewerHandle> viewer_handle_; @@ -85,6 +92,9 @@ class DomDistillerViewerSource::RequestViewerHandle // the viewer. int page_count_; + // Interface for accessing preferences for distilled pages. + DistilledPagePrefs* distilled_page_prefs_; + // Whether the page is sufficiently initialized to handle updates from the // distiller. bool waiting_for_page_ready_; @@ -98,16 +108,20 @@ DomDistillerViewerSource::RequestViewerHandle::RequestViewerHandle( content::WebContents* web_contents, const std::string& expected_scheme, const std::string& expected_request_path, - const content::URLDataSource::GotDataCallback& callback) + const content::URLDataSource::GotDataCallback& callback, + DistilledPagePrefs* distilled_page_prefs) : expected_scheme_(expected_scheme), expected_request_path_(expected_request_path), callback_(callback), page_count_(0), + distilled_page_prefs_(distilled_page_prefs), waiting_for_page_ready_(true) { content::WebContentsObserver::Observe(web_contents); + distilled_page_prefs_->AddObserver(this); } DomDistillerViewerSource::RequestViewerHandle::~RequestViewerHandle() { + distilled_page_prefs_->RemoveObserver(this); } void DomDistillerViewerSource::RequestViewerHandle::SendJavaScript( @@ -173,7 +187,8 @@ void DomDistillerViewerSource::RequestViewerHandle::OnArticleReady( const DistilledArticleProto* article_proto) { if (page_count_ == 0) { // This is a single-page article. - std::string unsafe_page_html = viewer::GetUnsafeArticleHtml(article_proto); + std::string unsafe_page_html = viewer::GetUnsafeArticleHtml( + article_proto, distilled_page_prefs_->GetTheme()); callback_.Run(base::RefCountedString::TakeString(&unsafe_page_html)); } else if (page_count_ == article_proto->pages_size()) { // We may still be showing the "Loading" indicator. @@ -201,7 +216,8 @@ void DomDistillerViewerSource::RequestViewerHandle::OnArticleUpdated( article_update.GetDistilledPage(page_count_); if (page_count_ == 0) { // This is the first page, so send Viewer page scaffolding too. - std::string unsafe_page_html = viewer::GetUnsafePartialArticleHtml(&page); + std::string unsafe_page_html = viewer::GetUnsafePartialArticleHtml( + &page, distilled_page_prefs_->GetTheme()); callback_.Run(base::RefCountedString::TakeString(&unsafe_page_html)); } else { SendJavaScript( @@ -215,6 +231,11 @@ void DomDistillerViewerSource::RequestViewerHandle::TakeViewerHandle( viewer_handle_ = viewer_handle.Pass(); } +void DomDistillerViewerSource::RequestViewerHandle::OnChangeTheme( + DistilledPagePrefs::Theme new_theme) { + SendJavaScript(viewer::GetDistilledPageThemeJs(new_theme)); +} + DomDistillerViewerSource::DomDistillerViewerSource( DomDistillerServiceInterface* dom_distiller_service, const std::string& scheme) @@ -261,7 +282,8 @@ void DomDistillerViewerSource::StartDataRequest( const std::string path_after_query_separator = path.size() > 0 ? path.substr(1) : ""; RequestViewerHandle* request_viewer_handle = new RequestViewerHandle( - web_contents, scheme_, path_after_query_separator, callback); + web_contents, scheme_, path_after_query_separator, callback, + dom_distiller_service_->GetDistilledPagePrefs()); scoped_ptr<ViewerHandle> viewer_handle = viewer::CreateViewRequest( dom_distiller_service_, path, request_viewer_handle); @@ -276,7 +298,8 @@ void DomDistillerViewerSource::StartDataRequest( // |RequestViewerHandle| will never be called, so clean up now. delete request_viewer_handle; - std::string error_page_html = viewer::GetErrorPageHtml(); + std::string error_page_html = viewer::GetErrorPageHtml( + dom_distiller_service_->GetDistilledPagePrefs()->GetTheme()); callback.Run(base::RefCountedString::TakeString(&error_page_html)); } }; diff --git a/components/dom_distiller/content/resources/dom_distiller_viewer.html b/components/dom_distiller/content/resources/dom_distiller_viewer.html index 9812c4d..e5b0e99 100644 --- a/components/dom_distiller/content/resources/dom_distiller_viewer.html +++ b/components/dom_distiller/content/resources/dom_distiller_viewer.html @@ -12,11 +12,11 @@ found in the LICENSE file. <link rel="stylesheet" href="/$2"> <script src="/$3"></script> </head> -<body> +<body class="$4"> <div id="mainContent"> <article> <header> - <h1>$4</h1> + <h1>$1</h1> </header> <div id="content">$5</div> </article> diff --git a/components/dom_distiller/content/resources/dom_distiller_viewer.js b/components/dom_distiller/content/resources/dom_distiller_viewer.js index 854e1d0..9429946 100644 --- a/components/dom_distiller/content/resources/dom_distiller_viewer.js +++ b/components/dom_distiller/content/resources/dom_distiller_viewer.js @@ -11,4 +11,18 @@ function addToPage(html) { function showLoadingIndicator(isLastPage) { document.getElementById('loadingIndicator').className = isLastPage ? 'hidden' : 'visible'; -}
\ No newline at end of file +} + +// Maps JS theme to CSS class and then changes body class name. +// CSS classes must agree with distilledpage.css. +function useTheme(theme) { + var cssClass; + if (theme == "sepia") { + cssClass = "sepia"; + } else if (theme == "dark") { + cssClass = "dark"; + } else { + cssClass = "light"; + } + document.body.className = cssClass; +} diff --git a/components/dom_distiller/core/css/distilledpage.css b/components/dom_distiller/core/css/distilledpage.css index edd12ff..c4c42ba 100644 --- a/components/dom_distiller/core/css/distilledpage.css +++ b/components/dom_distiller/core/css/distilledpage.css @@ -75,7 +75,6 @@ th { body, html { - color: #333; font-family: 'Open Sans', sans-serif; font-size: 14px; line-height: 1.4; @@ -83,6 +82,25 @@ html { overflow-x: hidden; } +/* Classes for light, dark and sepia themes. + * Must agree with classes returned by useTheme() in dom_distiller_viewer.js + * and with CSS class constants in viewer.cc */ + +.light { + color: #333; + background-color: #FFF; +} + +.dark { + color: #FFF; + background-color: #000; +} + +.sepia { + color: #000; + background-color: rgb(203, 173, 141); +} + /* Define vertical rhythm (baseline grid of 4px). */ blockquote, @@ -133,8 +151,14 @@ h6 { margin: 1.296rem 1.296rem auto; } -a { - color: #222; +/* Link colors for light, dark and sepia themes */ + +a:link { + color: #55F; +} + +a:visited { + color: #902290; } blockquote { @@ -203,13 +227,27 @@ ul { margin-left: 1.296rem; } -code, -pre { +.light code, +.light pre, +.sepia code, +.sepia pre { background-color: #f8f8f8; border: 1px solid #eee; border-radius: 2px; } +.dark code, +.dark pre { + background-color: #333; + border: 1px solid #555; + border-radius: 2px; +} + +code { + display: block; + padding: .4444rem; +} + pre code { border: none; padding: 0; diff --git a/components/dom_distiller/core/distilled_page_prefs.cc b/components/dom_distiller/core/distilled_page_prefs.cc new file mode 100644 index 0000000..5c23fb4 --- /dev/null +++ b/components/dom_distiller/core/distilled_page_prefs.cc @@ -0,0 +1,71 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/dom_distiller/core/distilled_page_prefs.h" + +#include "base/bind.h" +#include "base/memory/weak_ptr.h" +#include "base/message_loop/message_loop.h" +#include "base/observer_list.h" +#include "base/prefs/pref_service.h" +#include "components/pref_registry/pref_registry_syncable.h" + +namespace { + +// Path to the integer corresponding to user's preference theme. +const char kThemePref[] = "dom_distiller.theme"; +} + +namespace dom_distiller { + +DistilledPagePrefs::DistilledPagePrefs(PrefService* pref_service) + : pref_service_(pref_service), weak_ptr_factory_(this) { +} + +DistilledPagePrefs::~DistilledPagePrefs() { +} + +// static +void DistilledPagePrefs::RegisterProfilePrefs( + user_prefs::PrefRegistrySyncable* registry) { + registry->RegisterIntegerPref( + kThemePref, + DistilledPagePrefs::LIGHT, + user_prefs::PrefRegistrySyncable::SYNCABLE_PREF); +} + +void DistilledPagePrefs::SetTheme(DistilledPagePrefs::Theme new_theme) { + pref_service_->SetInteger(kThemePref, new_theme); + base::MessageLoop::current()->PostTask( + FROM_HERE, + base::Bind(&DistilledPagePrefs::NotifyOnChangeTheme, + weak_ptr_factory_.GetWeakPtr(), + new_theme)); +} + +DistilledPagePrefs::Theme DistilledPagePrefs::GetTheme() { + int theme = pref_service_->GetInteger(kThemePref); + if (theme < 0 || theme >= DistilledPagePrefs::THEME_COUNT) { + // Persisted data was incorrect, trying to clean it up by storing the + // default. + SetTheme(DistilledPagePrefs::LIGHT); + return DistilledPagePrefs::LIGHT; + } + return (Theme) theme; +} + +void DistilledPagePrefs::AddObserver(Observer* obs) { + observers_.AddObserver(obs); +} + +void DistilledPagePrefs::RemoveObserver(Observer* obs) { + observers_.RemoveObserver(obs); +} + +void DistilledPagePrefs::NotifyOnChangeTheme( + DistilledPagePrefs::Theme new_theme) { + FOR_EACH_OBSERVER(Observer, observers_, OnChangeTheme(new_theme)); +} + +} // namespace dom_distiller diff --git a/components/dom_distiller/core/distilled_page_prefs.h b/components/dom_distiller/core/distilled_page_prefs.h new file mode 100644 index 0000000..9b93be5 --- /dev/null +++ b/components/dom_distiller/core/distilled_page_prefs.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_CORE_DISTILLED_PAGE_PREFS_H_ +#define COMPONENTS_DOM_DISTILLER_CORE_DISTILLED_PAGE_PREFS_H_ + +#include "base/macros.h" +#include "base/memory/weak_ptr.h" +#include "base/observer_list.h" + +class PrefService; + +namespace user_prefs { +class PrefRegistrySyncable; +} + +namespace dom_distiller { + +// Interface for preferences used for distilled page. +class DistilledPagePrefs { + public: + // Possible themes for distilled page. + enum Theme { + LIGHT, + DARK, + SEPIA, + THEME_COUNT + }; + + class Observer { + public: + virtual void OnChangeTheme(Theme theme) = 0; + }; + + explicit DistilledPagePrefs(PrefService* pref_service); + ~DistilledPagePrefs(); + + static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry); + + // Sets the user's preference for the theme of distilled pages. + void SetTheme(Theme new_theme); + // Returns the user's preference for the theme of distilled pages. + Theme GetTheme(); + + void AddObserver(Observer* obs); + void RemoveObserver(Observer* obs); + + private: + // Notifies all Observers of new theme. + void NotifyOnChangeTheme(Theme theme); + + PrefService* pref_service_; + ObserverList<Observer> observers_; + + base::WeakPtrFactory<DistilledPagePrefs> weak_ptr_factory_; + + DISALLOW_COPY_AND_ASSIGN(DistilledPagePrefs); +}; + +} // namespace dom_distiller + +#endif // COMPONENTS_DOM_DISTILLER_CORE_DISTILLED_PAGE_PREFS_H_ diff --git a/components/dom_distiller/core/distilled_page_prefs_unittests.cc b/components/dom_distiller/core/distilled_page_prefs_unittests.cc new file mode 100644 index 0000000..1c2ccf8 --- /dev/null +++ b/components/dom_distiller/core/distilled_page_prefs_unittests.cc @@ -0,0 +1,75 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/dom_distiller/core/distilled_page_prefs.h" + +#include "base/message_loop/message_loop.h" +#include "base/run_loop.h" +#include "components/pref_registry/testing_pref_service_syncable.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace dom_distiller { + +namespace { + +class TestingObserver : public DistilledPagePrefs::Observer { + public: + TestingObserver() : theme_(DistilledPagePrefs::LIGHT) {} + + virtual void OnChangeTheme(DistilledPagePrefs::Theme new_theme) OVERRIDE { + theme_ = new_theme; + } + + DistilledPagePrefs::Theme GetTheme() { return theme_; } + + private: + DistilledPagePrefs::Theme theme_; +}; + +} // namespace + +class DistilledPagePrefsTest : public testing::Test { + protected: + virtual void SetUp() OVERRIDE { + user_prefs::TestingPrefServiceSyncable* pref_service = + new user_prefs::TestingPrefServiceSyncable(); + DistilledPagePrefs::RegisterProfilePrefs(pref_service->registry()); + distilled_page_prefs_ = new DistilledPagePrefs(pref_service); + } + + DistilledPagePrefs* distilled_page_prefs_; + + private: + base::MessageLoop message_loop_; +}; + +TEST_F(DistilledPagePrefsTest, TestingOnChangeThemeIsBeingCalled) { + TestingObserver* obs = new TestingObserver(); + distilled_page_prefs_->AddObserver(obs); + distilled_page_prefs_->SetTheme(DistilledPagePrefs::SEPIA); + EXPECT_EQ(DistilledPagePrefs::LIGHT, obs->GetTheme()); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(DistilledPagePrefs::SEPIA, obs->GetTheme()); + distilled_page_prefs_->SetTheme(DistilledPagePrefs::DARK); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(DistilledPagePrefs::DARK, obs->GetTheme()); +} + +TEST_F(DistilledPagePrefsTest, TestingMultipleObservers) { + TestingObserver* obs = new TestingObserver(); + distilled_page_prefs_->AddObserver(obs); + TestingObserver* obs2 = new TestingObserver(); + distilled_page_prefs_->AddObserver(obs2); + distilled_page_prefs_->SetTheme(DistilledPagePrefs::SEPIA); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(DistilledPagePrefs::SEPIA, obs->GetTheme()); + EXPECT_EQ(DistilledPagePrefs::SEPIA, obs2->GetTheme()); + distilled_page_prefs_->RemoveObserver(obs); + distilled_page_prefs_->SetTheme(DistilledPagePrefs::LIGHT); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(DistilledPagePrefs::SEPIA, obs->GetTheme()); + EXPECT_EQ(DistilledPagePrefs::LIGHT, obs2->GetTheme()); +} + +} // namespace dom_distiller diff --git a/components/dom_distiller/core/dom_distiller_service.cc b/components/dom_distiller/core/dom_distiller_service.cc index fd1405d..311d6e1 100644 --- a/components/dom_distiller/core/dom_distiller_service.cc +++ b/components/dom_distiller/core/dom_distiller_service.cc @@ -39,11 +39,13 @@ void RunArticleAvailableCallback( DomDistillerService::DomDistillerService( scoped_ptr<DomDistillerStoreInterface> store, scoped_ptr<DistillerFactory> distiller_factory, - scoped_ptr<DistillerPageFactory> distiller_page_factory) + scoped_ptr<DistillerPageFactory> distiller_page_factory, + scoped_ptr<DistilledPagePrefs> distilled_page_prefs) : store_(store.Pass()), content_store_(new InMemoryContentStore(kDefaultMaxNumCachedEntries)), distiller_factory_(distiller_factory.Pass()), - distiller_page_factory_(distiller_page_factory.Pass()) { + distiller_page_factory_(distiller_page_factory.Pass()), + distilled_page_prefs_(distilled_page_prefs.Pass()) { } DomDistillerService::~DomDistillerService() { @@ -239,4 +241,8 @@ void DomDistillerService::RemoveObserver(DomDistillerObserver* observer) { store_->RemoveObserver(observer); } +DistilledPagePrefs* DomDistillerService::GetDistilledPagePrefs() { + return distilled_page_prefs_.get(); +} + } // namespace dom_distiller diff --git a/components/dom_distiller/core/dom_distiller_service.h b/components/dom_distiller/core/dom_distiller_service.h index cd59950..a48db14 100644 --- a/components/dom_distiller/core/dom_distiller_service.h +++ b/components/dom_distiller/core/dom_distiller_service.h @@ -13,6 +13,7 @@ #include "base/memory/scoped_vector.h" #include "base/memory/weak_ptr.h" #include "components/dom_distiller/core/article_entry.h" +#include "components/dom_distiller/core/distilled_page_prefs.h" #include "components/dom_distiller/core/distiller_page.h" class GURL; @@ -90,6 +91,10 @@ class DomDistillerServiceInterface { virtual void AddObserver(DomDistillerObserver* observer) = 0; virtual void RemoveObserver(DomDistillerObserver* observer) = 0; + // Returns the DistilledPagePrefs owned by the instance of + // DomDistillerService. + virtual DistilledPagePrefs* GetDistilledPagePrefs() = 0; + protected: DomDistillerServiceInterface() {} @@ -102,7 +107,8 @@ class DomDistillerService : public DomDistillerServiceInterface { public: DomDistillerService(scoped_ptr<DomDistillerStoreInterface> store, scoped_ptr<DistillerFactory> distiller_factory, - scoped_ptr<DistillerPageFactory> distiller_page_factory); + scoped_ptr<DistillerPageFactory> distiller_page_factory, + scoped_ptr<DistilledPagePrefs> distilled_page_prefs); virtual ~DomDistillerService(); // DomDistillerServiceInterface implementation. @@ -127,6 +133,7 @@ class DomDistillerService : public DomDistillerServiceInterface { scoped_ptr<SourcePageHandle> handle) OVERRIDE; virtual void AddObserver(DomDistillerObserver* observer) OVERRIDE; virtual void RemoveObserver(DomDistillerObserver* observer) OVERRIDE; + virtual DistilledPagePrefs* GetDistilledPagePrefs() OVERRIDE; private: void CancelTask(TaskTracker* task); @@ -148,6 +155,7 @@ class DomDistillerService : public DomDistillerServiceInterface { scoped_ptr<DistilledContentStore> content_store_; scoped_ptr<DistillerFactory> distiller_factory_; scoped_ptr<DistillerPageFactory> distiller_page_factory_; + scoped_ptr<DistilledPagePrefs> distilled_page_prefs_; typedef ScopedVector<TaskTracker> TaskList; TaskList tasks_; diff --git a/components/dom_distiller/core/dom_distiller_service_unittest.cc b/components/dom_distiller/core/dom_distiller_service_unittest.cc index 777e3fe..f606bda36 100644 --- a/components/dom_distiller/core/dom_distiller_service_unittest.cc +++ b/components/dom_distiller/core/dom_distiller_service_unittest.cc @@ -11,6 +11,7 @@ #include "base/run_loop.h" #include "base/strings/string_number_conversions.h" #include "components/dom_distiller/core/article_entry.h" +#include "components/dom_distiller/core/distilled_page_prefs.h" #include "components/dom_distiller/core/dom_distiller_model.h" #include "components/dom_distiller/core/dom_distiller_store.h" #include "components/dom_distiller/core/dom_distiller_test_util.h" @@ -89,7 +90,8 @@ class DomDistillerServiceTest : public testing::Test { service_.reset(new DomDistillerService( scoped_ptr<DomDistillerStoreInterface>(store_), scoped_ptr<DistillerFactory>(distiller_factory_), - scoped_ptr<DistillerPageFactory>(distiller_page_factory_))); + scoped_ptr<DistillerPageFactory>(distiller_page_factory_), + scoped_ptr<DistilledPagePrefs>())); fake_db->InitCallback(true); fake_db->LoadCallback(true); } diff --git a/components/dom_distiller/core/viewer.cc b/components/dom_distiller/core/viewer.cc index e3c7796..8a9d937 100644 --- a/components/dom_distiller/core/viewer.cc +++ b/components/dom_distiller/core/viewer.cc @@ -10,6 +10,7 @@ #include "base/json/json_writer.h" #include "base/memory/scoped_ptr.h" #include "base/strings/string_util.h" +#include "components/dom_distiller/core/distilled_page_prefs.h" #include "components/dom_distiller/core/dom_distiller_service.h" #include "components/dom_distiller/core/proto/distilled_article.pb.h" #include "components/dom_distiller/core/proto/distilled_page.pb.h" @@ -28,11 +29,42 @@ namespace dom_distiller { namespace { +// JS Themes. Must agree with useTheme() in dom_distiller_viewer.js. +const char kDarkJsTheme[] = "dark"; +const char kLightJsTheme[] = "light"; +const char kSepiaJsTheme[] = "sepia"; + +// CSS classes. Must agree with classes in distilledpage.css. +const char kDarkCssClass[] = "dark"; +const char kLightCssClass[] = "light"; +const char kSepiaCssClass[] = "sepia"; + +// Maps themes to JS themes. +const std::string GetJsTheme(DistilledPagePrefs::Theme theme) { + if (theme == DistilledPagePrefs::DARK) { + return kDarkJsTheme; + } else if (theme == DistilledPagePrefs::SEPIA) { + return kSepiaJsTheme; + } + return kLightJsTheme; +} + +// Maps themes to CSS classes. +const std::string GetCssClass(DistilledPagePrefs::Theme theme) { + if (theme == DistilledPagePrefs::DARK) { + return kDarkCssClass; + } else if (theme == DistilledPagePrefs::SEPIA) { + return kSepiaCssClass; + } + return kLightCssClass; +} + std::string ReplaceHtmlTemplateValues( const std::string& title, const std::string& content, const std::string& loading_indicator_class, - const std::string& original_url) { + const std::string& original_url, + const DistilledPagePrefs::Theme theme) { base::StringPiece html_template = ResourceBundle::GetSharedInstance().GetRawDataResource( IDR_DOM_DISTILLER_VIEWER_HTML); @@ -40,7 +72,7 @@ std::string ReplaceHtmlTemplateValues( substitutions.push_back(title); // $1 substitutions.push_back(kViewerCssPath); // $2 substitutions.push_back(kViewerJsPath); // $3 - substitutions.push_back(title); // $4 + substitutions.push_back(GetCssClass(theme)); // $4 substitutions.push_back(content); // $5 substitutions.push_back(loading_indicator_class); // $6 substitutions.push_back( @@ -76,7 +108,8 @@ const std::string GetToggleLoadingIndicatorJs(const bool is_last_page) { } const std::string GetUnsafePartialArticleHtml( - const DistilledPageProto* page_proto) { + const DistilledPageProto* page_proto, + const DistilledPagePrefs::Theme theme) { DCHECK(page_proto); std::string title = net::EscapeForHTML(page_proto->title()); std::ostringstream unsafe_output_stream; @@ -86,11 +119,13 @@ const std::string GetUnsafePartialArticleHtml( return ReplaceHtmlTemplateValues(title, unsafe_article_html, "visible", - original_url); + original_url, + theme); } const std::string GetUnsafeArticleHtml( - const DistilledArticleProto* article_proto) { + const DistilledArticleProto* article_proto, + const DistilledPagePrefs::Theme theme) { DCHECK(article_proto); std::string title; std::string unsafe_article_html; @@ -116,21 +151,21 @@ const std::string GetUnsafeArticleHtml( return ReplaceHtmlTemplateValues(title, unsafe_article_html, "hidden", - original_url); + original_url, + theme); } -const std::string GetErrorPageHtml() { +const std::string GetErrorPageHtml(const DistilledPagePrefs::Theme theme) { std::string title = l10n_util::GetStringUTF8( IDS_DOM_DISTILLER_VIEWER_FAILED_TO_FIND_ARTICLE_TITLE); std::string content = l10n_util::GetStringUTF8( IDS_DOM_DISTILLER_VIEWER_FAILED_TO_FIND_ARTICLE_CONTENT); - return ReplaceHtmlTemplateValues(title, content, "hidden", ""); + return ReplaceHtmlTemplateValues(title, content, "hidden", "", theme); } const std::string GetCss() { - return ResourceBundle::GetSharedInstance() - .GetRawDataResource(IDR_DISTILLER_CSS) - .as_string(); + return ResourceBundle::GetSharedInstance().GetRawDataResource( + IDR_DISTILLER_CSS).as_string(); } const std::string GetJavaScript() { @@ -175,6 +210,10 @@ scoped_ptr<ViewerHandle> CreateViewRequest( return scoped_ptr<ViewerHandle>(); } +const std::string GetDistilledPageThemeJs(DistilledPagePrefs::Theme theme) { + return "useTheme('" + GetJsTheme(theme) + "');"; +} + } // namespace viewer } // namespace dom_distiller diff --git a/components/dom_distiller/core/viewer.h b/components/dom_distiller/core/viewer.h index 98a7e99..4339fc7 100644 --- a/components/dom_distiller/core/viewer.h +++ b/components/dom_distiller/core/viewer.h @@ -11,6 +11,7 @@ #include "base/compiler_specific.h" #include "base/memory/scoped_ptr.h" #include "base/strings/string16.h" +#include "components/dom_distiller/core/distilled_page_prefs.h" namespace dom_distiller { @@ -26,7 +27,8 @@ namespace viewer { // to be displayed to the end user. The returned HTML should be considered // unsafe, so callers must ensure rendering it does not compromise Chrome. const std::string GetUnsafeArticleHtml( - const DistilledArticleProto* article_proto); + const DistilledArticleProto* article_proto, + const DistilledPagePrefs::Theme theme); // Returns the base Viewer HTML page based on the given |page_proto|. This is // supposed to be displayed to the end user. The returned HTML should be @@ -35,7 +37,8 @@ const std::string GetUnsafeArticleHtml( // for displaying an in-flight distillation instead of waiting for the full // article. const std::string GetUnsafePartialArticleHtml( - const DistilledPageProto* page_proto); + const DistilledPageProto* page_proto, + const DistilledPagePrefs::Theme theme); // Returns a JavaScript blob for updating a partial view request with additional // distilled content. Meant for use when viewing a slow or long multi-page @@ -51,7 +54,7 @@ const std::string GetUnsafeIncrementalDistilledPageJs( const std::string GetToggleLoadingIndicatorJs(const bool is_last_page); // Returns a full HTML page which displays a generic error. -const std::string GetErrorPageHtml(); +const std::string GetErrorPageHtml(const DistilledPagePrefs::Theme theme); // Returns the default CSS to be used for a viewer. const std::string GetCss(); @@ -66,6 +69,9 @@ scoped_ptr<ViewerHandle> CreateViewRequest( const std::string& path, ViewRequestDelegate* view_request_delegate); +// Returns JavaScript corresponding to setting a specific theme. +const std::string GetDistilledPageThemeJs(DistilledPagePrefs::Theme theme); + } // namespace viewer } // namespace dom_distiller diff --git a/components/dom_distiller/core/viewer_unittest.cc b/components/dom_distiller/core/viewer_unittest.cc index 7c1b075..20fd27d 100644 --- a/components/dom_distiller/core/viewer_unittest.cc +++ b/components/dom_distiller/core/viewer_unittest.cc @@ -4,6 +4,7 @@ #include "components/dom_distiller/core/viewer.h" +#include "components/dom_distiller/core/distilled_page_prefs.h" #include "components/dom_distiller/core/dom_distiller_service.h" #include "components/dom_distiller/core/dom_distiller_test_util.h" #include "components/dom_distiller/core/task_tracker.h" @@ -66,6 +67,7 @@ class TestDomDistillerService : public DomDistillerServiceInterface { scoped_ptr<SourcePageHandle> handle) { return scoped_ptr<DistillerPage>(); } + virtual DistilledPagePrefs* GetDistilledPagePrefs() OVERRIDE; }; class DomDistillerViewerTest : public testing::Test { @@ -129,4 +131,23 @@ TEST_F(DomDistillerViewerTest, TestCreatingInvalidViewRequest) { view_request_delegate.get()); } +DistilledPagePrefs* TestDomDistillerService::GetDistilledPagePrefs() { + return NULL; +} + +TEST_F(DomDistillerViewerTest, TestGetDistilledPageThemeJsOutput) { + std::string kDarkJs = "useTheme('dark');"; + std::string kSepiaJs = "useTheme('sepia');"; + std::string kLightJs = "useTheme('light');"; + EXPECT_EQ(kDarkJs.compare(viewer::GetDistilledPageThemeJs( + DistilledPagePrefs::DARK)), + 0); + EXPECT_EQ(kLightJs.compare(viewer::GetDistilledPageThemeJs( + DistilledPagePrefs::LIGHT)), + 0); + EXPECT_EQ(kSepiaJs.compare(viewer::GetDistilledPageThemeJs( + DistilledPagePrefs::SEPIA)), + 0); +} + } // namespace dom_distiller diff --git a/components/dom_distiller/standalone/content_extractor.cc b/components/dom_distiller/standalone/content_extractor.cc index 7f7a216..accc0d3 100644 --- a/components/dom_distiller/standalone/content_extractor.cc +++ b/components/dom_distiller/standalone/content_extractor.cc @@ -13,6 +13,7 @@ #include "base/strings/string_split.h" #include "components/dom_distiller/content/distiller_page_web_contents.h" #include "components/dom_distiller/core/article_entry.h" +#include "components/dom_distiller/core/distilled_page_prefs.h" #include "components/dom_distiller/core/distiller.h" #include "components/dom_distiller/core/dom_distiller_service.h" #include "components/dom_distiller/core/dom_distiller_store.h" @@ -21,6 +22,7 @@ #include "components/dom_distiller/core/task_tracker.h" #include "components/leveldb_proto/proto_database.h" #include "components/leveldb_proto/proto_database_impl.h" +#include "components/pref_registry/testing_pref_service_syncable.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/browser_thread.h" #include "content/public/test/content_browser_test.h" @@ -97,10 +99,17 @@ scoped_ptr<DomDistillerService> CreateDomDistillerService( scoped_ptr<DistillerFactory> distiller_factory( new DistillerFactoryImpl(distiller_url_fetcher_factory.Pass(), options)); + // Setting up PrefService for DistilledPagePrefs. + user_prefs::TestingPrefServiceSyncable* pref_service = + new user_prefs::TestingPrefServiceSyncable(); + DistilledPagePrefs::RegisterProfilePrefs(pref_service->registry()); + return scoped_ptr<DomDistillerService>(new DomDistillerService( dom_distiller_store.PassAs<DomDistillerStoreInterface>(), distiller_factory.Pass(), - distiller_page_factory.Pass())); + distiller_page_factory.Pass(), + scoped_ptr<DistilledPagePrefs>( + new DistilledPagePrefs(pref_service)))); } void AddComponentsResources() { diff --git a/components/pref_registry.gypi b/components/pref_registry.gypi index 0661514..14ce007 100644 --- a/components/pref_registry.gypi +++ b/components/pref_registry.gypi @@ -30,6 +30,7 @@ 'type': 'static_library', 'dependencies': [ 'pref_registry', + '../base/base.gyp:base_prefs_test_support', ], 'include_dirs': [ '..', |