Commit 4b6112cc authored by wychen's avatar wychen Committed by Commit Bot

Add "AllArticles" mode to Reader Mode heuristics

This is a preliminary change to enable the desired mode in the back end.

BUG=736168

Review-Url: https://codereview.chromium.org/2961533002
Cr-Commit-Position: refs/heads/master@{#484987}
parent 5fff83c9
......@@ -339,6 +339,9 @@ const FeatureEntry::Choice kReaderModeHeuristicsChoices[] = {
switches::reader_mode_heuristics::kAlwaysTrue},
{flag_descriptions::kReaderModeHeuristicsAlwaysOff,
switches::kReaderModeHeuristics, switches::reader_mode_heuristics::kNone},
{flag_descriptions::kReaderModeHeuristicsAllArticles,
switches::kReaderModeHeuristics,
switches::reader_mode_heuristics::kAllArticles},
};
const FeatureEntry::Choice kChromeHomeSwipeLogicChoices[] = {
......
......@@ -202,4 +202,26 @@ IN_PROC_BROWSER_TEST_F(DistillablePageUtilsBrowserTestAdaboost,
}
}
using DistillablePageUtilsBrowserTestAllArticles =
DistillablePageUtilsBrowserTestOption<kAllArticles>;
IN_PROC_BROWSER_TEST_F(DistillablePageUtilsBrowserTestAllArticles,
TestDelegate) {
const char* paths[] = {kSimpleArticlePath, kSimpleArticleIFramePath};
for (unsigned i = 0; i < sizeof(paths) / sizeof(paths[0]); ++i) {
testing::InSequence dummy;
EXPECT_CALL(holder_, OnResult(true, false)).Times(1);
EXPECT_CALL(holder_, OnResult(true, true))
.WillOnce(testing::InvokeWithoutArgs(QuitSoon));
NavigateAndWait(paths[i], 0);
}
{
testing::InSequence dummy;
EXPECT_CALL(holder_, OnResult(false, false)).Times(1);
EXPECT_CALL(holder_, OnResult(false, true))
.WillOnce(testing::InvokeWithoutArgs(QuitSoon));
NavigateAndWait(kNonArticlePath, 0);
}
}
} // namespace dom_distiller
......@@ -1939,11 +1939,13 @@ const char kAutofillAccessoryViewDescription[] =
const char kReaderModeHeuristicsName[] = "Reader Mode triggering";
const char kReaderModeHeuristicsDescription[] =
"Determines what pages the Reader Mode button is shown on.";
"Determines what pages the Reader Mode infobar is shown on.";
const char kReaderModeHeuristicsMarkup[] = "With article structured markup";
const char kReaderModeHeuristicsAdaboost[] = "Appears to be an article";
const char kReaderModeHeuristicsAdaboost[] = "Non-mobile-friendly articles";
const char kReaderModeHeuristicsAllArticles[] = "All articles";
const char kReaderModeHeuristicsAlwaysOff[] = "Never";
......
......@@ -1107,6 +1107,7 @@ extern const char kReaderModeHeuristicsName[];
extern const char kReaderModeHeuristicsDescription[];
extern const char kReaderModeHeuristicsMarkup[];
extern const char kReaderModeHeuristicsAdaboost[];
extern const char kReaderModeHeuristicsAllArticles[];
extern const char kReaderModeHeuristicsAlwaysOff[];
extern const char kReaderModeHeuristicsAlwaysOn[];
......
......@@ -41,7 +41,8 @@ enum RejectionBuckets {
// The number of updates can be from 0 to 2. See the tests in
// "distillable_page_utils_browsertest.cc".
// Most heuristics types only require one update after parsing.
// Adaboost is the only one doing the second update, which is after loading.
// Adaboost-based heuristics are the only ones doing the second update,
// which is after loading.
bool NeedToUpdate(bool is_loaded) {
switch (GetDistillerHeuristicsType()) {
case DistillerHeuristicsType::ALWAYS_TRUE:
......@@ -49,6 +50,7 @@ bool NeedToUpdate(bool is_loaded) {
case DistillerHeuristicsType::OG_ARTICLE:
return !is_loaded;
case DistillerHeuristicsType::ADABOOST_MODEL:
case DistillerHeuristicsType::ALL_ARTICLES:
return true;
case DistillerHeuristicsType::NONE:
default:
......@@ -58,7 +60,8 @@ bool NeedToUpdate(bool is_loaded) {
// Returns whether this update is the last one for the page.
bool IsLast(bool is_loaded) {
if (GetDistillerHeuristicsType() == DistillerHeuristicsType::ADABOOST_MODEL)
if (GetDistillerHeuristicsType() == DistillerHeuristicsType::ADABOOST_MODEL ||
GetDistillerHeuristicsType() == DistillerHeuristicsType::ALL_ARTICLES)
return is_loaded;
return true;
......@@ -76,7 +79,8 @@ bool IsBlacklisted(const GURL& url) {
bool IsDistillablePageAdaboost(WebDocument& doc,
const DistillablePageDetector* detector,
const DistillablePageDetector* long_page,
bool is_last) {
bool is_last,
bool exclude_mobile) {
WebDistillabilityFeatures features = doc.DistillabilityFeatures();
GURL parsed_url(doc.Url());
if (!parsed_url.is_valid()) {
......@@ -145,7 +149,7 @@ bool IsDistillablePageAdaboost(WebDocument& doc,
if (blacklisted) {
return false;
}
if (features.is_mobile_friendly) {
if (exclude_mobile && features.is_mobile_friendly) {
return false;
}
return distillable && long_article;
......@@ -158,9 +162,13 @@ bool IsDistillablePage(WebDocument& doc, bool is_last) {
case DistillerHeuristicsType::OG_ARTICLE:
return doc.DistillabilityFeatures().open_graph;
case DistillerHeuristicsType::ADABOOST_MODEL:
return IsDistillablePageAdaboost(doc,
DistillablePageDetector::GetNewModel(),
DistillablePageDetector::GetLongPageModel(), is_last);
return IsDistillablePageAdaboost(
doc, DistillablePageDetector::GetNewModel(),
DistillablePageDetector::GetLongPageModel(), is_last, true);
case DistillerHeuristicsType::ALL_ARTICLES:
return IsDistillablePageAdaboost(
doc, DistillablePageDetector::GetNewModel(),
DistillablePageDetector::GetLongPageModel(), is_last, false);
case DistillerHeuristicsType::NONE:
default:
return false;
......
......@@ -14,6 +14,7 @@ const char kReaderModeFeedback[] = "reader-mode-feedback";
namespace reader_mode_heuristics {
const char kAdaBoost[] = "adaboost";
const char kAllArticles[] = "allarticles";
const char kOGArticle[] = "opengraph";
const char kAlwaysTrue[] = "alwaystrue";
const char kNone[] = "none";
......
......@@ -28,6 +28,7 @@ extern const char kReaderModeFeedback[];
namespace reader_mode_heuristics {
extern const char kAdaBoost[];
extern const char kAllArticles[];
extern const char kOGArticle[];
extern const char kAlwaysTrue[];
extern const char kNone[];
......
......@@ -21,6 +21,9 @@ DistillerHeuristicsType GetDistillerHeuristicsType() {
if (switch_value == switches::reader_mode_heuristics::kAdaBoost) {
return DistillerHeuristicsType::ADABOOST_MODEL;
}
if (switch_value == switches::reader_mode_heuristics::kAllArticles) {
return DistillerHeuristicsType::ALL_ARTICLES;
}
if (switch_value == switches::reader_mode_heuristics::kOGArticle) {
return DistillerHeuristicsType::OG_ARTICLE;
}
......@@ -36,6 +39,10 @@ DistillerHeuristicsType GetDistillerHeuristicsType() {
base::CompareCase::INSENSITIVE_ASCII)) {
return DistillerHeuristicsType::ADABOOST_MODEL;
}
if (base::StartsWith(group_name, "AllArticles",
base::CompareCase::INSENSITIVE_ASCII)) {
return DistillerHeuristicsType::ALL_ARTICLES;
}
if (base::StartsWith(group_name, "OGArticle",
base::CompareCase::INSENSITIVE_ASCII)) {
return DistillerHeuristicsType::OG_ARTICLE;
......
......@@ -6,14 +6,15 @@
#define COMPONENTS_DOM_DISTILLER_CORE_EXPERIMENTS_H_
namespace dom_distiller {
enum class DistillerHeuristicsType {
NONE,
OG_ARTICLE,
ADABOOST_MODEL,
ALWAYS_TRUE,
};
enum class DistillerHeuristicsType {
NONE,
OG_ARTICLE,
ADABOOST_MODEL,
ALL_ARTICLES,
ALWAYS_TRUE,
};
DistillerHeuristicsType GetDistillerHeuristicsType();
DistillerHeuristicsType GetDistillerHeuristicsType();
}
#endif // COMPONENTS_DOM_DISTILLER_CORE_EXPERIMENTS_H_
......@@ -148,6 +148,7 @@ void ReaderModeChecker::CheckIsDistillable() {
CheckIsDistillableOG(receiver);
break;
case dom_distiller::DistillerHeuristicsType::ADABOOST_MODEL:
case dom_distiller::DistillerHeuristicsType::ALL_ARTICLES:
CheckIsDistillableDetector(receiver);
break;
case dom_distiller::DistillerHeuristicsType::ALWAYS_TRUE:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment