diff options
author | mdjones <mdjones@chromium.org> | 2015-05-28 14:06:21 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-05-28 21:06:49 +0000 |
commit | efe2ac1d3e1650a1cec652566542ac492eafedb3 (patch) | |
tree | bcb33f1a4548c8816931eb5a15eabba4730a4d5a | |
parent | 22afe87604125f26beafa2bf04d7542f5275e66b (diff) | |
download | chromium_src-efe2ac1d3e1650a1cec652566542ac492eafedb3.zip chromium_src-efe2ac1d3e1650a1cec652566542ac492eafedb3.tar.gz chromium_src-efe2ac1d3e1650a1cec652566542ac492eafedb3.tar.bz2 |
Show template before distiller finishes
This change allows content to be shown on the page prior to the
distillation of the first page of an article having completed.
The template HTML and the loading animation will show while the
distillation completes.
BUG=
Review URL: https://codereview.chromium.org/1130703003
Cr-Commit-Position: refs/heads/master@{#331852}
19 files changed, 502 insertions, 208 deletions
diff --git a/build/ios/grit_whitelist.txt b/build/ios/grit_whitelist.txt index 3b9f3ee..f155898 100644 --- a/build/ios/grit_whitelist.txt +++ b/build/ios/grit_whitelist.txt @@ -328,6 +328,7 @@ IDS_DOM_DISTILLER_QUALITY_QUESTION IDS_DOM_DISTILLER_VIEWER_FAILED_TO_FIND_ARTICLE_CONTENT IDS_DOM_DISTILLER_VIEWER_FAILED_TO_FIND_ARTICLE_TITLE IDS_DOM_DISTILLER_VIEWER_LOADING_STRING +IDS_DOM_DISTILLER_VIEWER_LOADING_TITLE IDS_DOM_DISTILLER_VIEWER_NO_DATA_CONTENT IDS_DOM_DISTILLER_VIEWER_NO_DATA_TITLE IDS_DOM_DISTILLER_VIEWER_VIEW_ORIGINAL 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 ea54253..c2782ee 100644 --- a/chrome/browser/dom_distiller/dom_distiller_viewer_source_browsertest.cc +++ b/chrome/browser/dom_distiller/dom_distiller_viewer_source_browsertest.cc @@ -257,6 +257,64 @@ IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, } IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, + EarlyTemplateLoad) { + dom_distiller::DomDistillerServiceFactory::GetInstance() + ->SetTestingFactoryAndUse(browser()->profile(), &Build); + + scoped_refptr<content::MessageLoopRunner> distillation_done_runner = + new content::MessageLoopRunner; + + FakeDistiller* distiller = new FakeDistiller( + false, + distillation_done_runner->QuitClosure()); + EXPECT_CALL(*distiller_factory_, CreateDistillerImpl()) + .WillOnce(testing::Return(distiller)); + + // Navigate to a URL. + GURL url(dom_distiller::url_utils::GetDistillerViewUrlFromUrl( + kDomDistillerScheme, GURL("http://urlthatlooksvalid.com"))); + chrome::NavigateParams params(browser(), url, ui::PAGE_TRANSITION_TYPED); + chrome::Navigate(¶ms); + distillation_done_runner->Run(); + + content::WebContents* contents = + browser()->tab_strip_model()->GetActiveWebContents(); + + // Wait for the page load to complete (should only be template). + content::WaitForLoadStop(contents); + std::string result; + // Loading spinner should be on screen at this point. + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + contents, kGetLoadIndicatorClassName , &result)); + EXPECT_EQ("visible", result); + + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + contents, kGetContent , &result)); + EXPECT_THAT(result, Not(HasSubstr("content"))); + + // Finish distillation and make sure the spinner has been replaced by text. + std::vector<scoped_refptr<ArticleDistillationUpdate::RefCountedPageProto> > + update_pages; + scoped_ptr<DistilledArticleProto> article(new DistilledArticleProto()); + + scoped_refptr<base::RefCountedData<DistilledPageProto> > page_proto = + new base::RefCountedData<DistilledPageProto>(); + page_proto->data.set_url("http://foo.html"); + page_proto->data.set_html("<div>content</div>"); + update_pages.push_back(page_proto); + *(article->add_pages()) = page_proto->data; + + ArticleDistillationUpdate update(update_pages, true, false); + distiller->RunDistillerUpdateCallback(update); + + content::WaitForLoadStop(contents); + + EXPECT_TRUE(content::ExecuteScriptAndExtractString( + contents, kGetContent , &result)); + EXPECT_THAT(result, HasSubstr("content")); +} + +IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, MultiPageArticle) { expect_distillation_ = false; expect_distiller_page_ = true; diff --git a/chrome/browser/dom_distiller/tab_utils_browsertest.cc b/chrome/browser/dom_distiller/tab_utils_browsertest.cc index 124aaaf..608c4c1 100644 --- a/chrome/browser/dom_distiller/tab_utils_browsertest.cc +++ b/chrome/browser/dom_distiller/tab_utils_browsertest.cc @@ -22,6 +22,7 @@ #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_observer.h" #include "content/public/test/browser_test_utils.h" +#include "content/public/test/test_utils.h" #include "net/test/embedded_test_server/embedded_test_server.h" #include "testing/gtest/include/gtest/gtest.h" @@ -38,11 +39,17 @@ class DomDistillerTabUtilsBrowserTest : public InProcessBrowserTest { } }; +// WebContentsMainFrameHelper is used to detect if a distilled page has +// finished loading. This is done by checking how many times the title has +// been set rather than using "DidFinishLoad" directly due to the content +// being set by JavaScript. class WebContentsMainFrameHelper : public content::WebContentsObserver { public: WebContentsMainFrameHelper(content::WebContents* web_contents, const base::Closure& callback) - : callback_(callback) { + : callback_(callback), + title_set_count_(0), + loaded_distiller_page_(false) { content::WebContentsObserver::Observe(web_contents); } @@ -50,11 +57,24 @@ class WebContentsMainFrameHelper : public content::WebContentsObserver { const GURL& validated_url) override { if (!render_frame_host->GetParent() && validated_url.scheme() == kDomDistillerScheme) + loaded_distiller_page_ = true; + } + + void TitleWasSet(content::NavigationEntry* entry, + bool explicit_set) override { + // The title will be set twice on distilled pages; once for the placeholder + // and once when the distillation has finished. Watch for the second time + // as a signal that the JavaScript that sets the content has run. + title_set_count_++; + if (title_set_count_ >= 2 && loaded_distiller_page_) { callback_.Run(); + } } private: base::Closure callback_; + int title_set_count_; + bool loaded_distiller_page_; }; #if (defined(OS_LINUX) && defined(OS_CHROMEOS)) @@ -86,12 +106,15 @@ IN_PROC_BROWSER_TEST_F(DomDistillerTabUtilsBrowserTest, new_url_loaded_runner.QuitClosure())); new_url_loaded_runner.Run(); + std::string page_title; + content::ExecuteScriptAndGetValue(after_web_contents->GetMainFrame(), + "document.title")->GetAsString(&page_title); + // 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(kDomDistillerScheme)); - EXPECT_EQ("Test Page Title", - base::UTF16ToUTF8(after_web_contents->GetTitle())); + EXPECT_EQ("Test Page Title", page_title); } IN_PROC_BROWSER_TEST_F(DomDistillerTabUtilsBrowserTest, @@ -128,14 +151,17 @@ IN_PROC_BROWSER_TEST_F(DomDistillerTabUtilsBrowserTest, // Verify that the source WebContents is showing the original article. EXPECT_EQ(article_url, source_web_contents->GetLastCommittedURL()); - EXPECT_EQ("Test Page Title", - base::UTF16ToUTF8(source_web_contents->GetTitle())); + std::string page_title; + content::ExecuteScriptAndGetValue(source_web_contents->GetMainFrame(), + "document.title")->GetAsString(&page_title); + EXPECT_EQ("Test Page Title", page_title); // Verify the destination WebContents is showing distilled content. EXPECT_TRUE(destination_web_contents->GetLastCommittedURL().SchemeIs( kDomDistillerScheme)); - EXPECT_EQ("Test Page Title", - base::UTF16ToUTF8(destination_web_contents->GetTitle())); + content::ExecuteScriptAndGetValue(destination_web_contents->GetMainFrame(), + "document.title")->GetAsString(&page_title); + EXPECT_EQ("Test Page Title", page_title); content::WebContentsDestroyedWatcher destroyed_watcher( destination_web_contents); diff --git a/components/components_tests.gyp b/components/components_tests.gyp index 799a988..d8e492d 100644 --- a/components/components_tests.gyp +++ b/components/components_tests.gyp @@ -150,6 +150,7 @@ 'dom_distiller/core/distiller_url_fetcher_unittest.cc', 'dom_distiller/core/dom_distiller_model_unittest.cc', 'dom_distiller/core/dom_distiller_service_unittest.cc', + 'dom_distiller/core/dom_distiller_request_view_base_unittest.cc', 'dom_distiller/core/dom_distiller_store_unittest.cc', 'dom_distiller/core/page_features_unittest.cc', 'dom_distiller/core/task_tracker_unittest.cc', diff --git a/components/dom_distiller.gypi b/components/dom_distiller.gypi index c2bb83d..cc9da80 100644 --- a/components/dom_distiller.gypi +++ b/components/dom_distiller.gypi @@ -141,6 +141,7 @@ 'dom_distiller/core/fake_distiller.h', 'dom_distiller/core/fake_distiller_page.cc', 'dom_distiller/core/fake_distiller_page.h', + 'dom_distiller/core/test_request_view_handle.h', ], }, { 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 5244a19..aa01a86 100644 --- a/components/dom_distiller/content/distiller_page_web_contents_browsertest.cc +++ b/components/dom_distiller/content/distiller_page_web_contents_browsertest.cc @@ -417,42 +417,6 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, MarkupInfo) { EXPECT_EQ(600, markup_image2.height()); } -IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, TestTitleNeverEmpty) { - const std::string some_title = "some title"; - const std::string no_title = - l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_NO_DATA_TITLE); - - { // Test empty title for article. - scoped_ptr<DistilledArticleProto> article_proto( - new DistilledArticleProto()); - article_proto->set_title(""); - (*(article_proto->add_pages())).set_html(""); - std::string html = viewer::GetUnsafeArticleTemplateHtml( - &article_proto.get()->pages(0), DistilledPagePrefs::LIGHT, - DistilledPagePrefs::SERIF); - EXPECT_THAT(html, HasSubstr(no_title)); - EXPECT_THAT(html, Not(HasSubstr(some_title))); - } - - { // Test empty title for page. - scoped_ptr<DistilledPageProto> page_proto(new DistilledPageProto()); - page_proto->set_title(""); - page_proto->set_html(""); - std::string html = viewer::GetUnsafeArticleTemplateHtml( - page_proto.get(), DistilledPagePrefs::LIGHT, DistilledPagePrefs::SERIF); - EXPECT_THAT(html, HasSubstr(no_title)); - EXPECT_THAT(html, Not(HasSubstr(some_title))); - } - - { // Test missing title for page. - scoped_ptr<DistilledPageProto> page_proto(new DistilledPageProto()); - std::string html = viewer::GetUnsafeArticleTemplateHtml( - page_proto.get(), DistilledPagePrefs::LIGHT, DistilledPagePrefs::SERIF); - EXPECT_THAT(html, HasSubstr(no_title)); - EXPECT_THAT(html, Not(HasSubstr(some_title))); - } -} - IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, TestNoContentDoesNotCrash) { const std::string no_content = diff --git a/components/dom_distiller/content/dom_distiller_viewer_source.cc b/components/dom_distiller/content/dom_distiller_viewer_source.cc index f1ed1a8..b6b611c 100644 --- a/components/dom_distiller/content/dom_distiller_viewer_source.cc +++ b/components/dom_distiller/content/dom_distiller_viewer_source.cc @@ -20,6 +20,7 @@ #include "components/dom_distiller/core/feedback_reporter.h" #include "components/dom_distiller/core/task_tracker.h" #include "components/dom_distiller/core/url_constants.h" +#include "components/dom_distiller/core/url_utils.h" #include "components/dom_distiller/core/viewer.h" #include "content/public/browser/navigation_details.h" #include "content/public/browser/navigation_entry.h" @@ -28,35 +29,13 @@ #include "content/public/browser/user_metrics.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_observer.h" +#include "grit/components_strings.h" #include "net/base/url_util.h" #include "net/url_request/url_request.h" +#include "ui/base/l10n/l10n_util.h" namespace dom_distiller { -namespace { - -class ContentDataCallback : public DistillerDataCallback { - public: - ContentDataCallback(const content::URLDataSource::GotDataCallback& callback); - // Runs the callback. - void RunCallback(std::string& data) override; - - private: - // The callback that actually gets run. - content::URLDataSource::GotDataCallback callback_; -}; - -ContentDataCallback::ContentDataCallback( - const content::URLDataSource::GotDataCallback& callback) { - callback_ = callback; -} - -void ContentDataCallback::RunCallback(std::string& data) { - callback_.Run(base::RefCountedString::TakeString(&data)); -} - -} // namespace - // Handles receiving data asynchronously for a specific entry, and passing // it along to the data callback for the data source. Lifetime matches that of // the current main frame's page in the Viewer instance. @@ -67,7 +46,6 @@ class DomDistillerViewerSource::RequestViewerHandle RequestViewerHandle(content::WebContents* web_contents, const std::string& expected_scheme, const std::string& expected_request_path, - scoped_ptr<ContentDataCallback> callback, DistilledPagePrefs* distilled_page_prefs); ~RequestViewerHandle() override; @@ -109,9 +87,8 @@ DomDistillerViewerSource::RequestViewerHandle::RequestViewerHandle( content::WebContents* web_contents, const std::string& expected_scheme, const std::string& expected_request_path, - scoped_ptr<ContentDataCallback> callback, DistilledPagePrefs* distilled_page_prefs) - : DomDistillerRequestViewBase(callback.Pass(), distilled_page_prefs), + : DomDistillerRequestViewBase(distilled_page_prefs), expected_scheme_(expected_scheme), expected_request_path_(expected_request_path), waiting_for_page_ready_(true) { @@ -173,7 +150,12 @@ void DomDistillerViewerSource::RequestViewerHandle::DidFinishLoad( if (IsErrorPage()) { waiting_for_page_ready_ = false; SendJavaScript(viewer::GetErrorPageJs()); + std::string title(l10n_util::GetStringUTF8( + IDS_DOM_DISTILLER_VIEWER_FAILED_TO_FIND_ARTICLE_CONTENT)); + SendJavaScript(viewer::GetSetTitleJs(title)); + SendJavaScript(viewer::GetSetTextDirectionJs(std::string("auto"))); SendJavaScript(viewer::GetShowFeedbackFormJs()); + Cancel(); // This will cause the object to clean itself up. return; } @@ -250,17 +232,21 @@ void DomDistillerViewerSource::StartDataRequest( DCHECK(web_contents); // An empty |path| is invalid, but guard against it. If not empty, assume // |path| starts with '?', which is stripped away. - scoped_ptr<ContentDataCallback> data_callback( - new ContentDataCallback(callback)); 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, data_callback.Pass(), - dom_distiller_service_->GetDistilledPagePrefs()); + RequestViewerHandle* request_viewer_handle = + new RequestViewerHandle(web_contents, scheme_, path_after_query_separator, + dom_distiller_service_->GetDistilledPagePrefs()); scoped_ptr<ViewerHandle> viewer_handle = viewer::CreateViewRequest( dom_distiller_service_, path, request_viewer_handle, web_contents->GetContainerBounds().size()); + GURL current_url = web_contents->GetLastCommittedURL(); + std::string unsafe_page_html = viewer::GetUnsafeArticleTemplateHtml( + url_utils::GetOriginalUrlFromDistillerUrl(current_url).spec(), + dom_distiller_service_->GetDistilledPagePrefs()->GetTheme(), + dom_distiller_service_->GetDistilledPagePrefs()->GetFontFamily()); + if (viewer_handle) { // The service returned a |ViewerHandle| and guarantees it will call // the |RequestViewerHandle|, so passing ownership to it, to ensure the @@ -270,6 +256,9 @@ void DomDistillerViewerSource::StartDataRequest( } else { request_viewer_handle->FlagAsErrorPage(); } + + // Place template on the page. + callback.Run(base::RefCountedString::TakeString(&unsafe_page_html)); }; std::string DomDistillerViewerSource::GetMimeType( diff --git a/components/dom_distiller/core/BUILD.gn b/components/dom_distiller/core/BUILD.gn index 565f1fe..a14ad21 100644 --- a/components/dom_distiller/core/BUILD.gn +++ b/components/dom_distiller/core/BUILD.gn @@ -98,6 +98,7 @@ source_set("test_support") { "fake_distiller.h", "fake_distiller_page.cc", "fake_distiller_page.h", + "test_request_view_handle.h", ] deps = [ @@ -120,6 +121,7 @@ source_set("unit_tests") { "distiller_unittest.cc", "distiller_url_fetcher_unittest.cc", "dom_distiller_model_unittest.cc", + "dom_distiller_request_view_base_unittest.cc", "dom_distiller_service_unittest.cc", "dom_distiller_store_unittest.cc", "page_features_unittest.cc", @@ -134,6 +136,8 @@ source_set("unit_tests") { "//base", "//components/leveldb_proto:test_support", "//components/pref_registry:test_support", + "//components/resources", + "//components/strings", "//net:test_support", "//sync", "//testing/gmock", diff --git a/components/dom_distiller/core/dom_distiller_request_view_base.cc b/components/dom_distiller/core/dom_distiller_request_view_base.cc index 8a6a30f..8a5266f 100644 --- a/components/dom_distiller/core/dom_distiller_request_view_base.cc +++ b/components/dom_distiller/core/dom_distiller_request_view_base.cc @@ -25,10 +25,8 @@ namespace dom_distiller { DomDistillerRequestViewBase::DomDistillerRequestViewBase( - scoped_ptr<DistillerDataCallback> callback, DistilledPagePrefs* distilled_page_prefs) - : callback_(callback.Pass()), - page_count_(0), + : page_count_(0), distilled_page_prefs_(distilled_page_prefs), is_error_page_(false) { } @@ -38,10 +36,6 @@ DomDistillerRequestViewBase::~DomDistillerRequestViewBase() { void DomDistillerRequestViewBase::FlagAsErrorPage() { is_error_page_ = true; - std::string error_page_html = - viewer::GetErrorPageHtml(distilled_page_prefs_->GetTheme(), - distilled_page_prefs_->GetFontFamily()); - callback_->RunCallback(error_page_html); } bool DomDistillerRequestViewBase::IsErrorPage() { @@ -51,17 +45,15 @@ bool DomDistillerRequestViewBase::IsErrorPage() { void DomDistillerRequestViewBase::OnArticleReady( const DistilledArticleProto* article_proto) { if (page_count_ == 0) { - const DistilledPageProto* cur_page; - if (article_proto->pages().size() < 1) { - cur_page = new DistilledPageProto(); + std::string text_direction; + if (article_proto->pages().size() > 0) { + text_direction = article_proto->pages(0).text_direction(); } else { - cur_page = &article_proto->pages(0); + text_direction = "auto"; } - std::string unsafe_page_html = viewer::GetUnsafeArticleTemplateHtml( - cur_page, distilled_page_prefs_->GetTheme(), - distilled_page_prefs_->GetFontFamily()); - callback_->RunCallback(unsafe_page_html); - // Send first page to client. + // Send first page, title, and text direction to client. + SendJavaScript(viewer::GetSetTitleJs(article_proto->title())); + SendJavaScript(viewer::GetSetTextDirectionJs(text_direction)); SendJavaScript(viewer::GetUnsafeArticleContentJs(article_proto)); // If any content was loaded, show the feedback form. SendJavaScript(viewer::GetShowFeedbackFormJs()); @@ -92,11 +84,10 @@ void DomDistillerRequestViewBase::OnArticleUpdated( SendJavaScript(viewer::GetUnsafeIncrementalDistilledPageJs(&page, false)); if (page_count_ == 0) { - // This is the first page, so send Viewer page scaffolding too. - std::string unsafe_page_html = viewer::GetUnsafeArticleTemplateHtml( - &page, distilled_page_prefs_->GetTheme(), - distilled_page_prefs_->GetFontFamily()); - callback_->RunCallback(unsafe_page_html); + // This is the first page, so send the title and text direction to the + // client. + SendJavaScript(viewer::GetSetTitleJs(page.title())); + SendJavaScript(viewer::GetSetTextDirectionJs(page.text_direction())); // If any content was loaded, show the feedback form. SendJavaScript(viewer::GetShowFeedbackFormJs()); } @@ -116,6 +107,9 @@ void DomDistillerRequestViewBase::OnChangeFontFamily( void DomDistillerRequestViewBase::TakeViewerHandle( scoped_ptr<ViewerHandle> viewer_handle) { viewer_handle_ = viewer_handle.Pass(); + // Getting the viewer handle means this is not an error page, show the + // loading indicator. + SendJavaScript(viewer::GetToggleLoadingIndicatorJs(false)); } } // namespace dom_distiller diff --git a/components/dom_distiller/core/dom_distiller_request_view_base.h b/components/dom_distiller/core/dom_distiller_request_view_base.h index 16b8ca7..ef0fb9f 100644 --- a/components/dom_distiller/core/dom_distiller_request_view_base.h +++ b/components/dom_distiller/core/dom_distiller_request_view_base.h @@ -21,24 +21,13 @@ namespace dom_distiller { -// This interface is used to abstract the data callback from the distiller. The -// callbacks for different platforms have different numbers of parameters -// (namely iOS and Android) which makes this necessary. -class DistillerDataCallback { - public: - virtual ~DistillerDataCallback(){}; - virtual void RunCallback(std::string& data) = 0; -}; - // Handles receiving data asynchronously for a specific entry, and passing // it along to the data callback for the data source. Lifetime matches that of // the current main frame's page in the Viewer instance. -class DomDistillerRequestViewBase - : public ViewRequestDelegate, - public DistilledPagePrefs::Observer { +class DomDistillerRequestViewBase : public ViewRequestDelegate, + public DistilledPagePrefs::Observer { public: explicit DomDistillerRequestViewBase( - scoped_ptr<DistillerDataCallback> callback, DistilledPagePrefs* distilled_page_prefs); ~DomDistillerRequestViewBase() override; @@ -68,9 +57,6 @@ class DomDistillerRequestViewBase // needs to be kept around to ensure the distillation request finishes. scoped_ptr<ViewerHandle> viewer_handle_; - // Holds the callback to where the data retrieved is sent back. - scoped_ptr<DistillerDataCallback> callback_; - // Number of pages of the distilled article content that have been rendered by // the viewer. int page_count_; diff --git a/components/dom_distiller/core/dom_distiller_request_view_base_unittest.cc b/components/dom_distiller/core/dom_distiller_request_view_base_unittest.cc new file mode 100644 index 0000000..352d38c --- /dev/null +++ b/components/dom_distiller/core/dom_distiller_request_view_base_unittest.cc @@ -0,0 +1,239 @@ +// Copyright 2015 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 <vector> + +#include "components/dom_distiller/core/article_distillation_update.h" +#include "components/dom_distiller/core/test_request_view_handle.h" +#include "components/pref_registry/testing_pref_service_syncable.h" +#include "grit/components_resources.h" +#include "grit/components_strings.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "ui/base/l10n/l10n_util.h" + +using testing::HasSubstr; +using testing::Not; + +namespace dom_distiller { + +class DomDistillerRequestViewTest : public testing::Test { + protected: + void SetUp() override { + pref_service_.reset(new user_prefs::TestingPrefServiceSyncable()); + DistilledPagePrefs::RegisterProfilePrefs(pref_service_->registry()); + distilled_page_prefs_.reset(new DistilledPagePrefs(pref_service_.get())); + } + + scoped_ptr<user_prefs::TestingPrefServiceSyncable> pref_service_; + scoped_ptr<DistilledPagePrefs> distilled_page_prefs_; +}; + +TEST_F(DomDistillerRequestViewTest, TestTitleEscaped) { + const std::string no_title = + l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_NO_DATA_TITLE); + const std::string valid_title = "valid title"; + const std::string has_quotes = "\"" + valid_title + "\""; + const std::string escaped_quotes = "\\\"" + valid_title + "\\\""; + const std::string has_special_chars = "<" + valid_title + "\\"; + const std::string escaped_special_chars = "\\u003C" + valid_title + "\\\\"; + + TestRequestViewHandle handle(distilled_page_prefs_.get()); + + // Make sure title is properly escaped from quotes. + { + scoped_ptr<DistilledArticleProto> article_proto( + new DistilledArticleProto()); + article_proto->set_title(has_quotes); + + handle.OnArticleReady(article_proto.get()); + + EXPECT_THAT(handle.GetJavaScriptBuffer(), HasSubstr(escaped_quotes)); + EXPECT_THAT(handle.GetJavaScriptBuffer(), Not(HasSubstr(has_quotes))); + handle.ClearJavaScriptBuffer(); + } + + // Make sure title is properly escaped from special characters. + { + scoped_ptr<DistilledArticleProto> article_proto( + new DistilledArticleProto()); + article_proto->set_title(has_special_chars); + + handle.OnArticleReady(article_proto.get()); + + EXPECT_THAT(handle.GetJavaScriptBuffer(), HasSubstr(escaped_special_chars)); + EXPECT_THAT(handle.GetJavaScriptBuffer(), + Not(HasSubstr(has_special_chars))); + handle.ClearJavaScriptBuffer(); + } + +} + +TEST_F(DomDistillerRequestViewTest, TestTitleNeverEmpty) { + const std::string no_title = + l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_NO_DATA_TITLE); + const std::string valid_title = "valid title"; + + TestRequestViewHandle handle(distilled_page_prefs_.get()); + + // Test that the title actually gets shown. + { + scoped_ptr<DistilledArticleProto> article_proto( + new DistilledArticleProto()); + article_proto->set_title(valid_title); + + handle.OnArticleReady(article_proto.get()); + + EXPECT_THAT(handle.GetJavaScriptBuffer(), HasSubstr(valid_title)); + EXPECT_THAT(handle.GetJavaScriptBuffer(), Not(HasSubstr(no_title))); + handle.ClearJavaScriptBuffer(); + } + + // Test empty string title + { + scoped_ptr<DistilledArticleProto> article_proto( + new DistilledArticleProto()); + article_proto->set_title(""); + + handle.OnArticleReady(article_proto.get()); + + EXPECT_THAT(handle.GetJavaScriptBuffer(), HasSubstr(no_title)); + EXPECT_THAT(handle.GetJavaScriptBuffer(), Not(HasSubstr(valid_title))); + handle.ClearJavaScriptBuffer(); + } + + // Test no title. + { + scoped_ptr<DistilledArticleProto> article_proto( + new DistilledArticleProto()); + + handle.OnArticleReady(article_proto.get()); + + EXPECT_THAT(handle.GetJavaScriptBuffer(), HasSubstr(no_title)); + EXPECT_THAT(handle.GetJavaScriptBuffer(), Not(HasSubstr(valid_title))); + handle.ClearJavaScriptBuffer(); + } +} + +TEST_F(DomDistillerRequestViewTest, TestContentNeverEmpty) { + const std::string no_content = + l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_NO_DATA_CONTENT); + const std::string valid_content = "valid content"; + + TestRequestViewHandle handle(distilled_page_prefs_.get()); + + // Test single page content + { + scoped_ptr<DistilledArticleProto> article_proto( + new DistilledArticleProto()); + (*(article_proto->add_pages())).set_html(valid_content); + + handle.OnArticleReady(article_proto.get()); + + EXPECT_THAT(handle.GetJavaScriptBuffer(), HasSubstr(valid_content)); + EXPECT_THAT(handle.GetJavaScriptBuffer(), Not(HasSubstr(no_content))); + handle.ClearJavaScriptBuffer(); + } + + // Test multiple page content + { + scoped_ptr<DistilledArticleProto> article_proto( + new DistilledArticleProto()); + (*(article_proto->add_pages())).set_html(valid_content); + (*(article_proto->add_pages())).set_html(valid_content); + + handle.OnArticleReady(article_proto.get()); + + EXPECT_THAT(handle.GetJavaScriptBuffer(), + HasSubstr(valid_content + valid_content)); + EXPECT_THAT(handle.GetJavaScriptBuffer(), Not(HasSubstr(no_content))); + handle.ClearJavaScriptBuffer(); + } + + // Test empty string content + { + scoped_ptr<DistilledArticleProto> article_proto( + new DistilledArticleProto()); + (*(article_proto->add_pages())).set_html(""); + + handle.OnArticleReady(article_proto.get()); + + EXPECT_THAT(handle.GetJavaScriptBuffer(), HasSubstr(no_content)); + EXPECT_THAT(handle.GetJavaScriptBuffer(), Not(HasSubstr(valid_content))); + handle.ClearJavaScriptBuffer(); + } + + // Test page no content + { + scoped_ptr<DistilledArticleProto> article_proto( + new DistilledArticleProto()); + article_proto->add_pages(); + + handle.OnArticleReady(article_proto.get()); + + EXPECT_THAT(handle.GetJavaScriptBuffer(), HasSubstr(no_content)); + EXPECT_THAT(handle.GetJavaScriptBuffer(), Not(HasSubstr(valid_content))); + handle.ClearJavaScriptBuffer(); + } + + // Test no page. + { + scoped_ptr<DistilledArticleProto> article_proto( + new DistilledArticleProto()); + + handle.OnArticleReady(article_proto.get()); + + EXPECT_THAT(handle.GetJavaScriptBuffer(), HasSubstr(no_content)); + EXPECT_THAT(handle.GetJavaScriptBuffer(), Not(HasSubstr(valid_content))); + handle.ClearJavaScriptBuffer(); + } + + // Test empty string page update + { + std::vector<scoped_refptr<ArticleDistillationUpdate::RefCountedPageProto>> + pages; + scoped_refptr<base::RefCountedData<DistilledPageProto>> page_proto = + new base::RefCountedData<DistilledPageProto>(); + page_proto->data.set_html(""); + pages.push_back(page_proto); + + scoped_ptr<ArticleDistillationUpdate> article_update( + new ArticleDistillationUpdate(pages, false, false)); + + handle.OnArticleUpdated(*article_update.get()); + + EXPECT_THAT(handle.GetJavaScriptBuffer(), HasSubstr(no_content)); + EXPECT_THAT(handle.GetJavaScriptBuffer(), Not(HasSubstr(valid_content))); + handle.ClearJavaScriptBuffer(); + } +} + +TEST_F(DomDistillerRequestViewTest, TestLoadingIndicator) { + const std::string no_content = + l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_NO_DATA_CONTENT); + // Showing the indicator does mean passing 'false' as the parameter. + const std::string show_loader = "showLoadingIndicator(false);"; + + TestRequestViewHandle handle(distilled_page_prefs_.get()); + handle.TakeViewerHandle(NULL); + + // The loading indicator should show before any content is displayed. + EXPECT_THAT(handle.GetJavaScriptBuffer(), HasSubstr(show_loader)); + handle.ClearJavaScriptBuffer(); + + std::vector<scoped_refptr<ArticleDistillationUpdate::RefCountedPageProto>> + pages; + scoped_refptr<base::RefCountedData<DistilledPageProto>> page_proto = + new base::RefCountedData<DistilledPageProto>(); + pages.push_back(page_proto); + + scoped_ptr<ArticleDistillationUpdate> article_update( + new ArticleDistillationUpdate(pages, true, false)); + + handle.OnArticleUpdated(*article_update.get()); + + EXPECT_THAT(handle.GetJavaScriptBuffer(), HasSubstr(show_loader)); +} + +} // namespace dom_distiller diff --git a/components/dom_distiller/core/html/dom_distiller_viewer.html b/components/dom_distiller/core/html/dom_distiller_viewer.html index 2aef588..b4c02aa 100644 --- a/components/dom_distiller/core/html/dom_distiller_viewer.html +++ b/components/dom_distiller/core/html/dom_distiller_viewer.html @@ -9,10 +9,11 @@ found in the LICENSE file. <meta charset="utf-8"> <meta name="viewport" content="width=device-width,initial-scale=1,maximum-scale=1,user-scalable=no"> <title>$1</title> + <!-- Placeholder for CSS. --> $2 <link href='https://fonts.googleapis.com/css?family=Open+Sans' rel='stylesheet' type='text/css'> </head> -<body dir="$8" class="$4"> +<body class="$3"> <div id="contentWrap"> <div id="mainContent"> <article> @@ -20,7 +21,7 @@ found in the LICENSE file. <div id="titleCollapse"> <div class="verticalCenterOuter"> <div class="verticalCenterInner"> - <h1 id="titleHolder">$1</h1> + <h1 id="titleHolder"></h1> </div> </div> </div> @@ -28,7 +29,7 @@ found in the LICENSE file. <div id="content"></div> </article> </div> - <div id="loadingIndicator" class="$5"> + <div id="loadingIndicator" class="visible"> <div id="loader"> <div class="circle initial"> <span class="mask"> @@ -82,7 +83,7 @@ found in the LICENSE file. </div> </div> <div id="showOriginal"> - <a href="$6">$7</a> + <a href="$4">$5</a> </div> </div> <div id="feedbackContainer" class="footerFeedback"> @@ -96,5 +97,6 @@ found in the LICENSE file. <div class="clear"></div> </div> </body> -$3 +<!-- Placeholder for JavaScript. --> +$6 </html> diff --git a/components/dom_distiller/core/javascript/dom_distiller_viewer.js b/components/dom_distiller/core/javascript/dom_distiller_viewer.js index 4a80bba..75292a2 100644 --- a/components/dom_distiller/core/javascript/dom_distiller_viewer.js +++ b/components/dom_distiller/core/javascript/dom_distiller_viewer.js @@ -49,12 +49,18 @@ function setTitle(title) { collapse.style.height = "0px"; holder.textContent = title; + document.title = title; var newHeight = Math.max(90, holder.getBoundingClientRect().height); collapse.style.transition = "height 0.2s"; collapse.style.height = newHeight + "px"; } +// Set the text direction of the document ('ltr', 'rtl', or 'auto'). +function setTextDirection(direction) { + document.body.setAttribute('dir', direction); +} + // Maps JS Font Family to CSS class and then changes body class name. // CSS classes must agree with distilledpage.css. function useFontFamily(fontFamily) { diff --git a/components/dom_distiller/core/test_request_view_handle.h b/components/dom_distiller/core/test_request_view_handle.h new file mode 100644 index 0000000..5271e59 --- /dev/null +++ b/components/dom_distiller/core/test_request_view_handle.h @@ -0,0 +1,52 @@ +// Copyright 2015 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_TEST_REQUEST_VIEW_HANDLE_H_ +#define COMPONENTS_DOM_DISTILLER_CORE_TEST_REQUEST_VIEW_HANDLE_H_ + +#include <string> + +#include "components/dom_distiller/core/distilled_page_prefs.h" +#include "components/dom_distiller/core/dom_distiller_request_view_base.h" + +namespace dom_distiller { + +// TestRequestViewHandle allows the javascript buffer to be collected at any +// point and viewed. This class is for testing only. +class TestRequestViewHandle : public DomDistillerRequestViewBase { + public: + TestRequestViewHandle(DistilledPagePrefs* prefs); + ~TestRequestViewHandle() override; + + std::string GetJavaScriptBuffer(); + void ClearJavaScriptBuffer(); + + private: + void SendJavaScript(const std::string& buffer) override; + std::string buffer_; +}; + +TestRequestViewHandle::TestRequestViewHandle(DistilledPagePrefs* prefs) + : DomDistillerRequestViewBase(prefs) { +} + +TestRequestViewHandle::~TestRequestViewHandle() { + distilled_page_prefs_->RemoveObserver(this); +} + +std::string TestRequestViewHandle::GetJavaScriptBuffer() { + return buffer_; +} + +void TestRequestViewHandle::ClearJavaScriptBuffer() { + buffer_ = ""; +} + +void TestRequestViewHandle::SendJavaScript(const std::string& buffer) { + buffer_ += buffer; +} + +} // namespace dom_distiller + +#endif // COMPONENTS_DOM_DISTILLER_CORE_TEST_REQUEST_VIEW_HANDLE_H_ diff --git a/components/dom_distiller/core/viewer.cc b/components/dom_distiller/core/viewer.cc index 0244a4f..bfb10f0 100644 --- a/components/dom_distiller/core/viewer.cc +++ b/components/dom_distiller/core/viewer.cc @@ -104,19 +104,13 @@ void EnsureNonEmptyContent(std::string* content) { } std::string ReplaceHtmlTemplateValues( - const std::string& title, - const std::string& textDirection, - const std::string& loading_indicator_class, const std::string& original_url, const DistilledPagePrefs::Theme theme, const DistilledPagePrefs::FontFamily font_family) { base::StringPiece html_template = ResourceBundle::GetSharedInstance().GetRawDataResource( IDR_DOM_DISTILLER_VIEWER_HTML); - // TODO(mdjones): Many or all of these substitutions can be placed on the - // page via JavaScript. std::vector<std::string> substitutions; - substitutions.push_back(title); // $1 std::ostringstream css; std::ostringstream script; @@ -129,16 +123,19 @@ std::string ReplaceHtmlTemplateValues( css << "<link rel=\"stylesheet\" href=\"/" << kViewerCssPath << "\">"; script << "<script src=\"" << kViewerJsPath << "\"></script>"; #endif // defined(OS_IOS) + + substitutions.push_back( + l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_LOADING_TITLE)); // $1 substitutions.push_back(css.str()); // $2 - substitutions.push_back(script.str()); // $3 substitutions.push_back(GetThemeCssClass(theme) + " " + - GetFontCssClass(font_family)); // $4 - substitutions.push_back(loading_indicator_class); // $5 - substitutions.push_back(original_url); // $6 + GetFontCssClass(font_family)); // $3 + + substitutions.push_back(original_url); // $4 substitutions.push_back( - l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_VIEW_ORIGINAL)); // $7 - substitutions.push_back(textDirection); // $8 + l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_VIEW_ORIGINAL)); // $5 + + substitutions.push_back(script.str()); // $6 return ReplaceStringPlaceholders(html_template, substitutions, NULL); } @@ -190,6 +187,21 @@ const std::string GetErrorPageJs() { return page_update; } +const std::string GetSetTitleJs(std::string title) { + EnsureNonEmptyTitle(&title); + base::StringValue value(title); + std::string output; + base::JSONWriter::Write(value, &output); + return "setTitle(" + output + ");"; +} + +const std::string GetSetTextDirectionJs(const std::string& direction) { + base::StringValue value(direction); + std::string output; + base::JSONWriter::Write(value, &output); + return "setTextDirection(" + output + ");"; +} + const std::string GetToggleLoadingIndicatorJs(const bool is_last_page) { if (is_last_page) return "showLoadingIndicator(true);"; @@ -198,20 +210,10 @@ const std::string GetToggleLoadingIndicatorJs(const bool is_last_page) { } const std::string GetUnsafeArticleTemplateHtml( - const DistilledPageProto* page_proto, + const std::string original_url, const DistilledPagePrefs::Theme theme, const DistilledPagePrefs::FontFamily font_family) { - DCHECK(page_proto); - - std::string title = net::EscapeForHTML(page_proto->title()); - - EnsureNonEmptyTitle(&title); - - std::string text_direction = page_proto->text_direction(); - std::string original_url = page_proto->url(); - - return ReplaceHtmlTemplateValues(title, text_direction, "hidden", - original_url, theme, font_family); + return ReplaceHtmlTemplateValues(original_url, theme, font_family); } const std::string GetUnsafeArticleContentJs( @@ -232,15 +234,6 @@ const std::string GetUnsafeArticleContentJs( return page_update + GetToggleLoadingIndicatorJs(true); } -const std::string GetErrorPageHtml( - const DistilledPagePrefs::Theme theme, - const DistilledPagePrefs::FontFamily font_family) { - std::string title = l10n_util::GetStringUTF8( - IDS_DOM_DISTILLER_VIEWER_FAILED_TO_FIND_ARTICLE_TITLE); - return ReplaceHtmlTemplateValues(title, "auto", "hidden", "", theme, - font_family); -} - const std::string GetCss() { return ResourceBundle::GetSharedInstance().GetRawDataResource( IDR_DISTILLER_CSS).as_string(); diff --git a/components/dom_distiller/core/viewer.h b/components/dom_distiller/core/viewer.h index 7b9261f..61b0e40 100644 --- a/components/dom_distiller/core/viewer.h +++ b/components/dom_distiller/core/viewer.h @@ -33,7 +33,7 @@ const std::string GetShowFeedbackFormJs(); // considered unsafe, so callers must ensure rendering it does not compromise // Chrome. const std::string GetUnsafeArticleTemplateHtml( - const DistilledPageProto* page_proto, + const std::string original_url, const DistilledPagePrefs::Theme theme, const DistilledPagePrefs::FontFamily font_family); @@ -51,6 +51,12 @@ const std::string GetUnsafeIncrementalDistilledPageJs( const DistilledPageProto* page_proto, const bool is_last_page); +// Returns the JavaScript to set the title of the distilled article page. +const std::string GetSetTitleJs(std::string title); + +// Return the JavaScript to set the text direction of the distiller page. +const std::string GetSetTextDirectionJs(const std::string& direction); + // Returns a JavaScript blob for updating a view request with error page // contents. const std::string GetErrorPageJs(); @@ -60,11 +66,6 @@ const std::string GetErrorPageJs(); // the last page of the article (i.e. loading indicator should be removed). const std::string GetToggleLoadingIndicatorJs(const bool is_last_page); -// Returns a full HTML page which displays a generic error. -const std::string GetErrorPageHtml( - const DistilledPagePrefs::Theme theme, - const DistilledPagePrefs::FontFamily font_family); - // Returns the default CSS to be used for a viewer. const std::string GetCss(); diff --git a/components/dom_distiller_strings.grdp b/components/dom_distiller_strings.grdp index c05e9b6..79a4fe5 100644 --- a/components/dom_distiller_strings.grdp +++ b/components/dom_distiller_strings.grdp @@ -28,6 +28,9 @@ <message name="IDS_DOM_DISTILLER_VIEWER_FAILED_TO_FIND_ARTICLE_CONTENT" desc="The text to show in place of reading list article content if the article is not found."> Could not find the requested article. </message> + <message name="IDS_DOM_DISTILLER_VIEWER_LOADING_TITLE" desc="The text to show in place of a reading list article title while the article is loading."> + Reader Mode + </message> <message name="IDS_DOM_DISTILLER_VIEWER_NO_DATA_TITLE" desc="The text to show in place of a reading list article title if there is no data found."> Failed to display article. </message> diff --git a/ios/chrome/browser/dom_distiller/distiller_viewer.cc b/ios/chrome/browser/dom_distiller/distiller_viewer.cc index d078046..1e1aab9 100644 --- a/ios/chrome/browser/dom_distiller/distiller_viewer.cc +++ b/ios/chrome/browser/dom_distiller/distiller_viewer.cc @@ -18,72 +18,45 @@ namespace dom_distiller { -namespace { - -class IOSContentDataCallback : public DistillerDataCallback { - public: - IOSContentDataCallback( - const GURL& url, - const DistillerViewer::DistillationFinishedCallback& callback, - DistillerViewer* distiller_viewer_handle); - ~IOSContentDataCallback() override{}; - void RunCallback(std::string& data) override; - - private: - // Extra param needed by the callback specified below. - GURL url_; - // The callback to be run. - const DistillerViewer::DistillationFinishedCallback callback_; - // A handle to the DistillerViewer object. - DistillerViewer* distiller_viewer_handle_; -}; - -IOSContentDataCallback::IOSContentDataCallback( - const GURL& url, - const DistillerViewer::DistillationFinishedCallback& callback, - DistillerViewer* distiller_viewer_handle) - : url_(url), - callback_(callback), - distiller_viewer_handle_(distiller_viewer_handle) { -} - -void IOSContentDataCallback::RunCallback(std::string& data) { - std::string htmlAndScript(data); - htmlAndScript += "<script>" + - distiller_viewer_handle_->GetJavaScriptBuffer() + - "</script>"; - - callback_.Run(url_, htmlAndScript); -} - -} // namespace - DistillerViewer::DistillerViewer(ios::ChromeBrowserState* browser_state, const GURL& url, const DistillationFinishedCallback& callback) : DomDistillerRequestViewBase( - scoped_ptr<DistillerDataCallback>( - new IOSContentDataCallback(url, callback, this)).Pass(), - new DistilledPagePrefs(browser_state->GetPrefs())) { + new DistilledPagePrefs(browser_state->GetPrefs())), + url_(url), + callback_(callback) { DCHECK(browser_state); DCHECK(url.is_valid()); dom_distiller::DomDistillerService* distillerService = dom_distiller::DomDistillerServiceFactory::GetForBrowserState( browser_state); - viewer_handle_ = distillerService->ViewUrl( + scoped_ptr<ViewerHandle> viewer_handle = distillerService->ViewUrl( this, distillerService->CreateDefaultDistillerPage(gfx::Size()), url); + + TakeViewerHandle(viewer_handle.Pass()); } DistillerViewer::~DistillerViewer() { } -void DistillerViewer::SendJavaScript(const std::string& buffer) { - js_buffer_ += buffer; +void DistillerViewer::OnArticleReady( + const dom_distiller::DistilledArticleProto* article_proto) { + DomDistillerRequestViewBase::OnArticleReady(article_proto); + + const std::string html = viewer::GetUnsafeArticleTemplateHtml( + url_.spec(), distilled_page_prefs_->GetTheme(), + distilled_page_prefs_->GetFontFamily()); + + std::string html_and_script(html); + std::string hide_feedback = + "document.getElementById('feedbackContainer').style.display = 'none';"; + html_and_script += "<script>" + js_buffer_ + hide_feedback + "</script>"; + callback_.Run(url_, html_and_script); } -std::string DistillerViewer::GetJavaScriptBuffer() { - return js_buffer_; +void DistillerViewer::SendJavaScript(const std::string& buffer) { + js_buffer_ += buffer; } } // namespace dom_distiller diff --git a/ios/chrome/browser/dom_distiller/distiller_viewer.h b/ios/chrome/browser/dom_distiller/distiller_viewer.h index 629e745..1cb4f84 100644 --- a/ios/chrome/browser/dom_distiller/distiller_viewer.h +++ b/ios/chrome/browser/dom_distiller/distiller_viewer.h @@ -34,17 +34,18 @@ class DistillerViewer : public DomDistillerRequestViewBase { const DistillationFinishedCallback& callback); ~DistillerViewer() override; - void SendJavaScript(const std::string& buffer) override; + void OnArticleReady( + const dom_distiller::DistilledArticleProto* article_proto) override; - std::string GetJavaScriptBuffer(); + void SendJavaScript(const std::string& buffer) override; private: // The url of the distilled page. const GURL url_; - // Interface for accessing preferences for distilled pages. - scoped_ptr<DistilledPagePrefs> distilled_page_prefs_; // JavaScript buffer. std::string js_buffer_; + // Callback to run once distillation is complete. + const DistillationFinishedCallback callback_; DISALLOW_COPY_AND_ASSIGN(DistillerViewer); }; |