Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 8416: Introduce speedreader button. #5085

Merged
merged 14 commits into from
Apr 14, 2020
1 change: 1 addition & 0 deletions app/brave_command_ids.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#define IDC_ADD_NEW_PROFILE 56007
#define IDC_OPEN_GUEST_PROFILE 56008
#define IDC_SHOW_BRAVE_WEBCOMPAT_REPORTER 56009
#define IDC_TOGGLE_SPEEDREADER 56010

#define IDC_BRAVE_COMMANDS_LAST 57000

Expand Down
3 changes: 3 additions & 0 deletions app/vector_icons/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ aggregate_vector_icons("brave_vector_icons") {
"autoplay_status.icon",
"download_unlock.icon",
"sad_folder.icon",
"speedreader.icon",
"speedreader_on_active.icon",
"speedreader_on_inactive.icon",
"tor_profile.icon",
]
}
Expand Down
25 changes: 25 additions & 0 deletions app/vector_icons/speedreader.icon
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
CANVAS_DIMENSIONS, 24,
MOVE_TO, 17.46f, 8.17f,
H_LINE_TO, 5.54f,
R_ARC_TO, 0.54f, 0.54f, 0, 0, 1, 0, -1.08f,
R_H_LINE_TO, 11.92f,
R_ARC_TO, 0.54f, 0.54f, 0, 0, 1, 0, 1.08f,
CLOSE,
MOVE_TO, 5.54f, 10.33f,
R_H_LINE_TO, 7.58f,
R_ARC_TO, 0.54f, 0.54f, 0, 0, 1, 0, 1.08f,
H_LINE_TO, 5.54f,
R_ARC_TO, 0.54f, 0.54f, 0, 0, 1, 0, -1.08f,
CLOSE,
R_MOVE_TO, 0, 3.25f,
R_H_LINE_TO, 11.92f,
R_ARC_TO, 0.54f, 0.54f, 0, 0, 1, 0, 1.08f,
H_LINE_TO, 5.54f,
R_ARC_TO, 0.54f, 0.54f, 0, 0, 1, 0, -1.08f,
CLOSE,
R_MOVE_TO, 0, 3.25f,
R_H_LINE_TO, 7.58f,
R_ARC_TO, 0.54f, 0.54f, 0, 0, 1, 0, 1.08f,
H_LINE_TO, 5.54f,
R_ARC_TO, 0.54f, 0.54f, 0, 0, 1, 0, -1.08f,
CLOSE
46 changes: 46 additions & 0 deletions app/vector_icons/speedreader_on_active.icon
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
CANVAS_DIMENSIONS, 24,
PATH_COLOR_ARGB, 0xFF, 0xFF, 0xFF, 0xFF,
MOVE_TO, 17.85f, 4,
R_H_LINE_TO, -11.7f,
CUBIC_TO, 5.29f, 4, 4.6f, 4.71f, 4.6f, 5.58f,
R_V_LINE_TO, 12.5f,
R_CUBIC_TO, 0, 0.87f, 0.69f, 1.59f, 1.55f, 1.59f,
R_H_LINE_TO, 11.7f,
R_CUBIC_TO, 0.85f, 0, 1.55f, -0.71f, 1.55f, -1.58f,
R_V_LINE_TO, -12.5f,
R_CUBIC_TO, 0, -0.87f, -0.69f, -1.58f, -1.55f, -1.58f,
CLOSE,
NEW_PATH,
PATH_COLOR_ARGB, 0xFF, 0xFB, 0x54, 0x2B,
MOVE_TO, 15.94f, 8.58f,
H_LINE_TO, 7.64f,
R_ARC_TO, 0.74f, 0.74f, 0, 0, 1, 0, -1.47f,
R_H_LINE_TO, 8.31f,
R_ARC_TO, 0.74f, 0.74f, 0, 1, 1, 0, 1.47f,
MOVE_TO, 6.9f, 10.57f,
R_CUBIC_TO, 0, -0.41f, 0.41f, -0.74f, 0.91f, -0.74f,
R_H_LINE_TO, 6.14f,
R_CUBIC_TO, 0.51f, 0, 0.91f, 0.33f, 0.91f, 0.74f,
R_CUBIC_TO, 0, 0.41f, -0.41f, 0.74f, -0.91f, 0.74f,
H_LINE_TO, 7.81f,
R_CUBIC_TO, -0.5f, 0, -0.91f, -0.33f, -0.91f, -0.74f,
R_MOVE_TO, 8.93f, 3.54f,
H_LINE_TO, 7.64f,
R_ARC_TO, 0.74f, 0.74f, 0, 1, 1, 0, -1.47f,
R_H_LINE_TO, 8.2f,
R_ARC_TO, 0.74f, 0.74f, 0, 0, 1, 0, 1.47f,
R_MOVE_TO, -2.57f, 2.8f,
H_LINE_TO, 7.64f,
R_ARC_TO, 0.74f, 0.74f, 0, 0, 1, 0, -1.47f,
R_H_LINE_TO, 5.63f,
R_ARC_TO, 0.74f, 0.74f, 0, 1, 1, 0, 1.47f,
MOVE_TO, 17.85f, 4,
R_H_LINE_TO, -11.7f,
CUBIC_TO, 5.29f, 4, 4.6f, 4.71f, 4.6f, 5.58f,
R_V_LINE_TO, 12.5f,
R_CUBIC_TO, 0, 0.87f, 0.69f, 1.59f, 1.55f, 1.59f,
R_H_LINE_TO, 11.7f,
R_CUBIC_TO, 0.85f, 0, 1.55f, -0.71f, 1.55f, -1.58f,
R_V_LINE_TO, -12.5f,
R_CUBIC_TO, 0, -0.87f, -0.69f, -1.58f, -1.55f, -1.58f,
CLOSE
26 changes: 26 additions & 0 deletions app/vector_icons/speedreader_on_inactive.icon
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
CANVAS_DIMENSIONS, 24,
PATH_COLOR_ARGB, 0xFF, 0xFB, 0x54, 0x2B,
MOVE_TO, 17.46f, 7.17f,
H_LINE_TO, 5.54f,
R_ARC_TO, 0.54f, 0.54f, 0, 0, 1, 0, -1.08f,
R_H_LINE_TO, 11.92f,
R_ARC_TO, 0.54f, 0.54f, 0, 0, 1, 0, 1.08f,
CLOSE,
MOVE_TO, 5.54f, 9.33f,
R_H_LINE_TO, 7.58f,
R_ARC_TO, 0.54f, 0.54f, 0, 0, 1, 0, 1.08f,
H_LINE_TO, 5.54f,
R_ARC_TO, 0.54f, 0.54f, 0, 0, 1, 0, -1.08f,
CLOSE,
R_MOVE_TO, 0, 3.25f,
R_H_LINE_TO, 11.92f,
R_ARC_TO, 0.54f, 0.54f, 0, 0, 1, 0, 1.08f,
H_LINE_TO, 5.54f,
R_ARC_TO, 0.54f, 0.54f, 0, 0, 1, 0, -1.08f,
CLOSE,
R_MOVE_TO, 0, 3.25f,
R_H_LINE_TO, 7.58f,
R_ARC_TO, 0.54f, 0.54f, 0, 0, 1, 0, 1.08f,
H_LINE_TO, 5.54f,
R_ARC_TO, 0.54f, 0.54f, 0, 0, 1, 0, -1.08f,
CLOSE
7 changes: 7 additions & 0 deletions browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ source_set("browser_process") {
"//brave/components/p3a",
"//brave/components/resources",
"//brave/components/services:brave_content_manifest_overlays",
"//brave/components/speedreader:buildflags",
"//brave/services/network/public/cpp",
"//chrome/common",
"//components/autofill/core/common",
Expand Down Expand Up @@ -313,6 +314,12 @@ source_set("browser_process") {
}

if (enable_speedreader) {
sources += [
"//brave/browser/speedreader/speedreader_service_factory.cc",
"//brave/browser/speedreader/speedreader_service_factory.h",
"//brave/browser/speedreader/speedreader_tab_helper.cc",
"//brave/browser/speedreader/speedreader_tab_helper.h",
]
deps += [ "//brave/components/speedreader" ]
}

Expand Down
30 changes: 13 additions & 17 deletions browser/brave_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
#include "extensions/buildflags/buildflags.h"
#include "net/cookies/site_for_cookies.h"
#include "services/service_manager/public/cpp/manifest_builder.h"
#include "third_party/blink/public/common/loader/url_loader_throttle.h"
#include "ui/base/l10n/l10n_util.h"

using brave_shields::BraveShieldsWebContentsObserver;
Expand Down Expand Up @@ -97,9 +98,8 @@ using extensions::ChromeContentBrowserClientExtensionsPart;
#endif

#if BUILDFLAG(ENABLE_SPEEDREADER)
#include "brave/components/speedreader/speedreader_switches.h"
#include "brave/browser/speedreader/speedreader_tab_helper.h"
#include "brave/components/speedreader/speedreader_throttle.h"
#include "brave/components/speedreader/speedreader_whitelist.h"
#include "content/public/common/resource_type.h"
#endif

Expand Down Expand Up @@ -283,21 +283,17 @@ BraveContentBrowserClient::CreateURLLoaderThrottles(
request, browser_context, wc_getter, navigation_ui_data,
frame_tree_node_id);
#if BUILDFLAG(ENABLE_SPEEDREADER)
const auto* cmd_line = base::CommandLine::ForCurrentProcess();
if (cmd_line->HasSwitch(speedreader::kEnableSpeedreader)) {
// Work only with casual main frame navigations.
if (request.url.SchemeIsHTTPOrHTTPS() &&
request.resource_type ==
static_cast<int>(content::ResourceType::kMainFrame)) {
// Note that we check the whitelist before any redirects, while distilling
// will be performed on a final document (the last in the redirect chain).
auto* whitelist = g_brave_browser_process->speedreader_whitelist();
if (speedreader::IsWhitelisted(request.url) ||
whitelist->IsWhitelisted(request.url)) {
result.push_back(std::make_unique<speedreader::SpeedReaderThrottle>(
base::ThreadTaskRunnerHandle::Get()));
}
}
content::WebContents* contents = wc_getter.Run();
if (!contents) {
return result;
}
auto* tab_helper =
speedreader::SpeedreaderTabHelper::FromWebContents(contents);
if (tab_helper && tab_helper->IsActiveForMainFrame()
&& request.resource_type
== static_cast<int>(content::ResourceType::kMainFrame)) {
result.push_back(std::make_unique<speedreader::SpeedReaderThrottle>(
base::ThreadTaskRunnerHandle::Get()));
}
#endif // ENABLE_SPEEDREADER
return result;
Expand Down
9 changes: 9 additions & 0 deletions browser/brave_profile_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "brave/components/brave_wayback_machine/buildflags.h"
#include "brave/components/brave_webtorrent/browser/buildflags/buildflags.h"
#include "brave/components/ntp_background_images/browser/ntp_background_images_utils.h"
#include "brave/components/speedreader/buildflags.h"
#include "chrome/browser/net/prediction_options.h"
#include "chrome/browser/prefs/session_startup_pref.h"
#include "chrome/common/pref_names.h"
Expand Down Expand Up @@ -56,6 +57,10 @@
#include "brave/browser/gcm_driver/brave_gcm_utils.h"
#endif

#if BUILDFLAG(ENABLE_SPEEDREADER)
#include "brave/components/speedreader/speedreader_service.h"
#endif

using extensions::FeatureSwitch;

namespace brave {
Expand Down Expand Up @@ -218,6 +223,10 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
password_manager::prefs::kPasswordLeakDetectionEnabled,
base::Value(false));

#if BUILDFLAG(ENABLE_SPEEDREADER)
speedreader::SpeedreaderService::RegisterPrefs(registry);
#endif

RegisterProfilePrefsForMigration(registry);
}

Expand Down
9 changes: 9 additions & 0 deletions browser/brave_tab_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "brave/components/brave_shields/browser/buildflags/buildflags.h" // For STP
#include "brave/components/brave_wayback_machine/buildflags.h"
#include "brave/components/greaselion/browser/buildflags/buildflags.h"
#include "brave/components/speedreader/buildflags.h"
#include "content/public/browser/web_contents.h"
#include "third_party/widevine/cdm/buildflags.h"

Expand Down Expand Up @@ -46,6 +47,10 @@
#include "brave/components/brave_perf_predictor/browser/perf_predictor_tab_helper.h"
#endif

#if BUILDFLAG(ENABLE_SPEEDREADER)
#include "brave/browser/speedreader/speedreader_tab_helper.h"
#endif

namespace brave {

void AttachTabHelpers(content::WebContents* web_contents) {
Expand Down Expand Up @@ -88,6 +93,10 @@ void AttachTabHelpers(content::WebContents* web_contents) {
#endif

brave_ads::AdsTabHelper::CreateForWebContents(web_contents);

#if BUILDFLAG(ENABLE_SPEEDREADER)
speedreader::SpeedreaderTabHelper::CreateForWebContents(web_contents);
#endif
}

} // namespace brave
9 changes: 9 additions & 0 deletions browser/speedreader/speedreader_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@

#include "base/bind.h"
#include "base/path_service.h"
#include "brave/app/brave_command_ids.h"
#include "brave/common/brave_paths.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/test/browser_test_utils.h"
Expand Down Expand Up @@ -47,6 +49,7 @@ class SpeedReaderBrowserTest : public InProcessBrowserTest {
};

IN_PROC_BROWSER_TEST_F(SpeedReaderBrowserTest, SmokeTest) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more than a smoke test would be good

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll add more test when a component updater stuff is in place

chrome::ExecuteCommand(browser(), IDC_TOGGLE_SPEEDREADER);
const GURL url = https_server_.GetURL(kTestPage);
ui_test_utils::NavigateToURL(browser(), url);
content::WebContents* contents =
Expand All @@ -62,4 +65,10 @@ IN_PROC_BROWSER_TEST_F(SpeedReaderBrowserTest, SmokeTest) {
// style is injected.
EXPECT_LT(0ull, content::EvalJs(rfh, kGetStyle).ExtractString().size());
EXPECT_GT(4096ull, content::EvalJs(rfh, kGetContent).ExtractString().size());

// Check that disabled speedreader doesn't affect the page.
chrome::ExecuteCommand(browser(), IDC_TOGGLE_SPEEDREADER);
ui_test_utils::NavigateToURL(browser(), url);
EXPECT_LT(106000ull,
content::EvalJs(rfh, kGetContent).ExtractString().size());
}
42 changes: 42 additions & 0 deletions browser/speedreader/speedreader_service_factory.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/* Copyright (c) 2020 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/browser/speedreader/speedreader_service_factory.h"

#include "brave/components/speedreader/speedreader_service.h"
#include "chrome/browser/profiles/profile.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"

namespace speedreader {

// static
SpeedreaderServiceFactory* SpeedreaderServiceFactory::GetInstance() {
return base::Singleton<SpeedreaderServiceFactory>::get();
}

SpeedreaderService* SpeedreaderServiceFactory::GetForProfile(Profile* profile) {
return static_cast<SpeedreaderService*>(
SpeedreaderServiceFactory::GetInstance()->GetServiceForBrowserContext(
profile, true /*create*/));
}

SpeedreaderServiceFactory::SpeedreaderServiceFactory()
: BrowserContextKeyedServiceFactory(
"SpeedreaderService",
BrowserContextDependencyManager::GetInstance()) {}

SpeedreaderServiceFactory::~SpeedreaderServiceFactory() {}

KeyedService* SpeedreaderServiceFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const {
return new SpeedreaderService(
Profile::FromBrowserContext(context)->GetPrefs());
}

bool SpeedreaderServiceFactory::ServiceIsCreatedWithBrowserContext() const {
return true;
}

} // namespace speedreader
41 changes: 41 additions & 0 deletions browser/speedreader/speedreader_service_factory.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/* Copyright (c) 2020 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_BROWSER_SPEEDREADER_SPEEDREADER_SERVICE_FACTORY_H_
#define BRAVE_BROWSER_SPEEDREADER_SPEEDREADER_SERVICE_FACTORY_H_

#include "base/memory/singleton.h"
#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
#include "components/keyed_service/core/keyed_service.h"

class Profile;

namespace speedreader {

class SpeedreaderService;

class SpeedreaderServiceFactory : public BrowserContextKeyedServiceFactory {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this service & factory aren't necessary.
If this is only used for get/set related prefs value, I think just using simple apis seems fine. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree, though I decided to keep it in case we would add a user preference (and would need to keep a pref registrar), or something like this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, ok. if service will have registrar in the future, having service seems reasonable.

public:
static SpeedreaderServiceFactory* GetInstance();
static SpeedreaderService* GetForProfile(Profile* profile);

private:
friend struct base::DefaultSingletonTraits<SpeedreaderServiceFactory>;
SpeedreaderServiceFactory();
~SpeedreaderServiceFactory() override;

SpeedreaderServiceFactory(const SpeedreaderServiceFactory&) = delete;
SpeedreaderServiceFactory& operator=(const SpeedreaderServiceFactory&) =
delete;

// BrowserContextKeyedServiceFactory overrides:
KeyedService* BuildServiceInstanceFor(
content::BrowserContext* context) const override;
bool ServiceIsCreatedWithBrowserContext() const override;
};

} // namespace speedreader

#endif // BRAVE_BROWSER_SPEEDREADER_SPEEDREADER_SERVICE_FACTORY_H_
Loading