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

Moved from tabs to core option to control opening pages in custom tabs #5199

Merged
merged 3 commits into from
Apr 10, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 2 additions & 0 deletions android/brave_java_sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ brave_java_sources = [
"../../brave/android/java/org/chromium/chrome/browser/BraveFeatureList.java",
"../../brave/android/java/org/chromium/chrome/browser/BraveHelper.java",
"../../brave/android/java/org/chromium/chrome/browser/BraveIntentHandler.java",
"../../brave/android/java/org/chromium/chrome/browser/BraveLaunchIntentDispatcher.java",
"../../brave/android/java/org/chromium/chrome/browser/BraveRelaunchUtils.java",
"../../brave/android/java/org/chromium/chrome/browser/BraveSyncWorker.java",
"../../brave/android/java/org/chromium/chrome/browser/BraveRewardsNativeWorker.java",
Expand Down Expand Up @@ -78,6 +79,7 @@ brave_java_sources = [
"../../brave/android/java/org/chromium/chrome/browser/settings/AppearancePreferences.java",
"../../brave/android/java/org/chromium/chrome/browser/settings/BackgroundImagesPreferences.java",
"../../brave/android/java/org/chromium/chrome/browser/settings/BackgroundVideoPlaybackPreference.java",
"../../brave/android/java/org/chromium/chrome/browser/settings/BraveCustomTabsPreference.java",
"../../brave/android/java/org/chromium/chrome/browser/settings/BraveLicensePreferences.java",
"../../brave/android/java/org/chromium/chrome/browser/settings/BraveMainPreferencesBase.java",
"../../brave/android/java/org/chromium/chrome/browser/settings/BravePreferenceFragment.java",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/* 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/. */

package org.chromium.chrome.browser;

import android.content.Intent;
import android.content.SharedPreferences;

import org.chromium.base.ContextUtils;
import org.chromium.chrome.browser.preferences.BravePreferenceKeys;

public class BraveLaunchIntentDispatcher {
Copy link
Collaborator

Choose a reason for hiding this comment

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

in other classes I left a comment // see BraveLaunchIntentDispatcherClassAdapter so it's easier to track down how this works

public static boolean isCustomTabIntent(Intent intent) {
if (!useCustomTabs()) {
return false;
}
return LaunchIntentDispatcher.isCustomTabIntent(intent);
}

public static boolean useCustomTabs() {
SharedPreferences sharedPreferences = ContextUtils.getAppSharedPreferences();
return sharedPreferences.getBoolean(BravePreferenceKeys.BRAVE_USE_CUSTOM_TABS, true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@
public final class BravePreferenceKeys {
public static final String BRAVE_BOTTOM_TOOLBAR_ENABLED_KEY = "brave_bottom_toolbar_enabled_key";
public static final String BRAVE_BOTTOM_TOOLBAR_SET_KEY = "brave_bottom_toolbar_enabled";
public static final String BRAVE_USE_CUSTOM_TABS = "use_custom_tabs";
}
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/. */

package org.chromium.chrome.browser.settings;

import android.os.Bundle;
import android.content.SharedPreferences;
import android.support.v7.preference.Preference;

import org.chromium.base.ContextUtils;
import org.chromium.chrome.browser.BraveLaunchIntentDispatcher;
import org.chromium.chrome.browser.preferences.BravePreferenceKeys;
import org.chromium.chrome.browser.settings.BravePreferenceFragment;
import org.chromium.chrome.R;

public class BraveCustomTabsPreference extends BravePreferenceFragment
implements Preference.OnPreferenceChangeListener {
public static int getPreferenceSummary() {
return BraveLaunchIntentDispatcher.useCustomTabs() ? R.string.text_on : R.string.text_off;
}

@Override
public void onCreatePreferences(Bundle savedInstanceState, String rootKey) {
getActivity().setTitle(R.string.prefs_use_custom_tabs);
SettingsUtils.addPreferencesFromResource(this, R.xml.use_custom_tabs_brave_preference);

ChromeSwitchPreference pref = (ChromeSwitchPreference) findPreference(
BravePreferenceKeys.BRAVE_USE_CUSTOM_TABS);
pref.setChecked(BraveLaunchIntentDispatcher.useCustomTabs());
pref.setOnPreferenceChangeListener(this);
}

@Override
public boolean onPreferenceChange(Preference preference, Object newValue) {
SharedPreferences.Editor sharedPreferencesEditor = ContextUtils.getAppSharedPreferences().edit();
sharedPreferencesEditor.putBoolean(BravePreferenceKeys.BRAVE_USE_CUSTOM_TABS, (boolean) newValue);
sharedPreferencesEditor.apply();
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public class BraveMainPreferencesBase extends BravePreferenceFragment {
private static final String PREF_BACKGROUND_IMAGES = "backgroud_images";
private static final String PREF_BRAVE_REWARDS = "brave_rewards";
private static final String PREF_HOMEPAGE = "homepage";
private static final String PREF_USE_CUSTOM_TABS = "use_custom_tabs";

private final HashMap<String, Preference> mRemovedPreferences = new HashMap<>();

Expand Down Expand Up @@ -122,6 +123,7 @@ private void rearrangePreferenceOrders() {
// and we deleted original 0 ~ 2 ordered preferences.
// Advanced section will be located below our controls section.
int order = findPreference(PREF_CLOSING_ALL_TABS_CLOSES_BRAVE).getOrder();
findPreference(PREF_USE_CUSTOM_TABS).setOrder(++order);
findPreference(PREF_ADVANCED_SECTION).setOrder(++order);
findPreference(PREF_PRIVACY).setOrder(++order);
findPreference(PREF_BRAVE_REWARDS).setOrder(++order);
Expand Down Expand Up @@ -168,6 +170,8 @@ private void updateControlSectionPreferences() {
p.setSummary(BackgroundVideoPlaybackPreference.getPreferenceSummary());
p = findPreference(PREF_CLOSING_ALL_TABS_CLOSES_BRAVE);
p.setSummary(ClosingAllTabsClosesBravePreference.getPreferenceSummary());
p = findPreference(PREF_USE_CUSTOM_TABS);
p.setSummary(BraveCustomTabsPreference.getPreferenceSummary());
}

private void overrideChromiumPreferences() {
Expand Down
9 changes: 7 additions & 2 deletions android/java/res/xml/brave_main_preferences.xml
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,19 @@
android:key="closing_all_tabs_closes_brave"
android:order="17"
android:title="@string/prefs_closing_all_tabs_closes_brave"/>
<org.chromium.chrome.browser.settings.ChromeBasePreference
android:fragment="org.chromium.chrome.browser.settings.BraveCustomTabsPreference"
android:key="use_custom_tabs"
android:order="18"
android:title="@string/prefs_use_custom_tabs"/>
<org.chromium.chrome.browser.settings.ChromeBasePreference
android:fragment="org.chromium.chrome.browser.settings.BraveRewardsPreferences"
android:key="brave_rewards"
android:order="18"
android:order="19"
android:title="@string/brave_ui_brave_rewards"/>
<org.chromium.chrome.browser.settings.ChromeBasePreference
android:fragment="org.chromium.chrome.browser.settings.BraveSyncScreensPreference"
android:key="brave_sync_layout"
android:order="19"
android:order="20"
android:title="@string/sync_category_title"/>
</android.support.v7.preference.PreferenceScreen>
14 changes: 14 additions & 0 deletions android/java/res/xml/use_custom_tabs_brave_preference.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- 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/. -->

<android.support.v7.preference.PreferenceScreen xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto">
<org.chromium.chrome.browser.settings.ChromeSwitchPreference
android:key="use_custom_tabs"
android:title="@string/prefs_use_custom_tabs"
android:summaryOn="@string/text_on"
android:summaryOff="@string/text_off" />
</android.support.v7.preference.PreferenceScreen>
3 changes: 3 additions & 0 deletions browser/ui/android/strings/android_brave_strings.grd
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,9 @@ until they verify, or until 90 days have passed.
<message name="IDS_TURN_ON_BRAVE_ADS_TO_CLAIM" desc="Text description for NTP banner.">
Get paid to see this background image. Turn on Brave Ads to claim your share.
</message>
<message name="IDS_PREFS_USE_CUSTOM_TABS" desc="Title for closing all tabs closes brave in controls section">
Open tabs in Custom Tabs
</message>
</messages>
</release>
</grit>
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public static ClassVisitor createAdapter(ClassVisitor chain) {
chain = new BraveBookmarkModelClassAdapter(chain);
chain = new BraveMainPreferenceBaseClassAdapter(chain);
chain = new BraveAndroidSyncSettingsClassAdapter(chain);
chain = new BraveLaunchIntentDispatcherClassAdapter(chain);
return chain;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ public void visitMethodInsn(int opcode,
// the method now
opcode = INVOKEVIRTUAL;
}
String newOwner = shouldChangeOwner(owner, name);
if (!newOwner.isEmpty()) {
System.out.println("changing owner for " + mName + "." + name +
" - new owner " + newOwner);
owner = newOwner;
}
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
}
}
Expand All @@ -87,6 +93,8 @@ public void visitMethodInsn(int opcode,
new HashMap<String, ArrayList<String>>();
private Map<String, ArrayList<String>> mMakePublicMethods =
new HashMap<String, ArrayList<String>>();
private Map<String, Map<String, String>> mChangeOwnerMethods =
new HashMap<String, Map<String, String>>();
private Map<String, ArrayList<String>> mMakeProtectedFields =
new HashMap<String, ArrayList<String>>();
private Map<String, Map<String, ArrayList<String>>> mAddAnnotations =
Expand Down Expand Up @@ -147,6 +155,28 @@ protected void makePublicMethod(String className, String methodName) {
methods.add(methodName);
}

private String shouldChangeOwner(String owner, String methodName) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

since the semantics of this are slightly different than the making methods public, maybe call it maybeChangeOwner and return the original owner instead of an empty string if it doesn't change? Then you don't need this https://github.com/brave/brave-core/pull/5199/files#diff-125eb2dc484493790df1e4b6cca9fd3bR77 and you can add the println inside shouldChangeOwner. Just a suggestion though

if (mChangeOwnerMethods.containsKey(owner)) {
Map<String, String> methods = mChangeOwnerMethods.get(owner);
if (methods.containsKey(methodName)) {
String newOwner = methods.get(methodName);
if (!newOwner.equals(mName)) {
return newOwner;
}
}
}
return "";
}

protected void changeMethodOwner(String currentOwner, String methodName, String newOwner) {
Map methods = mChangeOwnerMethods.get(currentOwner);
if (methods == null) {
methods = new HashMap<String, String>();
mChangeOwnerMethods.put(currentOwner, methods);
}
methods.put(methodName, newOwner);
}

private boolean shouldDeleteField(String fieldName) {
for(Map.Entry<String, ArrayList<String>> entry :
mDeleteFields.entrySet()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/* 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/. */

package org.brave.bytecode;

import org.objectweb.asm.ClassVisitor;

public class BraveLaunchIntentDispatcherClassAdapter extends BraveClassVisitor {
static String sLaunchIntentDispatcherClassName = "org/chromium/chrome/browser/LaunchIntentDispatcher";
static String sBraveLaunchIntentDispatcherClassName = "org/chromium/chrome/browser/BraveLaunchIntentDispatcher";

public BraveLaunchIntentDispatcherClassAdapter(ClassVisitor visitor) {
super(visitor);
changeMethodOwner(sLaunchIntentDispatcherClassName, "isCustomTabIntent", sBraveLaunchIntentDispatcherClassName);
}
}
1 change: 1 addition & 0 deletions build/android/bytecode/java_sources.gni
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
brave_java_bytecode_files = [
"../../../brave/build/android/bytecode/java/org/brave/bytecode/BraveClassAdapter.java",
"../../../brave/build/android/bytecode/java/org/brave/bytecode/BraveClassVisitor.java",
"../../../brave/build/android/bytecode/java/org/brave/bytecode/BraveLaunchIntentDispatcherClassAdapter.java",
"../../../brave/build/android/bytecode/java/org/brave/bytecode/BraveAndroidSyncSettingsClassAdapter.java",
"../../../brave/build/android/bytecode/java/org/brave/bytecode/BraveBookmarkModelClassAdapter.java",
"../../../brave/build/android/bytecode/java/org/brave/bytecode/BraveMainPreferenceBaseClassAdapter.java",
Expand Down