Skip to content

Commit

Permalink
Omnibox: Make QueryInOmniboxFactory so Java bridge is all-static.
Browse files Browse the repository at this point in the history
QueryInOmnibox functionality is intrinsically profile-keyed, since it
relies on two other profile-keyed services (AutocompleteClassifier and
TemplateURLService).

This suggests that QueryInOmnibox should also be a
BrowserContextKeyedService, just like those two classes.

If QueryInOmnibox is profile-keyed, it would be burdensome to have the
Java side manage its own instances, so this CL also makes all the Java
exposed methods static, with a Profile key.

Bug: 874592
Change-Id: Ic8de22b4d19479a349129e24b9e8732a06002ff0
Reviewed-on: https://chromium-review.googlesource.com/1199818
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588567}
  • Loading branch information
Tommy C. Li authored and Commit Bot committed Sep 4, 2018
1 parent 9b3146a commit d5db3d9
Show file tree
Hide file tree
Showing 8 changed files with 135 additions and 108 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,48 +11,33 @@
* Bridge to the native QueryInOmniboxAndroid.
*/
public class QueryInOmnibox {
private long mNativeQueryInOmniboxAndroid;

public QueryInOmnibox(Profile profile) {
assert profile != null;
assert profile.isNativeInitialized();
mNativeQueryInOmniboxAndroid = nativeInit(profile);
}

public void destroy() {
nativeDestroy(mNativeQueryInOmniboxAndroid);
mNativeQueryInOmniboxAndroid = 0;
}

/**
* Extracts query terms from the current URL if it's a SRP URL from the default search engine.
*
* @param profile The Profile associated with the tab.
* @param securityLevel The {@link ConnectionSecurityLevel} of the tab.
* @param url The URL to extract search terms from.
* @return The extracted search terms. Returns null if the Omnibox should not display the
* search terms.
*/
public String getDisplaySearchTerms(@ConnectionSecurityLevel int securityLevel, String url) {
assert mNativeQueryInOmniboxAndroid != 0;
return nativeGetDisplaySearchTerms(mNativeQueryInOmniboxAndroid, securityLevel, url);
public static String getDisplaySearchTerms(
Profile profile, @ConnectionSecurityLevel int securityLevel, String url) {
return nativeGetDisplaySearchTerms(profile, securityLevel, url);
}

/**
* Sets a flag telling the model to ignore the security level in its check for whether to
* display search terms or not. This is useful for avoiding the flicker that occurs when loading
* a SRP URL before our SSL state updates.
* display search terms or not. This is useful for avoiding the flicker that occurs when
* loading a SRP URL before our SSL state updates.
*
* @param profile The Profile associated with the tab.
* @param ignore Whether or not we should ignore the security level.
*/
public void setIgnoreSecurityLevelForSearchTerms(boolean ignore) {
assert mNativeQueryInOmniboxAndroid != 0;
nativeSetIgnoreSecurityLevel(mNativeQueryInOmniboxAndroid, ignore);
public static void setIgnoreSecurityLevelForSearchTerms(Profile profile, boolean ignore) {
nativeSetIgnoreSecurityLevel(profile, ignore);
}

private native long nativeInit(Profile profile);
private native void nativeDestroy(long nativeQueryInOmniboxAndroid);
private native String nativeGetDisplaySearchTerms(
long nativeQueryInOmniboxAndroid, int securityLevel, String url);
private native void nativeSetIgnoreSecurityLevel(
long nativeQueryInOmniboxAndroid, boolean ignore);
private static native String nativeGetDisplaySearchTerms(
Profile profile, int securityLevel, String url);
private static native void nativeSetIgnoreSecurityLevel(Profile profile, boolean ignore);
}
1 change: 0 additions & 1 deletion chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2237,7 +2237,6 @@ jumbo_split_static_library("browser") {
"android/omnibox/omnibox_prerender.cc",
"android/omnibox/omnibox_prerender.h",
"android/omnibox/query_in_omnibox_android.cc",
"android/omnibox/query_in_omnibox_android.h",
"android/oom_intervention/near_oom_monitor.cc",
"android/oom_intervention/near_oom_monitor.h",
"android/oom_intervention/oom_intervention_config.cc",
Expand Down
57 changes: 18 additions & 39 deletions chrome/browser/android/omnibox/query_in_omnibox_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,63 +2,42 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/android/omnibox/query_in_omnibox_android.h"

#include "base/android/jni_string.h"
#include "chrome/browser/autocomplete/autocomplete_classifier_factory.h"
#include "chrome/browser/profiles/profile_android.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/browser/ui/omnibox/query_in_omnibox_factory.h"
#include "components/omnibox/browser/query_in_omnibox.h"
#include "jni/QueryInOmnibox_jni.h"

using base::android::JavaParamRef;

QueryInOmniboxAndroid::QueryInOmniboxAndroid(Profile* profile)
: query_in_omnibox_(new QueryInOmnibox(
AutocompleteClassifierFactory::GetForProfile(profile),
TemplateURLServiceFactory::GetForProfile(profile))) {}

QueryInOmniboxAndroid::~QueryInOmniboxAndroid() {}

static jlong JNI_QueryInOmnibox_Init(JNIEnv* env,
const JavaParamRef<jobject>& obj,
const JavaParamRef<jobject>& jprofile) {
Profile* profile = ProfileAndroid::FromProfileAndroid(jprofile);
if (!profile)
return 0;

QueryInOmniboxAndroid* native_bridge = new QueryInOmniboxAndroid(profile);
return reinterpret_cast<intptr_t>(native_bridge);
}

void QueryInOmniboxAndroid::Destroy(JNIEnv* env,
const JavaParamRef<jobject>& obj) {
delete this;
}

base::android::ScopedJavaLocalRef<jstring>
QueryInOmniboxAndroid::GetDisplaySearchTerms(
JNIEnv* env,
const JavaParamRef<jobject>& obj,
jint j_security_level,
const JavaParamRef<jstring>& j_url) {
static base::android::ScopedJavaLocalRef<jstring>
JNI_QueryInOmnibox_GetDisplaySearchTerms(JNIEnv* env,
const JavaParamRef<jclass>&,
const JavaParamRef<jobject>& j_profile,
jint j_security_level,
const JavaParamRef<jstring>& j_url) {
Profile* profile = ProfileAndroid::FromProfileAndroid(j_profile);
security_state::SecurityLevel security_level =
static_cast<security_state::SecurityLevel>(j_security_level);
GURL url = GURL(base::android::ConvertJavaStringToUTF16(env, j_url));

base::string16 search_terms;
bool should_display = query_in_omnibox_->GetDisplaySearchTerms(
security_level, url, &search_terms);
bool should_display =
QueryInOmniboxFactory::GetForProfile(profile)->GetDisplaySearchTerms(
security_level, url, &search_terms);

if (!should_display)
return nullptr;

return base::android::ConvertUTF16ToJavaString(env, search_terms);
}

void QueryInOmniboxAndroid::SetIgnoreSecurityLevel(
void JNI_QueryInOmnibox_SetIgnoreSecurityLevel(
JNIEnv* env,
const JavaParamRef<jobject>& obj,
bool ignore) {
query_in_omnibox_->set_ignore_security_level(ignore);
const JavaParamRef<jclass>&,
const JavaParamRef<jobject>& j_profile,
jboolean ignore) {
Profile* profile = ProfileAndroid::FromProfileAndroid(j_profile);
QueryInOmniboxFactory::GetForProfile(profile)->set_ignore_security_level(
ignore);
}
40 changes: 0 additions & 40 deletions chrome/browser/android/omnibox/query_in_omnibox_android.h

This file was deleted.

2 changes: 2 additions & 0 deletions chrome/browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1162,6 +1162,8 @@ jumbo_split_static_library("ui") {
"android/view_android_helper.cc",
"android/view_android_helper.h",
"browser_otr_state_android.cc",
"omnibox/query_in_omnibox_factory.cc",
"omnibox/query_in_omnibox_factory.h",
"screen_capture_notification_ui_stub.cc",
"webui/eoc_internals/eoc_internals_page_handler.cc",
"webui/eoc_internals/eoc_internals_page_handler.h",
Expand Down
56 changes: 56 additions & 0 deletions chrome/browser/ui/omnibox/query_in_omnibox_factory.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/ui/omnibox/query_in_omnibox_factory.h"

#include "chrome/browser/autocomplete/autocomplete_classifier_factory.h"
#include "chrome/browser/profiles/incognito_helpers.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/omnibox/browser/query_in_omnibox.h"

// static
QueryInOmnibox* QueryInOmniboxFactory::GetForProfile(Profile* profile) {
return static_cast<QueryInOmnibox*>(
GetInstance()->GetServiceForBrowserContext(profile, true));
}

// static
QueryInOmniboxFactory* QueryInOmniboxFactory::GetInstance() {
return base::Singleton<QueryInOmniboxFactory>::get();
}

// static
std::unique_ptr<KeyedService> QueryInOmniboxFactory::BuildInstanceFor(
content::BrowserContext* context) {
Profile* profile = static_cast<Profile*>(context);
return std::make_unique<QueryInOmnibox>(
AutocompleteClassifierFactory::GetForProfile(profile),
TemplateURLServiceFactory::GetForProfile(profile));
}

QueryInOmniboxFactory::QueryInOmniboxFactory()
: BrowserContextKeyedServiceFactory(
"QueryInOmnibox",
BrowserContextDependencyManager::GetInstance()) {
DependsOn(AutocompleteClassifierFactory::GetInstance());
DependsOn(TemplateURLServiceFactory::GetInstance());
}

QueryInOmniboxFactory::~QueryInOmniboxFactory() {}

content::BrowserContext* QueryInOmniboxFactory::GetBrowserContextToUse(
content::BrowserContext* context) const {
return chrome::GetBrowserContextRedirectedInIncognito(context);
}

bool QueryInOmniboxFactory::ServiceIsNULLWhileTesting() const {
return true;
}

KeyedService* QueryInOmniboxFactory::BuildServiceInstanceFor(
content::BrowserContext* profile) const {
return BuildInstanceFor(static_cast<Profile*>(profile)).release();
}
45 changes: 45 additions & 0 deletions chrome/browser/ui/omnibox/query_in_omnibox_factory.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_UI_OMNIBOX_QUERY_IN_OMNIBOX_FACTORY_H_
#define CHROME_BROWSER_UI_OMNIBOX_QUERY_IN_OMNIBOX_FACTORY_H_

#include <memory>

#include "base/macros.h"
#include "base/memory/singleton.h"
#include "components/keyed_service/content/browser_context_keyed_service_factory.h"

class QueryInOmnibox;
class Profile;

// Singleton that owns all QueryInOmnibox instances and associates them with
// Profiles.
class QueryInOmniboxFactory : public BrowserContextKeyedServiceFactory {
public:
// Returns the QueryInOmnibox for |profile|.
static QueryInOmnibox* GetForProfile(Profile* profile);

static QueryInOmniboxFactory* GetInstance();

static std::unique_ptr<KeyedService> BuildInstanceFor(
content::BrowserContext* context);

private:
friend struct base::DefaultSingletonTraits<QueryInOmniboxFactory>;

QueryInOmniboxFactory();
~QueryInOmniboxFactory() override;

// BrowserContextKeyedServiceFactory:
content::BrowserContext* GetBrowserContextToUse(
content::BrowserContext* context) const override;
bool ServiceIsNULLWhileTesting() const override;
KeyedService* BuildServiceInstanceFor(
content::BrowserContext* profile) const override;

DISALLOW_COPY_AND_ASSIGN(QueryInOmniboxFactory);
};

#endif // CHROME_BROWSER_UI_OMNIBOX_QUERY_IN_OMNIBOX_FACTORY_H_
3 changes: 2 additions & 1 deletion components/omnibox/browser/query_in_omnibox.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "base/macros.h"
#include "base/strings/string16.h"
#include "components/keyed_service/core/keyed_service.h"
#include "components/security_state/core/security_state.h"
#include "url/gurl.h"

Expand All @@ -15,7 +16,7 @@ class TemplateURLService;

// This class holds the business logic for Query in Omnibox shared between both
// Android and Desktop.
class QueryInOmnibox {
class QueryInOmnibox : public KeyedService {
public:
QueryInOmnibox(AutocompleteClassifier* autocomplete_classifier,
TemplateURLService* template_url_service);
Expand Down

0 comments on commit d5db3d9

Please sign in to comment.