Merge pull request #2903 from brave/76.0.3809.62-followup

Follow up on comments for c76 update
brave · Jul 18, 2019 · de551ba · de551ba
2 parents ac49f09 + a0212eb
commit de551ba
Show file tree

Hide file tree

Showing 46 changed files with 295 additions and 220 deletions.
diff --git a/browser/extensions/brave_extension_service.cc b/browser/extensions/brave_extension_service.cc
@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/one_shot_event.h"
+#include "brave/browser/profiles/profile_util.h"
 #include "brave/common/extensions/extension_constants.h"
 #include "chrome/browser/extensions/api/content_settings/content_settings_service.h"
 #include "chrome/browser/extensions/extension_action.h"
@@ -42,7 +43,7 @@ void BraveExtensionService::AddComponentExtension(const Extension* extension) {
   // the extension when BraveActionViewController is queried about the
   // visibility of the action.
   if ((extension->id() == brave_rewards_extension_id) &&
-      (profile_->IsGuestSession() || profile_->IsTorProfile())) {
+      (profile_->IsGuestSession() || brave::IsTorProfile(profile_))) {
     extensions::ExtensionActionManager* extension_action_manager =
         ExtensionActionManager::Get(profile_);
     ExtensionAction* action =

diff --git a/browser/geolocation/brave_geolocation_permission_context.cc b/browser/geolocation/brave_geolocation_permission_context.cc
@@ -7,7 +7,7 @@
 
 #include <utility>
 
-#include "chrome/browser/profiles/profile.h"
+#include "brave/browser/profiles/profile_util.h"
 #include "components/content_settings/core/common/content_settings.h"
 #include "components/prefs/pref_service.h"
 #include "brave/common/tor/pref_names.h"
@@ -26,7 +26,7 @@ void BraveGeolocationPermissionContext::DecidePermission(
     const GURL& embedding_origin,
     bool user_gesture,
     BrowserPermissionCallback callback) {
-  if (profile()->IsTorProfile()) {
+  if (brave::IsTorProfile(profile())) {
     std::move(callback).Run(ContentSetting::CONTENT_SETTING_BLOCK);
     return;
   }

diff --git a/browser/profiles/BUILD.gn b/browser/profiles/BUILD.gn
@@ -2,6 +2,8 @@ source_set("profiles") {
   sources = [
     "brave_profile_manager.cc",
     "brave_profile_manager.h",
+    "profile_util.cc",
+    "profile_util.h",
   ]
   deps = [
     "//base",

diff --git a/browser/profiles/profile_util.cc b/browser/profiles/profile_util.cc
@@ -0,0 +1,30 @@
+/* Copyright (c) 2019 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/profiles/profile_util.h"
+
+#include "brave/browser/tor/buildflags.h"
+#include "brave/common/tor/pref_names.h"
+#include "chrome/browser/profiles/profile.h"
+#include "components/prefs/pref_service.h"
+
+namespace brave {
+
+bool IsTorProfile(const Profile* profile) {
+#if BUILDFLAG(ENABLE_TOR)
+  DCHECK(profile);
+  return profile->GetPrefs()->GetBoolean(tor::prefs::kProfileUsingTor);
+#else
+  return false;
+#endif
+}
+
+bool IsGuestProfile(Profile* profile) {
+  DCHECK(profile);
+  return profile->GetOriginalProfile()->IsGuestSession() &&
+         !IsTorProfile(profile);
+}
+
+}  // namespace brave
diff --git a/browser/profiles/profile_util.h b/browser/profiles/profile_util.h
@@ -0,0 +1,26 @@
+/* Copyright (c) 2019 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_PROFILES_PROFILE_UTIL_H_
+#define BRAVE_BROWSER_PROFILES_PROFILE_UTIL_H_
+
+class Profile;
+
+namespace brave {
+
+bool IsTorProfile(const Profile* profile);
+
+// This function is similar to Profile::IsGuestSession. It is used to get
+// around the bug(?) in OffTheRecordProfileImpl::Init() code that calls
+// set_is_guest_profile on off-the-record profile only after calling
+// BrowserContextDependencyManager::CreateBrowserContextServices. Because of
+// this, in search_engine_provider_service_factory.cc (for example), in
+// InitializeSearchEngineProviderServiceIfNeeded we can't correctly identify
+// the guest profile by calling IsGuestSession and have to use this function.
+bool IsGuestProfile(Profile* profile);
+
+}  // namespace brave
+
+#endif  // BRAVE_BROWSER_PROFILES_PROFILE_UTIL_H_
diff --git a/browser/search_engines/guest_window_search_engine_provider_service.cc b/browser/search_engines/guest_window_search_engine_provider_service.cc
@@ -6,6 +6,7 @@
 #include "brave/browser/search_engines/guest_window_search_engine_provider_service.h"
 
 #include "base/auto_reset.h"
+#include "brave/browser/profiles/profile_util.h"
 #include "brave/browser/search_engines/search_engine_provider_util.h"
 #include "brave/components/search_engines/brave_prepopulated_engines.h"
 #include "chrome/browser/profiles/profile.h"
@@ -16,8 +17,8 @@
 GuestWindowSearchEngineProviderService::GuestWindowSearchEngineProviderService(
     Profile* otr_profile)
     : SearchEngineProviderService(otr_profile) {
-  DCHECK(IsGuestProfile(otr_profile));
-  DCHECK(!otr_profile->IsTorProfile());
+  DCHECK(brave::IsGuestProfile(otr_profile));
+  DCHECK(otr_profile->IsOffTheRecord());
   DCHECK(!brave::IsRegionForQwant(otr_profile));
 
   // Monitor otr(off the record) profile's search engine changing to tracking

diff --git a/browser/search_engines/search_engine_provider_service_browsertest.cc b/browser/search_engines/search_engine_provider_service_browsertest.cc
@@ -5,6 +5,7 @@
 
 #include "base/strings/utf_string_conversions.h"
 #include "brave/browser/profiles/brave_profile_manager.h"
+#include "brave/browser/profiles/profile_util.h"
 #include "brave/browser/search_engines/search_engine_provider_util.h"
 #include "brave/browser/tor/buildflags.h"
 #include "brave/browser/ui/browser_commands.h"
@@ -141,7 +142,7 @@ IN_PROC_BROWSER_TEST_F(SearchEngineProviderServiceTest,
   content::RunAllTasksUntilIdle();
 
   Profile* tor_profile = BrowserList::GetInstance()->GetLastActive()->profile();
-  EXPECT_TRUE(tor_profile->IsTorProfile());
+  EXPECT_TRUE(brave::IsTorProfile(tor_profile));
 
   auto* service = TemplateURLServiceFactory::GetForProfile(tor_profile);
 
@@ -170,7 +171,7 @@ IN_PROC_BROWSER_TEST_F(SearchEngineProviderServiceTest,
   content::RunAllTasksUntilIdle();
 
   Profile* tor_profile = BrowserList::GetInstance()->GetLastActive()->profile();
-  EXPECT_TRUE(tor_profile->IsTorProfile());
+  EXPECT_TRUE(brave::IsTorProfile(tor_profile));
 
   int expected_provider_id =
       TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_BING;

diff --git a/browser/search_engines/search_engine_provider_service_factory.cc b/browser/search_engines/search_engine_provider_service_factory.cc
@@ -5,6 +5,7 @@
 
 #include "brave/browser/search_engines/search_engine_provider_service_factory.h"
 
+#include "brave/browser/profiles/profile_util.h"
 #include "brave/browser/search_engines/guest_window_search_engine_provider_service.h"
 #include "brave/browser/search_engines/private_window_search_engine_provider_service.h"
 #include "brave/browser/search_engines/search_engine_provider_util.h"
@@ -18,21 +19,12 @@
 #include "components/pref_registry/pref_registry_syncable.h"
 
 namespace {
+
 // Factory owns service object.
 KeyedService* InitializeSearchEngineProviderServiceIfNeeded(Profile* profile) {
-  // In non qwant region, controller is also needed for private profile.
-  // We uses separate TemplateURLService for normal and off the recored profile.
-  // That means changing normal profile's provider doesn't affect otr profile's.
-  // This controller monitor's normal profile's service and apply it's change to
-  // otr profile to use same provider.
-  // Private profile's setting is shared with normal profile's setting.
-  if (profile->IsIncognitoProfile()) {
-    return new PrivateWindowSearchEngineProviderService(profile);
-  }
-
   // Regardless of qwant region, tor profile needs controller to store
   // previously set search engine provider.
-  if (profile->IsTorProfile() && IsGuestProfile(profile)) {
+  if (brave::IsTorProfile(profile) && profile->IsOffTheRecord()) {
     return new TorWindowSearchEngineProviderService(profile);
   }
 
@@ -41,10 +33,20 @@ KeyedService* InitializeSearchEngineProviderServiceIfNeeded(Profile* profile) {
   if (brave::IsRegionForQwant(profile))
     return nullptr;
 
-  if (IsGuestProfile(profile)) {
+  if (brave::IsGuestProfile(profile) && profile->IsOffTheRecord()) {
     return new GuestWindowSearchEngineProviderService(profile);
   }
 
+  // In non qwant region, controller is also needed for private profile.
+  // We use separate TemplateURLService for normal and off the recored profile.
+  // That means changing normal profile's provider doesn't affect otr profile's.
+  // This controller monitor's normal profile's service and apply its change to
+  // otr profile to use same provider.
+  // Private profile's setting is shared with normal profile's setting.
+  if (profile->IsIncognitoProfile()) {
+    return new PrivateWindowSearchEngineProviderService(profile);
+  }
+
   return nullptr;
 }
 

diff --git a/browser/search_engines/search_engine_provider_util.cc b/browser/search_engines/search_engine_provider_util.cc
@@ -1,4 +1,5 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
+/* Copyright (c) 2019 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/. */
 

diff --git a/browser/search_engines/search_engine_provider_util.h b/browser/search_engines/search_engine_provider_util.h
@@ -1,4 +1,5 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
+/* Copyright (c) 2019 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/. */
 

diff --git a/browser/search_engines/tor_window_search_engine_provider_service.cc b/browser/search_engines/tor_window_search_engine_provider_service.cc
@@ -5,6 +5,7 @@
 
 #include "brave/browser/search_engines/tor_window_search_engine_provider_service.h"
 
+#include "brave/browser/profiles/profile_util.h"
 #include "brave/browser/search_engines/search_engine_provider_util.h"
 #include "brave/common/pref_names.h"
 #include "brave/components/search_engines/brave_prepopulated_engines.h"
@@ -15,8 +16,8 @@
 TorWindowSearchEngineProviderService::
 TorWindowSearchEngineProviderService(Profile* otr_profile)
     : SearchEngineProviderService(otr_profile) {
-  DCHECK(otr_profile->IsTorProfile());
-  DCHECK(IsGuestProfile(otr_profile));
+  DCHECK(brave::IsTorProfile(otr_profile));
+  DCHECK(otr_profile->IsOffTheRecord());
 
   alternative_search_engine_provider_in_tor_.Init(
       kAlternativeSearchEngineProviderInTor,

diff --git a/browser/themes/brave_theme_service.cc b/browser/themes/brave_theme_service.cc
@@ -10,6 +10,7 @@
 #include "base/command_line.h"
 #include "base/strings/string_util.h"
 #include "brave/browser/extensions/brave_theme_event_router.h"
+#include "brave/browser/profiles/profile_util.h"
 #include "brave/browser/themes/theme_properties.h"
 #include "brave/browser/themes/brave_theme_utils.h"
 #include "brave/common/brave_switches.h"
@@ -180,7 +181,7 @@ SkColor BraveThemeService::GetDefaultColor(int id, bool incognito) const {
 #endif
 
   // Brave Tor profiles are always 'incognito' (for now)
-  if (!incognito && profile()->IsTorProfile())
+  if (!incognito && brave::IsTorProfile(profile()))
     incognito = true;
   const BraveThemeType theme = GetActiveBraveThemeType(profile());
   const base::Optional<SkColor> braveColor =

diff --git a/browser/ui/BUILD.gn b/browser/ui/BUILD.gn
@@ -94,6 +94,8 @@ source_set("ui") {
       "views/profiles/brave_incognito_menu_view.h",
       "views/profiles/brave_profile_chooser_view.cc",
       "views/profiles/brave_profile_chooser_view.h",
+      "views/profiles/brave_profile_menu_view_helper.cc",
+      "views/profiles/brave_profile_menu_view_helper.h",
       "webui/brave_welcome_ui.cc",
       "webui/brave_welcome_ui.h",
     ]

diff --git a/browser/ui/content_settings/brave_content_setting_bubble_model.cc b/browser/ui/content_settings/brave_content_setting_bubble_model.cc
@@ -1,13 +1,17 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
+/* Copyright (c) 2019 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/ui/content_settings/brave_content_setting_bubble_model.h"
 
+#include <memory>
+#include <string>
+
+#include "brave/browser/profiles/profile_util.h"
 #include "brave/components/brave_shields/common/brave_shield_constants.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/plugins/plugin_utils.h"
-#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/subresource_filter/chrome_subresource_filter_client.h"
 #include "chrome/browser/ui/content_settings/content_setting_bubble_model_delegate.h"
 #include "chrome/grit/generated_resources.h"
@@ -29,7 +33,7 @@ BraveContentSettingPluginBubbleModel::BraveContentSettingPluginBubbleModel(
   set_show_learn_more(true);
 
   // Do not show "Run flash this time" and "Manage" button in Tor profile.
-  if (GetProfile()->IsTorProfile()) {
+  if (brave::IsTorProfile(GetProfile())) {
     set_manage_text_style(ContentSettingBubbleModel::ManageTextStyle::kNone);
     return;
   }

diff --git a/browser/ui/toolbar/brave_app_menu_model.cc b/browser/ui/toolbar/brave_app_menu_model.cc
@@ -6,6 +6,7 @@
 #include "brave/browser/ui/toolbar/brave_app_menu_model.h"
 
 #include "brave/app/brave_command_ids.h"
+#include "brave/browser/profiles/profile_util.h"
 #include "brave/components/brave_sync/brave_sync_service.h"
 #include "brave/grit/brave_generated_resources.h"
 #include "chrome/app/chrome_command_ids.h"
@@ -57,7 +58,7 @@ void BraveAppMenuModel::InsertBraveMenuItems() {
       GetIndexOfCommandId(IDC_SHOW_DOWNLOADS),
       IDC_SHOW_BRAVE_ADBLOCK,
       IDS_SHOW_BRAVE_ADBLOCK);
-  if (browser_->profile()->IsTorProfile()) {
+  if (brave::IsTorProfile(browser_->profile())) {
     InsertItemWithStringIdAt(
         GetIndexOfCommandId(IDC_NEW_WINDOW),
         IDC_NEW_TOR_CONNECTION_FOR_SITE,

diff --git a/browser/ui/views/brave_browser_main_extra_parts_views_linux.cc b/browser/ui/views/brave_browser_main_extra_parts_views_linux.cc
@@ -6,6 +6,7 @@
 #include "brave/browser/ui/views/brave_browser_main_extra_parts_views_linux.h"
 
 #include "base/bind.h"
+#include "brave/browser/profiles/profile_util.h"
 #include "brave/browser/themes/brave_theme_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/themes/theme_service_factory.h"
@@ -34,7 +35,7 @@ ui::NativeTheme* GetNativeThemeForWindow(aura::Window* window) {
                             BraveThemeService::GetActiveBraveThemeType(profile);
   const bool dark_mode =
       (active_builtin_theme == BraveThemeType::BRAVE_THEME_TYPE_DARK ||
-       profile->IsIncognitoProfile() || profile->IsTorProfile());
+       profile->IsIncognitoProfile() || brave::IsTorProfile(profile));
   if (dark_mode && BrowserView::GetBrowserViewForNativeWindow(window)) {
     return ui::NativeThemeDarkAura::instance();
   }

diff --git a/browser/ui/views/location_bar/brave_location_bar_view.cc b/browser/ui/views/location_bar/brave_location_bar_view.cc
@@ -5,10 +5,11 @@
 
 #include "brave/browser/ui/views/location_bar/brave_location_bar_view.h"
 
+#include "brave/browser/profiles/profile_util.h"
 #include "brave/browser/themes/brave_theme_service.h"
+#include "brave/browser/ui/views/brave_actions/brave_actions_container.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/themes/theme_service_factory.h"
-#include "brave/browser/ui/views/brave_actions/brave_actions_container.h"
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/omnibox/omnibox_theme.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
@@ -82,7 +83,7 @@ OmniboxTint BraveLocationBarView::CalculateTint() const {
   // Match the user-selectable brave theme, even if there is a theme extension
   // installed, allowing non-extension-themeable elements to fit in better with
   // a theme extension.
-  if (profile()->IsIncognitoProfile() || profile()->IsTorProfile()) {
+  if (profile()->IsIncognitoProfile() || brave::IsTorProfile(profile())) {
     return OmniboxTint::PRIVATE;  // special extra enum value
   }
   // TODO(petemill): BraveThemeService can have a simpler get dark / light

diff --git a/browser/ui/views/profiles/brave_avatar_toolbar_button.cc b/browser/ui/views/profiles/brave_avatar_toolbar_button.cc
@@ -1,7 +1,12 @@
+/* Copyright (c) 2019 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/ui/views/profiles/brave_avatar_toolbar_button.h"
 
 #include "brave/app/vector_icons/vector_icons.h"
-#include "chrome/browser/profiles/profile.h"
+#include "brave/browser/profiles/profile_util.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/ui/views/profiles/avatar_toolbar_button.h"
 #include "ui/base/material_design/material_design_controller.h"
@@ -10,7 +15,7 @@
 #include "ui/gfx/paint_vector_icon.h"
 
 gfx::ImageSkia BraveAvatarToolbarButton::GetAvatarIcon() const {
-  if (profile_->IsTorProfile()) {
+  if (brave::IsTorProfile(profile_)) {
     const int icon_size =
       ui::MaterialDesignController::touch_ui() ? 24 : 20;
     const SkColor icon_color =