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

Fixes crash when suggesting in Private window. #5221

Merged
merged 2 commits into from
Apr 15, 2020
Merged
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
2 changes: 2 additions & 0 deletions browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ source_set("browser_process") {
sources = [
"autocomplete/brave_autocomplete_provider_client.cc",
"autocomplete/brave_autocomplete_provider_client.h",
"autocomplete/brave_autocomplete_provider_client_for_classifier.cc",
"autocomplete/brave_autocomplete_provider_client_for_classifier.h",
"autocomplete/brave_autocomplete_scheme_classifier.cc",
"autocomplete/brave_autocomplete_scheme_classifier.h",
"brave_rewards/rewards_tab_helper.cc",
Expand Down
18 changes: 3 additions & 15 deletions browser/autocomplete/brave_autocomplete_provider_client.cc
Original file line number Diff line number Diff line change
@@ -1,33 +1,21 @@
/* 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/autocomplete/brave_autocomplete_provider_client.h"

#include "base/strings/utf_string_conversions.h"
#include "brave/common/webui_url_constants.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/common/webui_url_constants.h"

BraveAutocompleteProviderClient::BraveAutocompleteProviderClient(
Profile* profile)
: ChromeAutocompleteProviderClient(profile->GetOriginalProfile()),
Copy link
Collaborator

@bridiver bridiver Apr 15, 2020

Choose a reason for hiding this comment

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

I'm fairly certain we're leaking private autocomplete into the regular profile here in the old code and here in the updated code https://github.com/brave/brave-core/pull/5221/files#diff-2325211a427ac6f007ad338411e100aaR14 cc @diracdeltas @simonhong

profile_(profile) {
Profile* profile) : ChromeAutocompleteProviderClient(profile) {
}

BraveAutocompleteProviderClient::~BraveAutocompleteProviderClient() {
}

TemplateURLService* BraveAutocompleteProviderClient::GetTemplateURLService() {
return TemplateURLServiceFactory::GetForProfile(profile_);
}

const TemplateURLService*
BraveAutocompleteProviderClient::GetTemplateURLService() const {
return TemplateURLServiceFactory::GetForProfile(profile_);
}

std::vector<base::string16> BraveAutocompleteProviderClient::GetBuiltinURLs() {
std::vector<base::string16> v =
ChromeAutocompleteProviderClient::GetBuiltinURLs();
Expand Down
28 changes: 5 additions & 23 deletions browser/autocomplete/brave_autocomplete_provider_client.h
Original file line number Diff line number Diff line change
@@ -1,43 +1,25 @@
/* 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/. */

#ifndef BRAVE_BROWSER_AUTOCOMPLETE_BRAVE_AUTOCOMPLETE_PROVIDER_CLIENT_H_
#define BRAVE_BROWSER_AUTOCOMPLETE_BRAVE_AUTOCOMPLETE_PROVIDER_CLIENT_H_

#include <string>
#include <vector>

#include "chrome/browser/autocomplete/chrome_autocomplete_provider_client.h"

// In brave, different AutocompleteClassifiers are created for normal and
// incognito profile by changing
// AutocompleteClassifierFactory::GetBrowserContextToUse().
// This changes are needed to use different search engine used by web search in
// web page context menu.
// When context menu handles web search it gets search engine url from
// ChromeAutocompleteProviderClient via AutocompleteClassifiers.
// Because of this, private window will use same search engine url of normal
// window if same AutocompleteClassifiers are used on normal and incognito.
// So, we made this change.
// However, ChromeAutocompleteProviderClient exposes many other services based
// on profiles.
// We don't want to change other services. Only wanted to get proper
// TemplateURLService. To achieve this, BraveAutocompleteProviderClient is
// introduced. It initializes ChromeAutocompleteProviderClient with original
// profile and only overrided TemplateURLService getter.
// BraveAutocompleteSchemeClassifier also initialize
// ChromeAutocompleteSchemeClassifier with original profile for same reason.
class BraveAutocompleteProviderClient
: public ChromeAutocompleteProviderClient {
public:
explicit BraveAutocompleteProviderClient(Profile* profile);
~BraveAutocompleteProviderClient() override;

TemplateURLService* GetTemplateURLService() override;
const TemplateURLService* GetTemplateURLService() const override;
std::vector<base::string16> GetBuiltinURLs() override;

private:
Profile* profile_;

DISALLOW_COPY_AND_ASSIGN(BraveAutocompleteProviderClient);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* 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/autocomplete/brave_autocomplete_provider_client.h"
#include "brave/browser/autocomplete/brave_autocomplete_provider_client_for_classifier.h"

#include "chrome/browser/autocomplete/autocomplete_classifier_factory.h"
#include "chrome/browser/profiles/profile.h"
Expand All @@ -18,15 +18,15 @@ using BraveAutocompleteProviderClientTest = InProcessBrowserTest;
IN_PROC_BROWSER_TEST_F(BraveAutocompleteProviderClientTest,
DependentServiceCheckTest) {
Profile* profile = browser()->profile();
Profile* incognito_profile = profile->GetOffTheRecordProfile();
Profile* otr_profile = profile->GetOffTheRecordProfile();

// Brave initiates different AutocompleteClassifier service for
// normal/incognito profile.
EXPECT_NE(AutocompleteClassifierFactory::GetForProfile(profile),
AutocompleteClassifierFactory::GetForProfile(incognito_profile));
AutocompleteClassifierFactory::GetForProfile(otr_profile));

BraveAutocompleteProviderClient normal_client(profile);
BraveAutocompleteProviderClient incognito_client(incognito_profile);
BraveAutocompleteProviderClientForClassifier normal_client(profile);
BraveAutocompleteProviderClientForClassifier incognito_client(otr_profile);

// Check different TemplateURLService is used.
EXPECT_NE(normal_client.GetTemplateURLService(),
Expand Down
Original file line number Diff line number Diff line change
@@ -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/autocomplete/brave_autocomplete_provider_client_for_classifier.h"

#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"

BraveAutocompleteProviderClientForClassifier::
BraveAutocompleteProviderClientForClassifier(
Profile* profile)
: BraveAutocompleteProviderClient(profile->GetOriginalProfile()),
profile_(profile) {
}

BraveAutocompleteProviderClientForClassifier::
~BraveAutocompleteProviderClientForClassifier() {
}

TemplateURLService*
BraveAutocompleteProviderClientForClassifier::GetTemplateURLService() {
return TemplateURLServiceFactory::GetForProfile(profile_);
}

const TemplateURLService*
BraveAutocompleteProviderClientForClassifier::GetTemplateURLService() const {
return TemplateURLServiceFactory::GetForProfile(profile_);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/* 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_BROWSER_AUTOCOMPLETE_BRAVE_AUTOCOMPLETE_PROVIDER_CLIENT_FOR_CLASSIFIER_H_
#define BRAVE_BROWSER_AUTOCOMPLETE_BRAVE_AUTOCOMPLETE_PROVIDER_CLIENT_FOR_CLASSIFIER_H_

#include "brave/browser/autocomplete/brave_autocomplete_provider_client.h"

// In brave, different AutocompleteClassifiers are created for normal and
// incognito profile by changing
// AutocompleteClassifierFactory::GetBrowserContextToUse().
// This changes are needed to use different search engine used by web search in
// web page context menu.
// When context menu handles web search it gets search engine url from
// ChromeAutocompleteProviderClient via AutocompleteClassifiers.
// Because of this, private window will use same search engine url of normal
// window if same AutocompleteClassifiers are used on normal and incognito.
// So, we made this change.
// However, ChromeAutocompleteProviderClient exposes many other services based
// on profiles.
// We don't want to change other services. Only wanted to get proper
// TemplateURLService. To achieve this, BraveAutocompleteProviderClient is
// introduced. It initializes ChromeAutocompleteProviderClient with original
// profile and only overrided TemplateURLService getter.
// BraveAutocompleteSchemeClassifier also initialize
// ChromeAutocompleteSchemeClassifier with original profile for same reason.
class BraveAutocompleteProviderClientForClassifier
: public BraveAutocompleteProviderClient {
public:
explicit BraveAutocompleteProviderClientForClassifier(Profile* profile);
~BraveAutocompleteProviderClientForClassifier() override;

TemplateURLService* GetTemplateURLService() override;
const TemplateURLService* GetTemplateURLService() const override;

private:
Profile* profile_;

DISALLOW_COPY_AND_ASSIGN(BraveAutocompleteProviderClientForClassifier);
};

#endif // BRAVE_BROWSER_AUTOCOMPLETE_BRAVE_AUTOCOMPLETE_PROVIDER_CLIENT_FOR_CLASSIFIER_H_
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
/* 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/autocomplete/brave_autocomplete_provider_client.h"
#include "brave/browser/autocomplete/brave_autocomplete_provider_client_for_classifier.h"
#include "brave/browser/autocomplete/brave_autocomplete_scheme_classifier.h"
#include "brave/components/omnibox/browser/brave_autocomplete_controller.h"
#include "components/omnibox/browser/autocomplete_classifier.h"
#include "components/omnibox/browser/autocomplete_controller.h"

#define AutocompleteController BraveAutocompleteController
#define ChromeAutocompleteProviderClient BraveAutocompleteProviderClient
#define ChromeAutocompleteProviderClient BraveAutocompleteProviderClientForClassifier // NOLINT
#define ChromeAutocompleteSchemeClassifier BraveAutocompleteSchemeClassifier
#include "../../../../../chrome/browser/autocomplete/autocomplete_classifier_factory.cc"
#undef ChromeAutocompleteSchemeClassifier
Expand Down