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.

Sign up for GitHub

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

Jump to bottom

Fix #11444: Google accounts should not show in the profile menu #6516

Merged
merged 1 commit into from
Aug 31, 2020
Merged

Fix #11444: Google accounts should not show in the profile menu #6516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion app/brave_generated_resources.grd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ By installing this extension, you are agreeing to the Google Widevine Terms of U
Show the number of blocked items on the Shields icon
</message>
<message name="IDS_SETTINGS_BRAVE_SHIELDS_GOOGLE_LOGIN_LABEL" desc="Label for a switch control which allows Google social buttons to be enabled/disabled">
Allow Google login for extensions and third party sites
Allow Google login buttons on third party sites
</message>
<message name="IDS_SETTINGS_BRAVE_SHIELDS_FACEBOOK_EMBEDDED_POSTS_LABEL" desc="Label for a switch control which allows Facebook embedded posts">
Allow Facebook logins and embedded posts
Expand Down Expand Up @@ -653,6 +653,9 @@ By installing this extension, you are agreeing to the Google Widevine Terms of U
<message name="IDS_SETTINGS_LOAD_CRYPTO_WALLETS_ON_STARTUP" desc="The description for always loading the Crypto Wallets extension on startup">
Load Crypto Wallets on startup
</message>
<message name="IDS_SETTINGS_GOOGLE_LOGIN_FOR_EXTENSIONS" desc="Label for a switch control which allows Google login for extension">
Allow Google login for extensions
</message>
<message name="IDS_SETTINGS_HANGOUTS_ENABLED_DESC" desc="The description for Hangouts switch in settings">
Uses Hangouts component to enable screen sharing and other features in the browser.
</message>
Expand Down
2 changes: 1 addition & 1 deletion browser/brave_profile_prefs_browsertest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ IN_PROC_BROWSER_TEST_F(BraveProfilePrefsBrowserTest,
EXPECT_EQ(browser()->profile()->GetPrefs()->GetInteger(
prefs::kNetworkPredictionOptions),
chrome_browser_net::NETWORK_PREDICTION_NEVER);
EXPECT_TRUE(browser()->profile()->GetPrefs()->GetBoolean(
EXPECT_FALSE(browser()->profile()->GetPrefs()->GetBoolean(
prefs::kSigninAllowedOnNextStartup));
// Verify cloud print is disabled.
EXPECT_FALSE(browser()->profile()->GetPrefs()->GetBoolean(
Expand Down
4 changes: 4 additions & 0 deletions ...esources/settings/brave_default_extensions_page/brave_default_extensions_browser_proxy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ cr.define('settings', function() {
isTorManaged() {}
getRestartNeeded() {}
getWeb3ProviderList() {}
wasSignInEnabledAtStartup() {}
}

/**
Expand Down Expand Up @@ -59,6 +60,9 @@ cr.define('settings', function() {
getWeb3ProviderList() {
return new Promise(resolve => chrome.braveWallet.getWeb3ProviderList(resolve))
}
wasSignInEnabledAtStartup() {
Copy link
Member

Choose a reason for hiding this comment

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

nit (see related): doesn't need to be in proxy since not sending any data

return loadTimeData.getBoolean('signInAllowedOnNextStartupInitialValue');
}
}

cr.addSingletonGetter(BraveDefaultExtensionsBrowserProxyImpl);
Expand Down
12 changes: 12 additions & 0 deletions browser/resources/settings/brave_default_extensions_page/brave_default_extensions_page.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,18 @@
pref="{{prefs.brave.wallet.load_on_startup}}"
label="$i18n{loadCryptoWalletsOnStartupDesc}">
</settings-toggle-button>
<settings-toggle-button id="googleLoginForExtensions"
class="cr-row"
pref="{{prefs.signin.allowed_on_next_startup}}"
learn-more-url="$i18n{googleLoginLearnMoreURL}"
label="$i18n{googleLoginForExtensionsDesc}">
<template is="dom-if" if="[[shouldShowRestartForGoogleLogin_(
prefs.signin.allowed_on_next_startup.value)]]">
<cr-button on-click="restartBrowser_" slot="more-actions">
$i18n{restart}
</cr-button>
</template>
</settings-toggle-button>
<settings-toggle-button id="hangoutsEnabled"
class="cr-row"
pref="{{prefs.brave.hangouts_enabled}}"
Expand Down
8 changes: 7 additions & 1 deletion browser/resources/settings/brave_default_extensions_page/brave_default_extensions_page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ Polymer({
this.browserProxy_.setIPFSCompanionEnabled(this.$.ipfsCompanionEnabled.checked);
},

restartBrowser_: function() {
restartBrowser_: function(e) {
e.stopPropagation();
window.open("chrome://restart", "_self");
},

Expand All @@ -101,5 +102,10 @@ Polymer({
openWebStoreUrl_: function() {
window.open(loadTimeData.getString('getMoreExtensionsUrl'));
},

shouldShowRestartForGoogleLogin_: function(value) {
return this.browserProxy_.wasSignInEnabledAtStartup() != value;
Copy link
Member

Choose a reason for hiding this comment

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

nit: this doesn't need to be either a function (since it's static) or something from browserProxy (since this component can access loadTimeData just fine). Can simply be a polymer property initialized with the value of loadTimeData.getBoolean('signInAllowedOnNextStartupInitialValue'). Still, just a nit and only mentioning to reduce code or for the future.

},

});
})();
6 changes: 0 additions & 6 deletions browser/resources/settings/settings_resources_generated.grdp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,6 @@
preprocess="true"
compress="false"
type="BINDATA" />
<include name="IDR_SETTINGS_SOCIAL_BLOCKING_PAGE_PROXY_M_JS"
file="${root_gen_dir}/brave/browser/resources/settings/social_blocking_page/social_blocking_page_proxy.m.js"
use_base_dir="false"
preprocess="true"
compress="false"
type="BINDATA" />
<include name="IDR_SETTINGS_SOCIAL_BLOCKING_PAGE_M_JS"
file="${root_gen_dir}/brave/browser/resources/settings/social_blocking_page/social_blocking_page.m.js"
use_base_dir="false"
Expand Down
12 changes: 1 addition & 11 deletions browser/resources/settings/social_blocking_page/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,13 @@ import("../settings.gni")
group("web_modules") {
public_deps = [
":social_blocking_page_module",
":modules",
]
}

polymer_modulizer("social_blocking_page") {
js_file = "social_blocking_page.js"
html_file = "social_blocking_page.html"
html_type = "dom-module"
auto_imports = settings_auto_imports + [
"brave/browser/resources/settings/social_blocking_page/social_blocking_page_proxy.html|SocialBlockingPageProxyImpl",
]
namespace_rewrites = settings_namespace_rewrites
}

js_modulizer("modules") {
input_files = [
"social_blocking_page_proxy.js",
]
auto_imports = settings_auto_imports
namespace_rewrites = settings_namespace_rewrites
}
1 change: 0 additions & 1 deletion browser/resources/settings/social_blocking_page/social_blocking_page.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

<link rel="import" href="chrome://resources/html/i18n_behavior.html">
<link rel="import" href="chrome://resources/cr_elements/md_select_css.html">
<link rel="import" href="social_blocking_page_proxy.html">
<link rel="import" href="../settings_shared_css.html">
<link rel="import" href="../settings_vars_css.html">

Expand Down
31 changes: 1 addition & 30 deletions browser/resources/settings/social_blocking_page/social_blocking_page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,34 +7,5 @@
* social blocking options
*/
Polymer({
is: 'settings-social-blocking-page',

properties: {},

/** @private {?settings.SocialBlockingPageProxy} */
browserProxy_: null,

/** @override */
created: function() {
this.browserProxy_ = settings.SocialBlockingPageProxyImpl.getInstance();
},

/** @override */
ready: function() {
this.shouldShowRestart_ = this.shouldShowRestart_.bind(this)
this.restartBrowser_ = this.restartBrowser_.bind(this)
},

shouldShowRestart_: function(value) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jumde this function and setSignInEnabledAtNextStartup_ below have been removed from this .js but they are still referenced in the HTML. Were they meant to stay and just have their functionality moved here from the proxy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

checking

return this.browserProxy_.wasSignInEnabledAtStartup() != value;
},

setSignInEnabledAtNextStartup_: function() {
this.browserProxy_.setSignInEnabledAtNextStartup(this.$.googleLoginControlType.checked);
},

restartBrowser_: function(e) {
e.stopPropagation();
window.open("chrome://restart", "_self");
},
is: 'settings-social-blocking-page'
});
2 changes: 0 additions & 2 deletions browser/resources/settings/social_blocking_page/social_blocking_page_proxy.html
View file
Open in desktop

This file was deleted.

43 changes: 0 additions & 43 deletions browser/resources/settings/social_blocking_page/social_blocking_page_proxy.js
View file
Open in desktop

This file was deleted.

22 changes: 22 additions & 0 deletions browser/ui/views/profiles/brave_profile_menu_view.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,16 @@
#include <utility>

#include "brave/browser/profiles/profile_util.h"
#include "chrome/app/vector_icons/vector_icons.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_attributes_entry.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/profiles/profile_window.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/grit/generated_resources.h"
#include "components/vector_icons/vector_icons.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

are there existing deps for this and ui/base?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bridiver - here: https://github.com/brave/brave-core/blob/master/browser/ui/BUILD.gn#L220

#include "ui/base/l10n/l10n_util.h"

void BraveProfileMenuView::BuildIdentity() {
ProfileMenuView::BuildIdentity();
Expand All @@ -33,6 +39,22 @@ void BraveProfileMenuView::BuildAutofillButtons() {}
// We don't want to show any Chromium sync info.
void BraveProfileMenuView::BuildSyncInfo() {}

// We don't want feature buttons to manage google account
void BraveProfileMenuView::BuildFeatureButtons() {
Profile* profile = browser()->profile();
int window_count = chrome::GetBrowserCount(profile);
if (!profile->IsOffTheRecord() && profile->HasPrimaryOTRProfile())
window_count += chrome::GetBrowserCount(profile->GetPrimaryOTRProfile());
if (window_count > 1) {
AddFeatureButton(
l10n_util::GetPluralStringFUTF16(IDS_PROFILES_CLOSE_X_WINDOWS_BUTTON,
window_count),
base::BindRepeating(&ProfileMenuView::OnExitProfileButtonClicked,
base::Unretained(this)),
vector_icons::kCloseIcon);
}
}

gfx::ImageSkia BraveProfileMenuView::GetSyncIcon() const {
// We don't need sync overlay.
return gfx::ImageSkia();
Expand Down
1 change: 1 addition & 0 deletions browser/ui/views/profiles/brave_profile_menu_view.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class BraveProfileMenuView : public ProfileMenuView {
void BuildIdentity() override;
void BuildAutofillButtons() override;
void BuildSyncInfo() override;
void BuildFeatureButtons() override;
gfx::ImageSkia GetSyncIcon() const override;

DISALLOW_COPY_AND_ASSIGN(BraveProfileMenuView);
Expand Down
15 changes: 15 additions & 0 deletions chromium_src/chrome/browser/profiles/gaia_info_update_service.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/* 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 "chrome/browser/profiles/gaia_info_update_service.h"

#define ShouldUseGAIAProfileInfo ShouldUseGAIAProfileInfo_ChromiumImpl
#include "../../../../../chrome/browser/profiles/gaia_info_update_service.cc"
#undef ShouldUseGAIAProfileInfo

// static
bool GAIAInfoUpdateService::ShouldUseGAIAProfileInfo(Profile* profile) {
return false;
}
15 changes: 15 additions & 0 deletions chromium_src/chrome/browser/profiles/gaia_info_update_service.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/* 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_CHROMIUM_SRC_CHROME_BROWSER_PROFILES_GAIA_INFO_UPDATE_SERVICE_H_
#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_PROFILES_GAIA_INFO_UPDATE_SERVICE_H_

#define ShouldUseGAIAProfileInfo \
ShouldUseGAIAProfileInfo_ChromiumImpl(Profile* profile); \
static bool ShouldUseGAIAProfileInfo
#include "../../../../../chrome/browser/profiles/gaia_info_update_service.h"
#undef ShouldUseGAIAProfileInfo

#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_PROFILES_GAIA_INFO_UPDATE_SERVICE_H_
3 changes: 3 additions & 0 deletions chromium_src/chrome/browser/profiles/pref_service_builder_utils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ void RegisterProfilePrefs(bool is_signin_profile,
// Disable spell check service
registry->SetDefaultPrefValue(
spellcheck::prefs::kSpellCheckUseSpellingService, base::Value(false));

registry->SetDefaultPrefValue(prefs::kSigninAllowedOnNextStartup,
base::Value(false));
#if defined(OS_LINUX)
// Use brave theme by default instead of gtk theme.
registry->SetDefaultPrefValue(prefs::kUsesSystemTheme, base::Value(false));
Expand Down
1 change: 1 addition & 0 deletions chromium_src/chrome/browser/profiles/profiles_state.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "brave/browser/profiles/brave_profile_manager.h"
#include "brave/browser/profiles/profile_util.h"
#include "ui/gfx/text_elider.h"

#define GetAvatarNameForProfile GetAvatarNameForProfile_ChromiumImpl
#include "../../../../../chrome/browser/profiles/profiles_state.cc"
#undef GetAvatarNameForProfile
Expand Down
2 changes: 2 additions & 0 deletions chromium_src/chrome/browser/ui/views/profiles/avatar_toolbar_button.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_PROFILES_AVATAR_TOOLBAR_BUTTON_H_

#define GetAvatarIcon virtual GetAvatarIcon
#define AvatarToolbarButtonDelegate BraveAvatarToolbarButtonDelegate
#include "../../../../../../../chrome/browser/ui/views/profiles/avatar_toolbar_button.h"
#undef AvatarToolbarButtonDelegate
#undef GetAvatarIcon

#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_PROFILES_AVATAR_TOOLBAR_BUTTON_H_
Expand Down
3 changes: 3 additions & 0 deletions chromium_src/chrome/browser/ui/views/profiles/avatar_toolbar_button_delegate.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,6 @@ AvatarToolbarButton::State BraveAvatarToolbarButtonDelegate::GetState() const {
return state;
}

gfx::Image BraveAvatarToolbarButtonDelegate::GetGaiaAccountImage() const {
return gfx::Image();
}
1 change: 1 addition & 0 deletions chromium_src/chrome/browser/ui/views/profiles/avatar_toolbar_button_delegate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class BraveAvatarToolbarButtonDelegate : public AvatarToolbarButtonDelegate {
~BraveAvatarToolbarButtonDelegate() override = default;

void Init(AvatarToolbarButton* button, Profile* profile) override;
gfx::Image GetGaiaAccountImage() const;
AvatarToolbarButton::State GetState() const override;

private:
Expand Down
2 changes: 2 additions & 0 deletions chromium_src/chrome/browser/ui/views/profiles/profile_menu_view.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
#define BuildAutofillButtons virtual BuildAutofillButtons
#define BuildIdentity virtual BuildIdentity
#define BuildSyncInfo virtual BuildSyncInfo
#define BuildFeatureButtons virtual BuildFeatureButtons

#include "../../../../../../../chrome/browser/ui/views/profiles/profile_menu_view.h"
#undef BuildFeatureButtons
#undef BuildSyncInfo
#undef BuildIdentity
#undef BuildAutofillButtons
Expand Down
4 changes: 3 additions & 1 deletion chromium_src/chrome/browser/ui/webui/settings/settings_localized_strings_provider.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const char kBraveReleaseTagPrefix[] =
"https://github.com/brave/brave-browser/releases/tag/v";
const char kGoogleLoginLearnMoreURL[] =
"https://github.com/brave/brave-browser/wiki/"
"Social-Blocking:-Allow-Google-login";
"Allow-Google-login---Third-Parties-and-Extensions";

void BraveAddCommonStrings(content::WebUIDataSource* html_source,
Profile* profile) {
Expand Down Expand Up @@ -267,6 +267,8 @@ void BraveAddCommonStrings(content::WebUIDataSource* html_source,
IDS_SETTINGS_BRAVE_WEB3_PROVIDER_DESC},
{"loadCryptoWalletsOnStartupDesc",
IDS_SETTINGS_LOAD_CRYPTO_WALLETS_ON_STARTUP},
{"googleLoginForExtensionsDesc",
IDS_SETTINGS_GOOGLE_LOGIN_FOR_EXTENSIONS},
{"hangoutsEnabledDesc",
IDS_SETTINGS_HANGOUTS_ENABLED_DESC},
{"ipfsCompanionEnabledDesc",
Expand Down