Bug 1261617 - Add UITelemetry for search counts. r=mfinkle draft
authorMichael Comella <michael.l.comella@gmail.com>
Wed, 11 May 2016 17:23:48 -0700
changeset 366123 7a5515c87deabfabd4a1af7ffc1804ed90294359
parent 365338 2f9351bae69d056e4615d21dda6bf42fec5d16b7
child 366143 84fe230ab36cda7b8b5e2ce74076e1732ee12616
push id17899
push usermichael.l.comella@gmail.com
push dateThu, 12 May 2016 00:27:17 +0000
reviewersmfinkle
bugs1261617, 1272166
milestone49.0a1
Bug 1261617 - Add UITelemetry for search counts. r=mfinkle I largerly kept this patch the same as Finkle's initial revision except that I don't include the engine identifiers due to bug 1272166. MozReview-Commit-ID: 84672CDOOVX
mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java
mobile/android/base/java/org/mozilla/gecko/home/SearchEngineRow.java
--- a/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
@@ -1935,16 +1935,17 @@ public class BrowserApp extends GeckoApp
 
                 // Display notification for Mozilla data reporting, if data should be collected.
                 if (AppConstants.MOZ_DATA_REPORTING && Restrictions.isAllowed(this, Restrictable.DATA_CHOICES)) {
                     DataReportingNotification.checkAndNotifyPolicy(GeckoAppShell.getContext());
                 }
 
             } else if (event.equals("Search:Keyword")) {
                 storeSearchQuery(message.getString("query"));
+                recordSearch(message.getString("identifier"), TelemetryContract.Method.ACTIONBAR);
             } else if (event.equals("LightweightTheme:Update")) {
                 ThreadUtils.postToUiThread(new Runnable() {
                     @Override
                     public void run() {
                         mDynamicToolbar.setVisible(true, VisibilityTransition.ANIMATE);
                     }
                 });
             } else if (event.equals("Prompt:ShowTop")) {
@@ -2385,52 +2386,40 @@ public class BrowserApp extends GeckoApp
                 // If there isn't a bookmark keyword, load the url. This may result in a query
                 // using the default search engine.
                 if (TextUtils.isEmpty(keywordUrl)) {
                     Tabs.getInstance().loadUrl(url, Tabs.LOADURL_USER_ENTERED);
                     Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.ACTIONBAR, "user");
                     return;
                 }
 
-                recordSearch(null, "barkeyword");
-
                 // Otherwise, construct a search query from the bookmark keyword.
                 // Replace lower case bookmark keywords with URLencoded search query or
                 // replace upper case bookmark keywords with un-encoded search query.
                 // This makes it match the same behaviour as on Firefox for the desktop.
                 final String searchUrl = keywordUrl.replace("%s", URLEncoder.encode(keywordSearch)).replace("%S", keywordSearch);
 
                 Tabs.getInstance().loadUrl(searchUrl, Tabs.LOADURL_USER_ENTERED);
                 Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL,
                                       TelemetryContract.Method.ACTIONBAR,
                                       "keyword");
             }
         });
     }
 
     /**
-     * Record in Health Report that a search has occurred.
+     * Records in telemetry that a search has occurred.
      *
-     * @param engine
-     *        a search engine instance. Can be null.
-     * @param where
-     *        where the search was initialized; one of the values in
-     *        {@link BrowserHealthRecorder#SEARCH_LOCATIONS}.
+     * @param where where the search was started from.
      */
-    private static void recordSearch(SearchEngine engine, String where) {
-        //try {
-        //    String identifier = (engine == null) ? "other" : engine.getEngineIdentifier();
-        //    JSONObject message = new JSONObject();
-        //    message.put("type", BrowserHealthRecorder.EVENT_SEARCH);
-        //    message.put("location", where);
-        //    message.put("identifier", identifier);
-        //    EventDispatcher.getInstance().dispatchEvent(message, null);
-        //} catch (Exception e) {
-        //    Log.e(LOGTAG, "Error recording search.", e);
-        //}
+    private static void recordSearch(@NonNull final String engineIdentifier,
+            @NonNull final TelemetryContract.Method where) {
+        // We could include the engine identifier as an extra but we'll
+        // just capture that with core ping telemetry (bug 1253319).
+        Telemetry.sendUIEvent(TelemetryContract.Event.SEARCH, where);
     }
 
     /**
      * Store search query in SearchHistoryProvider.
      *
      * @param query
      *        a search query to store. We won't store empty queries.
      */
@@ -3869,24 +3858,24 @@ public class BrowserApp extends GeckoApp
                 getResources().getString(R.string.new_tab_opened);
         final String buttonMessage = getResources().getString(R.string.switch_button_message);
 
         SnackbarHelper.showSnackbarWithAction(this, message, Snackbar.LENGTH_LONG, buttonMessage, callback);
     }
 
     // BrowserSearch.OnSearchListener
     @Override
-    public void onSearch(SearchEngine engine, String text) {
+    public void onSearch(SearchEngine engine, final String text, final TelemetryContract.Method method) {
         // Don't store searches that happen in private tabs. This assumes the user can only
         // perform a search inside the currently selected tab, which is true for searches
         // that come from SearchEngineRow.
         if (!Tabs.getInstance().getSelectedTab().isPrivate()) {
             storeSearchQuery(text);
         }
-        recordSearch(engine, "barsuggest");
+        recordSearch(engine.getEngineIdentifier(), method);
         openUrlAndStopEditing(text, engine.name);
     }
 
     // BrowserSearch.OnEditSuggestionListener
     @Override
     public void onEditSuggestion(String suggestion) {
         mBrowserToolbar.onEditSuggestion(suggestion);
     }
--- a/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java
@@ -181,17 +181,17 @@ public class BrowserSearch extends HomeF
 
     // Whether the suggestions will fade in when shown
     private boolean mAnimateSuggestions;
 
     // Opt-in prompt view for search suggestions
     private View mSuggestionsOptInPrompt;
 
     public interface OnSearchListener {
-        public void onSearch(SearchEngine engine, String text);
+        void onSearch(SearchEngine engine, String text, TelemetryContract.Method method);
     }
 
     public interface OnEditSuggestionListener {
         public void onEditSuggestion(String suggestion);
     }
 
     public static BrowserSearch newInstance() {
         BrowserSearch browserSearch = new BrowserSearch();
@@ -724,20 +724,19 @@ public class BrowserSearch extends HomeF
             mSearchEngineBar.setVisibility(View.VISIBLE);
         } else {
             mSearchEngineBar.setVisibility(View.GONE);
         }
     }
 
     @Override
     public void onSearchBarClickListener(final SearchEngine searchEngine) {
-        Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.LIST_ITEM,
-                "searchenginebar");
-
-        mSearchListener.onSearch(searchEngine, mSearchTerm);
+        final TelemetryContract.Method method = TelemetryContract.Method.LIST_ITEM;
+        Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, method, "searchenginebar");
+        mSearchListener.onSearch(searchEngine, mSearchTerm, method);
     }
 
     private void ensureSuggestClientIsSet(final String suggestTemplate) {
         // Don't update the suggestClient if we already have a client with the correct template
         if (mSuggestClient != null && suggestTemplate.equals(mSuggestClient.getSuggestTemplate())) {
             return;
         }
 
--- a/mobile/android/base/java/org/mozilla/gecko/home/SearchEngineRow.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/SearchEngineRow.java
@@ -111,17 +111,17 @@ class SearchEngineRow extends AnimatedHe
                         mUrlOpenListener.onUrlOpen(suggestion, EnumSet.noneOf(OnUrlOpenListener.Flags.class));
                     }
                 } else if (mSearchListener != null) {
                     if (v == mUserEnteredView) {
                         Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.SUGGESTION, "user");
                     } else {
                         Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.SUGGESTION, (String) v.getTag());
                     }
-                    mSearchListener.onSearch(mSearchEngine, suggestion);
+                    mSearchListener.onSearch(mSearchEngine, suggestion, TelemetryContract.Method.SUGGESTION);
                 }
             }
         };
 
         mLongClickListener = new OnLongClickListener() {
             @Override
             public boolean onLongClick(View v) {
                 if (mEditSuggestionListener != null) {
@@ -236,17 +236,17 @@ class SearchEngineRow extends AnimatedHe
 
     /**
      * Perform a search for the user-entered term.
      */
     public void performUserEnteredSearch() {
         String searchTerm = getSuggestionTextFromView(mUserEnteredView);
         if (mSearchListener != null) {
             Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.SUGGESTION, "user");
-            mSearchListener.onSearch(mSearchEngine, searchTerm);
+            mSearchListener.onSearch(mSearchEngine, searchTerm, TelemetryContract.Method.SUGGESTION);
         }
     }
 
     public void setSearchTerm(String searchTerm) {
         mUserEnteredTextView.setText(searchTerm);
 
         // mSearchEngine is not set in the first call to this method; the content description
         // is instead initially set in updateSuggestions().