Skip to content

Commit

Permalink
review changes
Browse files Browse the repository at this point in the history
Signed-off-by: Vadym Struts <[email protected]>
  • Loading branch information
vadimstruts committed Dec 16, 2024
1 parent a0d1ed1 commit 5e79e18
Show file tree
Hide file tree
Showing 21 changed files with 34 additions and 135 deletions.
2 changes: 1 addition & 1 deletion android/java/org/chromium/base/BraveFeatureList.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@ public abstract class BraveFeatureList {
public static final String BRAVE_DAY_ZERO_EXPERIMENT = "BraveDayZeroExperiment";
public static final String BRAVE_FALLBACK_DOH_PROVIDER = "BraveFallbackDoHProvider";
public static final String BRAVE_BLOCK_ALL_COOKIES_TOGGLE = "BlockAllCookiesToggle";
public static final String BRAVE_BLOCK_ELEMENTS_FEATURE = "BlockElementFeature";
public static final String BRAVE_SHIELDS_ELEMENT_PICKER = "BraveShieldsElementPicker";
}
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@
import org.chromium.chrome.browser.multiwindow.BraveMultiWindowUtils;
import org.chromium.chrome.browser.multiwindow.MultiInstanceManager;
import org.chromium.chrome.browser.multiwindow.MultiWindowUtils;
import org.chromium.chrome.browser.night_mode.GlobalNightModeStateProviderHolder;
import org.chromium.chrome.browser.notifications.permissions.NotificationPermissionController;
import org.chromium.chrome.browser.notifications.retention.RetentionNotificationUtil;
import org.chromium.chrome.browser.ntp.NewTabPageManager;
Expand Down Expand Up @@ -1624,10 +1623,6 @@ public void openBraveContentFilteringSettings() {
settingsLauncher.startSettings(this, ContentFilteringFragment.class);
}

public boolean isNightModeEnabled() {
return GlobalNightModeStateProviderHolder.getInstance().isInNightMode();
}

public int getBraveThemeBackgroundColor() {
return ContextUtils.getApplicationContext()
.getColor(R.color.toolbar_background_color_for_ntp);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,6 @@ public static int getThemeBackgroundColor() {
return backgroundColor;
}

@CalledByNative
public static boolean isNightModeEnabled() {
boolean isDarkModeEnabled = false;
try {
isDarkModeEnabled = BraveActivity.getBraveActivity().isNightModeEnabled();
} catch (BraveActivity.BraveActivityNotFoundException e) {
Log.e(TAG, "Get nightly mode status" + e);
}
return isDarkModeEnabled;
}

@NativeMethods
interface Natives {
boolean launchContentPickerForWebContent(Tab tab);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,7 @@ private void setUpSwitchLayouts() {
TextView blockElementsText =
mSecondaryLayout.findViewById(R.id.brave_shields_block_element_text);
blockElementsText.setVisibility(
ChromeFeatureList.isEnabled(BraveFeatureList.BRAVE_BLOCK_ELEMENTS_FEATURE)
ChromeFeatureList.isEnabled(BraveFeatureList.BRAVE_SHIELDS_ELEMENT_PICKER)
? View.VISIBLE
: View.GONE);
blockElementsText.setOnClickListener(
Expand Down
3 changes: 2 additions & 1 deletion browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,8 @@
"Allows to block sekected HTML element on the page" \
"marked as a known threat.", \
kOsAll, \
FEATURE_VALUE_TYPE(brave_shields::features::kBlockElementFeature), \
FEATURE_VALUE_TYPE( \
brave_shields::features::kBraveShieldsElementPicker), \
}, \
{ \
"brave-super-referral", \
Expand Down
5 changes: 0 additions & 5 deletions browser/android/cosmetic_filters/cosmetic_filters_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,4 @@ int32_t GetThemeBackgroundColor() {
return Java_BraveCosmeticFiltersUtils_getThemeBackgroundColor(env);
}

bool IsDarkModeEnabled() {
JNIEnv* env = base::android::AttachCurrentThread();
return Java_BraveCosmeticFiltersUtils_isNightModeEnabled(env);
}

} // namespace cosmetic_filters
1 change: 0 additions & 1 deletion browser/android/cosmetic_filters/cosmetic_filters_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ namespace cosmetic_filters {

void ShowCustomFilterSettings();
int GetThemeBackgroundColor();
bool IsDarkModeEnabled();

} // namespace cosmetic_filters

Expand Down
16 changes: 4 additions & 12 deletions browser/cosmetic_filters/cosmetic_filters_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "ui/color/color_provider.h"
#else
#include "brave/browser/android/cosmetic_filters/cosmetic_filters_utils.h"
#include "chrome/browser/flags/android/chrome_session_state.h"
#endif // !BUILDFLAG(IS_ANDROID)

namespace cosmetic_filters {
Expand Down Expand Up @@ -105,21 +106,12 @@ void CosmeticFiltersTabHelper::GetElementPickerThemeInfo(
GetWebContents().GetColorMode() == ui::ColorProviderKey::ColorMode::kDark,
color_provider.GetColor(kColorSidePanelBadgeBackground));
#else // !BUILDFLAG(IS_ANDROID)
std::move(callback).Run(IsDarkModeEnabled(), GetThemeBackgroundColor());
std::move(callback).Run(chrome::android::GetDarkModeState() ==
chrome::android::DarkModeState::kDarkModeSystem,
GetThemeBackgroundColor());
#endif // !BUILDFLAG(IS_ANDROID)
}

void CosmeticFiltersTabHelper::InitElementPicker(
InitElementPickerCallback callback) {
std::move(callback).Run(
#if !BUILDFLAG(IS_ANDROID)
mojom::RunningPlatform::kDesktop
#else // !BUILDFLAG(IS_ANDROID)
mojom::RunningPlatform::kAndroid
#endif // !BUILDFLAG(IS_ANDROID)
);
}

CosmeticFiltersTabHelper::CosmeticFiltersTabHelper(
content::WebContents* web_contents)
: content::WebContentsUserData<CosmeticFiltersTabHelper>(*web_contents),
Expand Down
1 change: 0 additions & 1 deletion browser/cosmetic_filters/cosmetic_filters_tab_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ class CosmeticFiltersTabHelper
void ManageCustomFilters() override;
void GetElementPickerThemeInfo(
GetElementPickerThemeInfoCallback callback) override;
void InitElementPicker(InitElementPickerCallback callback) override;

friend class content::WebContentsUserData<CosmeticFiltersTabHelper>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
&brave_shields::features::kBraveShowStrictFingerprintingMode, \
&brave_shields::features::kBraveLocalhostAccessPermission, \
&brave_shields::features::kBlockAllCookiesToggle, \
&brave_shields::features::kBlockElementFeature
&brave_shields::features::kBraveShieldsElementPicker

// clang-format on

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ void BraveRenderViewContextMenu::AppendDeveloperItems() {
const auto page_url = source_web_contents_->GetLastCommittedURL();
add_block_elements &= page_url.SchemeIsHTTPOrHTTPS();
add_block_elements &= base::FeatureList::IsEnabled(
brave_shields::features::kBlockElementFeature);
brave_shields::features::kBraveShieldsElementPicker);
if (add_block_elements) {
std::optional<size_t> inspect_index =
menu_model_.GetIndexOfCommandId(IDC_CONTENT_CONTEXT_INSPECTELEMENT);
Expand Down
5 changes: 1 addition & 4 deletions components/brave_shields/core/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,7 @@ static_library("common") {
sources += [ "brave_shield_localized_strings.h" ]
}

public_deps = [
"//brave/components/brave_shields/core/common/buildflags",
"//brave/components/resources:strings",
]
public_deps = [ "//brave/components/resources:strings" ]

deps = [
"//base",
Expand Down
12 changes: 0 additions & 12 deletions components/brave_shields/core/common/buildflags/BUILD.gn

This file was deleted.

12 changes: 0 additions & 12 deletions components/brave_shields/core/common/buildflags/buildflags.gni

This file was deleted.

11 changes: 5 additions & 6 deletions components/brave_shields/core/common/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include "brave/components/brave_shields/core/common/features.h"

#include "base/feature_list.h"
#include "brave/components/brave_shields/core/common/buildflags/buildflags.h"

namespace brave_shields::features {

Expand Down Expand Up @@ -135,12 +134,12 @@ BASE_FEATURE(kBlockAllCookiesToggle,
"BlockAllCookiesToggle",
base::FEATURE_DISABLED_BY_DEFAULT);
// when enabled, allow to select and block HTML elements
BASE_FEATURE(kBlockElementFeature,
"BlockElementFeature",
#if BUILDFLAG(ENABLE_BLOCK_ELEMENT_FEATURE)
base::FEATURE_ENABLED_BY_DEFAULT);
#else
BASE_FEATURE(kBraveShieldsElementPicker,
"BraveShieldsElementPicker",
#if BUILDFLAG(IS_ANDROID)
base::FEATURE_DISABLED_BY_DEFAULT);
#else
base::FEATURE_ENABLED_BY_DEFAULT);
#endif

// Enables extra TRACE_EVENTs in content filter js. The feature is
Expand Down
2 changes: 1 addition & 1 deletion components/brave_shields/core/common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ BASE_DECLARE_FEATURE(kCosmeticFilteringJsPerformance);
BASE_DECLARE_FEATURE(kCosmeticFilteringSyncLoad);
BASE_DECLARE_FEATURE(kBlockAllCookiesToggle);
BASE_DECLARE_FEATURE(kCosmeticFilteringCustomScriptlets);
BASE_DECLARE_FEATURE(kBlockElementFeature);
BASE_DECLARE_FEATURE(kBraveShieldsElementPicker);
extern const base::FeatureParam<int> kComponentUpdateCheckIntervalMins;
extern const base::FeatureParam<std::string>
kCosmeticFilteringSubFrameFirstSelectorsPollingDelayMs;
Expand Down
7 changes: 0 additions & 7 deletions components/cosmetic_filters/common/cosmetic_filters.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ interface CosmeticFiltersResources {
mojo_base.mojom.Value result);
};

enum RunningPlatform {
kDesktop,
kAndroid
};

interface CosmeticFiltersHandler {
// Adds a user cosmetic rule for the current site.
// (currently from the content picker feature).
Expand All @@ -32,8 +27,6 @@ interface CosmeticFiltersHandler {

GetElementPickerThemeInfo() => (
bool is_nightly_mode_enabled, int32 background_color);

InitElementPicker() => (RunningPlatform platform);
};

// An interface to render frame agent `CosmeticFiltersJSHandler` to control
Expand Down
51 changes: 11 additions & 40 deletions components/cosmetic_filters/renderer/cosmetic_filters_js_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "third_party/blink/public/web/web_local_frame.h"
#include "third_party/blink/public/web/web_script_source.h"
#include "ui/base/resource/resource_bundle.h"
#include "v8/include/v8-primitive.h"
#include "v8/include/v8.h"

namespace {
Expand Down Expand Up @@ -362,45 +363,15 @@ void CosmeticFiltersJSHandler::OnGetCosmeticFilterThemeInfo(
std::ignore = resolver->Resolve(context, result);
}

v8::Local<v8::Promise> CosmeticFiltersJSHandler::InitElementPicker(
v8::Local<v8::Value> CosmeticFiltersJSHandler::GetPlatform(
v8::Isolate* isolate) {
v8::MaybeLocal<v8::Promise::Resolver> resolver =
v8::Promise::Resolver::New(isolate->GetCurrentContext());

if (!resolver.IsEmpty()) {
auto promise_resolver =
std::make_unique<v8::Global<v8::Promise::Resolver>>();
promise_resolver->Reset(isolate, resolver.ToLocalChecked());
auto context_old = std::make_unique<v8::Global<v8::Context>>(
isolate, isolate->GetCurrentContext());

GetElementPickerRemoteHandler()->InitElementPicker(base::BindOnce(
&CosmeticFiltersJSHandler::OnInitElementPicker,
weak_ptr_factory_.GetWeakPtr(), std::move(promise_resolver), isolate,
std::move(context_old)));

return resolver.ToLocalChecked()->GetPromise();
}

return v8::Local<v8::Promise>();
}

void CosmeticFiltersJSHandler::OnInitElementPicker(
std::unique_ptr<v8::Global<v8::Promise::Resolver>> promise_resolver,
v8::Isolate* isolate,
std::unique_ptr<v8::Global<v8::Context>> context_old,
mojom::RunningPlatform platform) {
v8::HandleScope handle_scope(isolate);
v8::Local<v8::Context> context = context_old->Get(isolate);
v8::Context::Scope context_scope(context);
v8::MicrotasksScope microtasks(isolate, context->GetMicrotaskQueue(),
v8::MicrotasksScope::kDoNotRunMicrotasks);

v8::Local<v8::Promise::Resolver> resolver = promise_resolver->Get(isolate);
v8::Local<v8::Integer> result;
result = v8::Integer::New(isolate, static_cast<int>(platform));

std::ignore = resolver->Resolve(context, result);
return gin::StringToV8(isolate,
#if BUILDFLAG(IS_ANDROID)
"android"
#else // !BUILDFLAG(IS_ANDROID)
"desktop"
#endif // !BUILDFLAG(IS_ANDROID)
);
}

void CosmeticFiltersJSHandler::AddJavaScriptObjectToFrame(
Expand Down Expand Up @@ -482,8 +453,8 @@ void CosmeticFiltersJSHandler::BindFunctionsToObject(
base::Unretained(this)));

BindFunctionToObject(
isolate, javascript_object, "initElementPicker",
base::BindRepeating(&CosmeticFiltersJSHandler::InitElementPicker,
isolate, javascript_object, "getPlatform",
base::BindRepeating(&CosmeticFiltersJSHandler::GetPlatform,
base::Unretained(this), isolate));

if (perf_tracker_) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,7 @@ class CosmeticFiltersJSHandler : public mojom::CosmeticFiltersAgent {
bool OnIsFirstParty(const std::string& url_string);
void OnAddSiteCosmeticFilter(const std::string& selector);
void OnManageCustomFilters();
v8::Local<v8::Promise> InitElementPicker(v8::Isolate* isolate);
void OnInitElementPicker(
std::unique_ptr<v8::Global<v8::Promise::Resolver>> promise_resolver,
v8::Isolate* isolate,
std::unique_ptr<v8::Global<v8::Context>> context_old,
mojom::RunningPlatform platform);
v8::Local<v8::Value> GetPlatform(v8::Isolate* isolate);
v8::Local<v8::Promise> GetCosmeticFilterThemeInfo(v8::Isolate* isolate);
void OnGetCosmeticFilterThemeInfo(
std::unique_ptr<v8::Global<v8::Promise::Resolver>> promise_resolver,
Expand Down
10 changes: 4 additions & 6 deletions components/cosmetic_filters/resources/data/element_picker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ const api = {
callback(val.isDarkModeEnabled, val.bgcolor)
})
},
initElementPicker: (callback: (platform: number) => void) => {
cf_worker.initElementPicker().then(callback)
getPlatform: ():string => {
return cf_worker.getPlatform()
}
}

Expand Down Expand Up @@ -846,8 +846,6 @@ const highlightElements = () => {

const active = document.getElementById('brave-element-picker')
if (!active) {
api.initElementPicker((platform: number) => {
isAndroid = platform === 1
launchElementPicker(attachElementPicker())
})
isAndroid = api.getPlatform() === 'android'
launchElementPicker(attachElementPicker())
}
2 changes: 1 addition & 1 deletion components/definitions/chromel.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ declare namespace cf_worker {
const manageCustomFilters: () => void
const getElementPickerThemeInfo: () =>
Promise<{isDarkModeEnabled: boolean; bgcolor: number}>
const initElementPicker: () => Promise<number>
const getPlatform: () => string
}

declare namespace chrome.test {
Expand Down

0 comments on commit 5e79e18

Please sign in to comment.