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

Implement "Desktop Welcome Pages" feature #23289

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ net/proxy_resolution/ @brave/tor-reviewers
chromium_src/**/extensions/**/*_features.json @brave/sec-team
chromium_src/tools/json_schema_compiler/ @brave/sec-team

# Browser Commands (available via Brave Education content)
browser/ui/webui/browser_command/ @brave/sec-team

# Brave theme
browser/themes @simonhong

Expand Down
15 changes: 15 additions & 0 deletions browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "brave/components/brave_ads/core/public/ad_units/notification_ad/notification_ad_feature.h"
#include "brave/components/brave_ads/core/public/ads_feature.h"
#include "brave/components/brave_component_updater/browser/features.h"
#include "brave/components/brave_education/common/features.h"
#include "brave/components/brave_news/common/features.h"
#include "brave/components/brave_rewards/common/buildflags/buildflags.h"
#include "brave/components/brave_rewards/common/features.h"
Expand Down Expand Up @@ -442,6 +443,19 @@
FEATURE_VALUE_TYPE(kExtensionsManifestV2), \
}))

#if !BUILDFLAG(IS_ANDROID)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this behind a !IS_ANDROID flag?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's not meant to be available on Android. I'm not sure that it can be built on Android in any case since browser_command_handler.cc is not built on Android.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's not really an appropriate use of an os buildflag. If the entire feature is not available on android then it should have its own buildflag with = !is_android. OS buildflags should be reserved for os-specific code and also the feature flag itself is not behind any buildflag so this doesn't really make sense here as-is.

#define BRAVE_EDUCATION_FEATURE_ENTRIES \
EXPAND_FEATURE_ENTRIES({ \
"brave-show-getting-started-page", \
"Show getting started pages", \
"Show a getting started page after completing the Welcome UX.", \
kOsDesktop, \
FEATURE_VALUE_TYPE(brave_education::features::kShowGettingStartedPage), \
})
#else
#define BRAVE_EDUCATION_FEATURE_ENTRIES
#endif

// Keep the last item empty.
#define LAST_BRAVE_FEATURE_ENTRIES_ITEM

Expand Down Expand Up @@ -981,6 +995,7 @@
BRAVE_MIDDLE_CLICK_AUTOSCROLL_FEATURE_ENTRY \
BRAVE_EXTENSIONS_MANIFEST_V2 \
BRAVE_WORKAROUND_NEW_WINDOW_FLASH \
BRAVE_EDUCATION_FEATURE_ENTRIES \
LAST_BRAVE_FEATURE_ENTRIES_ITEM // Keep it as the last item.
namespace flags_ui {
namespace {
Expand Down
4 changes: 4 additions & 0 deletions browser/brave_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ using extensions::ChromeContentBrowserClientExtensionsPart;
#if !BUILDFLAG(IS_ANDROID)
#include "brave/browser/new_tab/new_tab_shows_navigation_throttle.h"
#include "brave/browser/ui/geolocation/brave_geolocation_permission_tab_helper.h"
#include "brave/browser/ui/webui/brave_education/brave_education_ui.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above, do not use os buildflags to guard features that are disabled on some platforms.

#include "brave/browser/ui/webui/brave_news_internals/brave_news_internals_ui.h"
#include "brave/browser/ui/webui/brave_rewards/rewards_page_top_ui.h"
#include "brave/browser/ui/webui/brave_rewards/rewards_panel_ui.h"
Expand Down Expand Up @@ -826,6 +827,9 @@ void BraveContentBrowserClient::RegisterBrowserInterfaceBindersForFrame(
content::RegisterWebUIControllerInterfaceBinder<
commands::mojom::CommandsService, BraveSettingsUI>(map);
}
content::RegisterWebUIControllerInterfaceBinder<
brave_education::mojom::EducationPageHandlerFactory,
brave_education::BraveEducationUI>(map);
#endif

#if BUILDFLAG(ENABLE_AI_CHAT)
Expand Down
11 changes: 11 additions & 0 deletions browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,12 @@ source_set("ui") {
"toolbar/brave_bookmark_sub_menu_model.cc",
"toolbar/brave_bookmark_sub_menu_model.h",
"toolbar/brave_recent_tabs_sub_menu_model.h",
"webui/brave_education/brave_education_ui.cc",
Copy link
Collaborator

Choose a reason for hiding this comment

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

these should ideally go in their own BUILD.gn unless there would be a circular dependency issue? I think there probably is one for brave_education_ui so it could go directly in webui/brave_education_ui and the rest of the code could go in webui/brave_education with a separate BUILD.gn

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I took a look at the code in browser/ui/webui and none of those folders use their own BUILD.gn. Is this a new pattern that we want to establish?

Copy link
Collaborator

Choose a reason for hiding this comment

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

it's not a new pattern, there is just a lot of incorrect code

"webui/brave_education/brave_education_ui.h",
"webui/brave_education/education_page_handler.cc",
"webui/brave_education/education_page_handler.h",
"webui/brave_education/getting_started_helper.cc",
"webui/brave_education/getting_started_helper.h",
"webui/brave_rewards/rewards_page_top_ui.cc",
"webui/brave_rewards/rewards_page_top_ui.h",
"webui/brave_rewards/rewards_panel_handler.cc",
Expand All @@ -268,6 +274,8 @@ source_set("ui") {
"webui/brave_shields/shields_panel_handler.h",
"webui/brave_shields/shields_panel_ui.cc",
"webui/brave_shields/shields_panel_ui.h",
"webui/browser_command/brave_browser_command_handler.cc",
"webui/browser_command/brave_browser_command_handler.h",
"webui/navigation_bar_data_provider.cc",
"webui/navigation_bar_data_provider.h",
"webui/new_tab_page/brave_new_tab_message_handler.cc",
Expand Down Expand Up @@ -331,6 +339,9 @@ source_set("ui") {
"//base",
"//brave/browser/profiles:util",
"//brave/common",
"//brave/components/brave_education/common",
"//brave/components/brave_education/common:mojom",
"//brave/components/brave_education/resources:generated_resources",
"//brave/components/constants",
"//brave/components/misc_metrics",
"//brave/components/sidebar/browser",
Expand Down
106 changes: 106 additions & 0 deletions browser/ui/webui/brave_education/brave_education_ui.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// Copyright (c) 2024 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 https://mozilla.org/MPL/2.0/.

#include "brave/browser/ui/webui/brave_education/brave_education_ui.h"

#include <memory>
#include <string>
#include <utility>
#include <vector>

#include "brave/browser/ui/webui/brave_education/education_page_handler.h"
#include "brave/browser/ui/webui/brave_webui_source.h"
#include "brave/browser/ui/webui/browser_command/brave_browser_command_handler.h"
#include "brave/components/brave_education/common/education_content_urls.h"
#include "brave/components/brave_education/resources/grit/brave_education_generated_map.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/webui/webui_util.h"
#include "chrome/grit/branded_strings.h"
#include "components/grit/brave_components_resources.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_ui.h"
#include "content/public/browser/web_ui_data_source.h"
#include "services/network/public/mojom/content_security_policy.mojom.h"
#include "ui/base/webui/web_ui_util.h"

namespace brave_education {

namespace {

std::vector<browser_command::mojom::Command> GetSupportedCommands(
const GURL& webui_url) {
using browser_command::mojom::Command;

auto content_type = EducationContentTypeFromBrowserURL(webui_url.spec());
if (!content_type) {
return {};
}

switch (*content_type) {
case EducationContentType::kGettingStarted:
return {Command::kOpenRewardsOnboarding, Command::kOpenWalletOnboarding,
Command::kOpenVPNOnboarding, Command::kOpenAIChat};
}
}

} // namespace

BraveEducationUI::BraveEducationUI(content::WebUI* web_ui,
const std::string& host_name)
: ui::MojoWebUIController(web_ui) {
content::WebUIDataSource* source = content::WebUIDataSource::CreateAndAdd(
Profile::FromWebUI(web_ui), host_name);

webui::SetupWebUIDataSource(
source,
base::make_span(kBraveEducationGenerated, kBraveEducationGeneratedSize),
IDR_BRAVE_EDUCATION_HTML);

AddBackgroundColorToSource(source, web_ui->GetWebContents());

static constexpr webui::LocalizedString kStrings[] = {
{"headerText", IDS_WELCOME_HEADER}};
source->AddLocalizedStrings(kStrings);

// Allow embedding of iframe from brave.com.
source->OverrideContentSecurityPolicy(
network::mojom::CSPDirectiveName::ChildSrc,
"child-src chrome://webui-test https://brave.com/;");
}

BraveEducationUI::~BraveEducationUI() = default;

void BraveEducationUI::BindInterface(
mojo::PendingReceiver<EducationPageHandlerFactory> pending_receiver) {
if (page_factory_receiver_.is_bound()) {
page_factory_receiver_.reset();
}
page_factory_receiver_.Bind(std::move(pending_receiver));
}

void BraveEducationUI::BindInterface(
mojo::PendingReceiver<CommandHandlerFactory> pending_receiver) {
if (command_handler_factory_receiver_.is_bound()) {
command_handler_factory_receiver_.reset();
}
command_handler_factory_receiver_.Bind(std::move(pending_receiver));
}

void BraveEducationUI::CreatePageHandler(
mojo::PendingReceiver<mojom::EducationPageHandler> handler) {
page_handler_ = std::make_unique<EducationPageHandler>(std::move(handler));
}

void BraveEducationUI::CreateBrowserCommandHandler(
mojo::PendingReceiver<browser_command::mojom::CommandHandler>
pending_handler) {
command_handler_ = std::make_unique<BraveBrowserCommandHandler>(
std::move(pending_handler), Profile::FromWebUI(web_ui()),
GetSupportedCommands(web_ui()->GetWebContents()->GetVisibleURL()));
}

WEB_UI_CONTROLLER_TYPE_IMPL(BraveEducationUI)

} // namespace brave_education
64 changes: 64 additions & 0 deletions browser/ui/webui/brave_education/brave_education_ui.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// Copyright (c) 2024 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 https://mozilla.org/MPL/2.0/.

#ifndef BRAVE_BROWSER_UI_WEBUI_BRAVE_EDUCATION_BRAVE_EDUCATION_UI_H_
#define BRAVE_BROWSER_UI_WEBUI_BRAVE_EDUCATION_BRAVE_EDUCATION_UI_H_

#include <memory>
#include <string>

#include "base/memory/raw_ptr.h"
#include "brave/components/brave_education/common/education_page.mojom.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "ui/webui/mojo_web_ui_controller.h"
#include "ui/webui/resources/js/browser_command/browser_command.mojom.h"

namespace content {
class WebUI;
}

namespace brave_education {

// The Web UI controller for the Brave product education page, which displays
// production education website content in an iframe.
class BraveEducationUI : public ui::MojoWebUIController,
public mojom::EducationPageHandlerFactory,
public browser_command::mojom::CommandHandlerFactory {
public:
BraveEducationUI(content::WebUI* web_ui, const std::string& host_name);
~BraveEducationUI() override;

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

void BindInterface(
mojo::PendingReceiver<EducationPageHandlerFactory> pending_receiver);

void BindInterface(
mojo::PendingReceiver<CommandHandlerFactory> pending_receiver);

// mojom::EducationPageHandlerFactory:
void CreatePageHandler(
mojo::PendingReceiver<mojom::EducationPageHandler> handler) override;

// browser_command::mojom::CommandHandlerFactory:
void CreateBrowserCommandHandler(
mojo::PendingReceiver<browser_command::mojom::CommandHandler> handler)
override;

private:
mojo::Receiver<EducationPageHandlerFactory> page_factory_receiver_{this};
mojo::Receiver<CommandHandlerFactory> command_handler_factory_receiver_{this};
std::unique_ptr<mojom::EducationPageHandler> page_handler_;
std::unique_ptr<browser_command::mojom::CommandHandler> command_handler_;

WEB_UI_CONTROLLER_TYPE_DECL();
};

} // namespace brave_education

#endif // BRAVE_BROWSER_UI_WEBUI_BRAVE_EDUCATION_BRAVE_EDUCATION_UI_H_
Loading
Loading