From 0576c813d2b0df4ce2eb91edf9aa4d8052a488ef Mon Sep 17 00:00:00 2001 From: Darnell Andries Date: Mon, 11 Sep 2023 21:37:49 -0700 Subject: [PATCH] Create AutocompleteMediator super class instead of patching, for capturing Android URL events in P3A --- android/brave_java_sources.gni | 1 + android/java/apk_for_test.flags | 4 +++ .../BraveAutocompleteMediator.java | 17 ---------- .../BraveAutocompleteMediatorBase.java | 33 +++++++++++++++++++ android/java/proguard.flags | 5 +++ .../chromium/chrome/browser/BytecodeTest.java | 14 ++++++++ ...browser_context_keyed_service_factories.cc | 5 +++ build/android/bytecode/BUILD.gn | 1 + ...eAutocompleteMediatorBaseClassAdapter.java | 25 ++++++++++++++ .../org/brave/bytecode/BraveClassAdapter.java | 1 + build/android/config.gni | 2 +- ...uggestions-AutocompleteMediator.java.patch | 13 -------- test/BUILD.gn | 1 + 13 files changed, 91 insertions(+), 31 deletions(-) create mode 100644 android/java/org/chromium/chrome/browser/omnibox/suggestions/BraveAutocompleteMediatorBase.java create mode 100644 build/android/bytecode/java/org/brave/bytecode/BraveAutocompleteMediatorBaseClassAdapter.java delete mode 100644 patches/chrome-browser-ui-android-omnibox-java-src-org-chromium-chrome-browser-omnibox-suggestions-AutocompleteMediator.java.patch diff --git a/android/brave_java_sources.gni b/android/brave_java_sources.gni index 7ee6b87da47..581808f5fa6 100644 --- a/android/brave_java_sources.gni +++ b/android/brave_java_sources.gni @@ -282,6 +282,7 @@ brave_java_sources = [ "../../brave/android/java/org/chromium/chrome/browser/omnibox/BraveLocationBarMediator.java", "../../brave/android/java/org/chromium/chrome/browser/omnibox/BraveLocationBarQRDialogFragment.java", "../../brave/android/java/org/chromium/chrome/browser/omnibox/suggestions/BraveAutocompleteMediator.java", + "../../brave/android/java/org/chromium/chrome/browser/omnibox/suggestions/BraveAutocompleteMediatorBase.java", "../../brave/android/java/org/chromium/chrome/browser/onboarding/BraveRewardsOnboardingFragment.java", "../../brave/android/java/org/chromium/chrome/browser/onboarding/OnViewPagerAction.java", "../../brave/android/java/org/chromium/chrome/browser/onboarding/OnboardingActivity.java", diff --git a/android/java/apk_for_test.flags b/android/java/apk_for_test.flags index 4bf9e900198..1ab121f15d3 100644 --- a/android/java/apk_for_test.flags +++ b/android/java/apk_for_test.flags @@ -541,6 +541,10 @@ public (...); } +-keep class org.chromium.chrome.browser.omnibox.suggestions.BraveAutocompleteMediatorBase { + public (...); +} + -keep class org.chromium.chrome.browser.notifications.permissions.NotificationPermissionRationaleDialogController { *** wrapDialogDismissalCallback(...); public (...); diff --git a/android/java/org/chromium/chrome/browser/omnibox/suggestions/BraveAutocompleteMediator.java b/android/java/org/chromium/chrome/browser/omnibox/suggestions/BraveAutocompleteMediator.java index 71024f8b9f1..7c098553dc0 100644 --- a/android/java/org/chromium/chrome/browser/omnibox/suggestions/BraveAutocompleteMediator.java +++ b/android/java/org/chromium/chrome/browser/omnibox/suggestions/BraveAutocompleteMediator.java @@ -13,7 +13,6 @@ import androidx.annotation.Nullable; import org.chromium.base.Callback; import org.chromium.base.supplier.Supplier; -import org.chromium.chrome.browser.app.BraveActivity; import org.chromium.chrome.browser.omnibox.LocationBarDataProvider; import org.chromium.chrome.browser.omnibox.UrlBarEditingTextStateProvider; import org.chromium.chrome.browser.omnibox.suggestions.basic.BasicSuggestionProcessor.BookmarkState; @@ -24,13 +23,10 @@ import org.chromium.chrome.browser.profiles.ProfileManager; import org.chromium.chrome.browser.share.ShareDelegate; import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tabmodel.TabWindowManager; -import org.chromium.components.omnibox.AutocompleteMatch; -import org.chromium.components.omnibox.OmniboxSuggestionType; import org.chromium.components.omnibox.action.OmniboxActionDelegate; import org.chromium.components.user_prefs.UserPrefs; import org.chromium.ui.modaldialog.ModalDialogManager; import org.chromium.ui.modelutil.PropertyModel; -import org.chromium.url.GURL; class BraveAutocompleteMediator extends AutocompleteMediator implements BraveSuggestionHost { private static final String AUTOCOMPLETE_ENABLED = "brave.autocomplete_enabled"; @@ -78,19 +74,6 @@ class BraveAutocompleteMediator extends AutocompleteMediator implements BraveSug super.onUrlFocusChange(hasFocus); } - @Override - void loadUrlForOmniboxMatch(int matchIndex, @NonNull AutocompleteMatch suggestion, - @NonNull GURL url, long inputStart, boolean inVisibleSuggestionList) { - super.loadUrlForOmniboxMatch( - matchIndex, suggestion, url, inputStart, inVisibleSuggestionList); - if (suggestion.getType() == OmniboxSuggestionType.SEARCH_WHAT_YOU_TYPED - || suggestion.getType() == OmniboxSuggestionType.SEARCH_SUGGEST) { - if (mContext != null && mContext instanceof BraveActivity) { - ((BraveActivity) mContext).getMiscAndroidMetrics().recordLocationBarQuery(); - } - } - } - @Override public void removeBraveSearchSuggestion() { if (mDropdownViewInfoListManager instanceof BraveDropdownItemViewInfoListManager) { diff --git a/android/java/org/chromium/chrome/browser/omnibox/suggestions/BraveAutocompleteMediatorBase.java b/android/java/org/chromium/chrome/browser/omnibox/suggestions/BraveAutocompleteMediatorBase.java new file mode 100644 index 00000000000..d7d75c72649 --- /dev/null +++ b/android/java/org/chromium/chrome/browser/omnibox/suggestions/BraveAutocompleteMediatorBase.java @@ -0,0 +1,33 @@ +/* Copyright (c) 2023 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 https://mozilla.org/MPL/2.0/. */ + +package org.chromium.chrome.browser.omnibox.suggestions; + +import android.content.Context; + +import androidx.annotation.NonNull; + +import org.chromium.base.BraveReflectionUtil; +import org.chromium.chrome.browser.app.BraveActivity; +import org.chromium.components.omnibox.AutocompleteMatch; +import org.chromium.components.omnibox.OmniboxSuggestionType; +import org.chromium.url.GURL; + +class BraveAutocompleteMediatorBase { + void loadUrlForOmniboxMatch(int matchIndex, @NonNull AutocompleteMatch suggestion, + @NonNull GURL url, long inputStart, boolean inVisibleSuggestionList) { + BraveReflectionUtil.InvokeMethod(AutocompleteMediator.class, this, "loadUrlForOmniboxMatch", + int.class, matchIndex, AutocompleteMatch.class, suggestion, GURL.class, url, + long.class, inputStart, boolean.class, inVisibleSuggestionList); + if (suggestion.getType() == OmniboxSuggestionType.SEARCH_WHAT_YOU_TYPED + || suggestion.getType() == OmniboxSuggestionType.SEARCH_SUGGEST) { + Context context = (Context) BraveReflectionUtil.getField( + AutocompleteMediator.class, "mContext", this); + if (context != null && context instanceof BraveActivity) { + ((BraveActivity) context).getMiscAndroidMetrics().recordLocationBarQuery(); + } + } + } +} diff --git a/android/java/proguard.flags b/android/java/proguard.flags index 578efbfcb05..d2eee9c07db 100644 --- a/android/java/proguard.flags +++ b/android/java/proguard.flags @@ -15,6 +15,11 @@ *** createViewProvider(...); } +-keep class org.chromium.chrome.browser.omnibox.suggestions.AutocompleteMediator { + *** mContext; + *** loadUrlForOmniboxMatch(...); +} + -keep class org.chromium.components.browser_ui.site_settings.ContentSettingsResources { *** getResourceItem(...); } diff --git a/android/javatests/org/chromium/chrome/browser/BytecodeTest.java b/android/javatests/org/chromium/chrome/browser/BytecodeTest.java index 627d22d657e..0a6540bd0b7 100644 --- a/android/javatests/org/chromium/chrome/browser/BytecodeTest.java +++ b/android/javatests/org/chromium/chrome/browser/BytecodeTest.java @@ -128,6 +128,7 @@ import org.chromium.components.embedder_support.contextmenu.ContextMenuParams; import org.chromium.components.external_intents.ExternalNavigationDelegate; import org.chromium.components.externalauth.ExternalAuthUtils; import org.chromium.components.favicon.LargeIconBridge; +import org.chromium.components.omnibox.AutocompleteMatch; import org.chromium.components.omnibox.action.OmniboxActionDelegate; import org.chromium.components.permissions.PermissionDialogController; import org.chromium.content_public.browser.BrowserContextHandle; @@ -140,6 +141,7 @@ import org.chromium.ui.modaldialog.ModalDialogManager; import org.chromium.ui.modelutil.MVCListAdapter; import org.chromium.ui.modelutil.MVCListAdapter.ModelList; import org.chromium.ui.modelutil.PropertyModel; +import org.chromium.url.GURL; import java.lang.reflect.Constructor; import java.lang.reflect.Field; @@ -292,6 +294,8 @@ public class BytecodeTest { "org/chromium/chrome/browser/omnibox/suggestions/AutocompleteCoordinator")); Assert.assertTrue(classExists( "org/chromium/chrome/browser/omnibox/suggestions/BraveAutocompleteCoordinator")); + Assert.assertTrue(classExists( + "org/chromium/chrome/browser/omnibox/suggestions/BraveAutocompleteMediatorBase")); Assert.assertTrue(classExists( "org/chromium/chrome/browser/omnibox/suggestions/DropdownItemViewInfoListBuilder")); Assert.assertTrue(classExists( @@ -534,6 +538,10 @@ public class BytecodeTest { "org/chromium/chrome/browser/omnibox/suggestions/AutocompleteCoordinator", "createViewProvider", true, ViewProvider.class, Context.class, MVCListAdapter.ModelList.class)); + Assert.assertTrue( + methodExists("org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator", + "loadUrlForOmniboxMatch", true, void.class, int.class, + AutocompleteMatch.class, GURL.class, long.class, boolean.class)); // Check for method type declaration changes here Assert.assertTrue(methodExists( @@ -1015,6 +1023,9 @@ public class BytecodeTest { Assert.assertTrue( fieldExists("org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator", "mDropdownViewInfoListManager")); + Assert.assertTrue( + fieldExists("org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator", + "mContext")); Assert.assertTrue(fieldExists( "org/chromium/chrome/browser/ntp/NewTabPageLayout", "mMvTilesContainerLayout")); Assert.assertTrue(fieldExists( @@ -1126,6 +1137,9 @@ public class BytecodeTest { Assert.assertTrue(checkSuperName( "org/chromium/chrome/browser/omnibox/suggestions/AutocompleteCoordinator", "org/chromium/chrome/browser/omnibox/suggestions/BraveAutocompleteCoordinator")); + Assert.assertTrue(checkSuperName( + "org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator", + "org/chromium/chrome/browser/omnibox/suggestions/BraveAutocompleteMediatorBase")); Assert.assertTrue(checkSuperName("org/chromium/chrome/browser/omnibox/LocationBarPhone", "org/chromium/chrome/browser/omnibox/BraveLocationBarLayout")); Assert.assertTrue(checkSuperName("org/chromium/chrome/browser/omnibox/LocationBarTablet", diff --git a/browser/browser_context_keyed_service_factories.cc b/browser/browser_context_keyed_service_factories.cc index 1a2d1a54612..d11788728b8 100644 --- a/browser/browser_context_keyed_service_factories.cc +++ b/browser/browser_context_keyed_service_factories.cc @@ -60,6 +60,7 @@ #else #include "brave/browser/brave_shields/cookie_list_opt_in_service_factory.h" #include "brave/browser/brave_shields/filter_list_service_factory.h" +#include "brave/browser/misc_metrics/misc_android_metrics_factory.h" #include "brave/browser/ntp_background/android/ntp_background_images_bridge.h" #endif @@ -201,6 +202,10 @@ void EnsureBrowserContextKeyedServiceFactoriesBuilt() { #if BUILDFLAG(ENABLE_SPEEDREADER) speedreader::SpeedreaderServiceFactory::GetInstance(); #endif + +#if BUILDFLAG(IS_ANDROID) + misc_metrics::MiscAndroidMetricsFactory::GetInstance(); +#endif } } // namespace brave diff --git a/build/android/bytecode/BUILD.gn b/build/android/bytecode/BUILD.gn index e73dbceb633..0ca8cb39b0d 100644 --- a/build/android/bytecode/BUILD.gn +++ b/build/android/bytecode/BUILD.gn @@ -14,6 +14,7 @@ java_binary("java_bytecode_rewriter") { "//brave/build/android/bytecode/java/org/brave/bytecode/BraveAppMenuClassAdapter.java", "//brave/build/android/bytecode/java/org/brave/bytecode/BraveApplicationImplBaseClassAdapter.java", "//brave/build/android/bytecode/java/org/brave/bytecode/BraveAutocompleteCoordinatorClassAdapter.java", + "//brave/build/android/bytecode/java/org/brave/bytecode/BraveAutocompleteMediatorBaseClassAdapter.java", "//brave/build/android/bytecode/java/org/brave/bytecode/BraveAutocompleteMediatorClassAdapter.java", "//brave/build/android/bytecode/java/org/brave/bytecode/BraveAutofillPopupBridgeClassAdapter.java", "//brave/build/android/bytecode/java/org/brave/bytecode/BraveBookmarkActivityClassAdapter.java", diff --git a/build/android/bytecode/java/org/brave/bytecode/BraveAutocompleteMediatorBaseClassAdapter.java b/build/android/bytecode/java/org/brave/bytecode/BraveAutocompleteMediatorBaseClassAdapter.java new file mode 100644 index 00000000000..55c58f53f99 --- /dev/null +++ b/build/android/bytecode/java/org/brave/bytecode/BraveAutocompleteMediatorBaseClassAdapter.java @@ -0,0 +1,25 @@ +/* Copyright (c) 2023 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 https://mozilla.org/MPL/2.0/. */ + +package org.brave.bytecode; + +import org.objectweb.asm.ClassVisitor; + +public class BraveAutocompleteMediatorBaseClassAdapter extends BraveClassVisitor { + static String sAutocompleteMediator = + "org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator"; + static String sBraveAutocompleteMediatorBase = + "org/chromium/chrome/browser/omnibox/suggestions/BraveAutocompleteMediatorBase"; + + public BraveAutocompleteMediatorBaseClassAdapter(ClassVisitor visitor) { + super(visitor); + + changeSuperName(sAutocompleteMediator, sBraveAutocompleteMediatorBase); + + makeProtectedField(sAutocompleteMediator, "mContext"); + changeMethodOwner( + sAutocompleteMediator, "loadUrlForOmniboxMatch", sBraveAutocompleteMediatorBase); + } +} diff --git a/build/android/bytecode/java/org/brave/bytecode/BraveClassAdapter.java b/build/android/bytecode/java/org/brave/bytecode/BraveClassAdapter.java index 7c7c3422e6c..daa00b00e19 100644 --- a/build/android/bytecode/java/org/brave/bytecode/BraveClassAdapter.java +++ b/build/android/bytecode/java/org/brave/bytecode/BraveClassAdapter.java @@ -14,6 +14,7 @@ public class BraveClassAdapter { chain = new BraveAppMenuClassAdapter(chain); chain = new BraveApplicationImplBaseClassAdapter(chain); chain = new BraveAutocompleteCoordinatorClassAdapter(chain); + chain = new BraveAutocompleteMediatorBaseClassAdapter(chain); chain = new BraveAutocompleteMediatorClassAdapter(chain); chain = new BraveAutofillPopupBridgeClassAdapter(chain); chain = new BraveBookmarkActivityClassAdapter(chain); diff --git a/build/android/config.gni b/build/android/config.gni index e396205ded9..79b426b48d2 100644 --- a/build/android/config.gni +++ b/build/android/config.gni @@ -124,7 +124,7 @@ brave_jni_headers_sources = [ "//brave/android/java/org/chromium/chrome/browser/crypto_wallet/util/WalletDataFilesInstaller.java", "//brave/android/java/org/chromium/chrome/browser/crypto_wallet/util/WalletNativeUtils.java", "//brave/android/java/org/chromium/chrome/browser/informers/BraveSyncAccountDeletedInformer.java", - "//brave/android/java/org/chromium/chrome/browser/misc_metrics/PrivacyHubMetricsFactory.java", + "//brave/android/java/org/chromium/chrome/browser/misc_metrics/MiscAndroidMetricsFactory.java", "//brave/android/java/org/chromium/chrome/browser/notifications/BraveNotificationPlatformBridge.java", "//brave/android/java/org/chromium/chrome/browser/notifications/BraveNotificationSettingsBridge.java", "//brave/android/java/org/chromium/chrome/browser/ntp_background_images/NTPBackgroundImagesBridge.java", diff --git a/patches/chrome-browser-ui-android-omnibox-java-src-org-chromium-chrome-browser-omnibox-suggestions-AutocompleteMediator.java.patch b/patches/chrome-browser-ui-android-omnibox-java-src-org-chromium-chrome-browser-omnibox-suggestions-AutocompleteMediator.java.patch deleted file mode 100644 index 86d81d0da12..00000000000 --- a/patches/chrome-browser-ui-android-omnibox-java-src-org-chromium-chrome-browser-omnibox-suggestions-AutocompleteMediator.java.patch +++ /dev/null @@ -1,13 +0,0 @@ -diff --git a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator.java b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator.java -index f7bcce140aca2638651b13cfb77c2e9bf71e866a..c8b1773894aa20f27125f76d3806ba1977b002dc 100644 ---- a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator.java -+++ b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator.java -@@ -809,7 +809,7 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, - * @param inputStart The timestamp the input was started. - * @param inVisibleSuggestionList Whether the suggestion is in the visible suggestion list. - */ -- private void loadUrlForOmniboxMatch(int matchIndex, @NonNull AutocompleteMatch suggestion, -+ void loadUrlForOmniboxMatch(int matchIndex, @NonNull AutocompleteMatch suggestion, - @NonNull GURL url, long inputStart, boolean inVisibleSuggestionList) { - try (TraceEvent e = TraceEvent.scoped("AutocompleteMediator.loadUrlFromOmniboxMatch")) { - OmniboxMetrics.recordFocusToOpenTime(System.currentTimeMillis() - mUrlFocusTime); diff --git a/test/BUILD.gn b/test/BUILD.gn index 11f7f22f39c..bb1ed2a495a 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -1282,6 +1282,7 @@ if (is_android) { "//third_party/androidx:androidx_test_runner_java", "//third_party/junit", "//ui/android:ui_no_recycler_view_java", + "//url:gurl_java", "//url/mojom:url_mojom_gurl_java", ] }