Commit ecef27c6 authored by Matthew Jones's avatar Matthew Jones Committed by Commit Bot

Move ContextualSearchObserver for ContextReporter

This patch delegates the task of reporting context to GSA to
Contextual Search. This is being done in preparation to make the
ContextualSearchObserver package protected since its use is extremely
feature specific.

Bug: 878006
Change-Id: I340a6196210a4709cebef03e477e241be1633bb4
Reviewed-on: https://chromium-review.googlesource.com/1198003Reviewed-by: default avatarTheresa <twellington@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587838}
parent 001bd26b
...@@ -71,6 +71,15 @@ import java.util.regex.Pattern; ...@@ -71,6 +71,15 @@ import java.util.regex.Pattern;
public class ContextualSearchManager public class ContextualSearchManager
implements ContextualSearchManagementDelegate, ContextualSearchTranslateInterface, implements ContextualSearchManagementDelegate, ContextualSearchTranslateInterface,
ContextualSearchNetworkCommunicator, ContextualSearchSelectionHandler { ContextualSearchNetworkCommunicator, ContextualSearchSelectionHandler {
/** A delegate for reporting selected context to GSA for search quality. */
public interface ContextReporterDelegate {
/**
* Reports that the given display selection has been established for the current tab.
* @param displaySelection The information about the selection being displayed.
*/
void reportDisplaySelection(@Nullable GSAContextDisplaySelection displaySelection);
}
// TODO(donnd): provide an inner class that implements some of these interfaces (like the // TODO(donnd): provide an inner class that implements some of these interfaces (like the
// ContextualSearchTranslateInterface) rather than having the manager itself implement the // ContextualSearchTranslateInterface) rather than having the manager itself implement the
// interface because that exposes all the public methods of that interface at the manager level. // interface because that exposes all the public methods of that interface at the manager level.
...@@ -187,6 +196,9 @@ public class ContextualSearchManager ...@@ -187,6 +196,9 @@ public class ContextualSearchManager
/** Whether ContextualSearch UI is suppressed for Smart Selection. */ /** Whether ContextualSearch UI is suppressed for Smart Selection. */
private boolean mDoSuppressContextualSearchForSmartSelection; private boolean mDoSuppressContextualSearchForSmartSelection;
/** An observer that reports selected context to GSA for search quality. */
private ContextualSearchObserver mContextReportingObserver;
/** /**
* The delegate that is responsible for promoting a {@link WebContents} to a {@link Tab} * The delegate that is responsible for promoting a {@link WebContents} to a {@link Tab}
* when necessary. * when necessary.
...@@ -1714,6 +1726,33 @@ public class ContextualSearchManager ...@@ -1714,6 +1726,33 @@ public class ContextualSearchManager
}; };
} }
/**
* @param reporter A context reporter for the feature to report the current selection when
* triggered.
*/
public void enableContextReporting(ContextReporterDelegate reporter) {
mContextReportingObserver = new ContextualSearchObserver() {
@Override
public void onShowContextualSearch(GSAContextDisplaySelection contextSelection) {
if (contextSelection != null) reporter.reportDisplaySelection(contextSelection);
}
@Override
public void onHideContextualSearch() {
reporter.reportDisplaySelection(null);
}
};
addObserver(mContextReportingObserver);
}
/**
* Disable context reporting for Contextual Search.
*/
public void disableContextReporting() {
removeObserver(mContextReportingObserver);
mContextReportingObserver = null;
}
// ============================================================================================ // ============================================================================================
// Test helpers // Test helpers
// ============================================================================================ // ============================================================================================
......
...@@ -10,7 +10,7 @@ import org.chromium.base.Log; ...@@ -10,7 +10,7 @@ import org.chromium.base.Log;
import org.chromium.base.metrics.RecordHistogram; import org.chromium.base.metrics.RecordHistogram;
import org.chromium.chrome.browser.ChromeActivity; import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.UrlConstants; import org.chromium.chrome.browser.UrlConstants;
import org.chromium.chrome.browser.contextualsearch.ContextualSearchObserver; import org.chromium.chrome.browser.contextualsearch.ContextualSearchManager;
import org.chromium.chrome.browser.sync.ProfileSyncService; import org.chromium.chrome.browser.sync.ProfileSyncService;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.TabModel.TabSelectionType; import org.chromium.chrome.browser.tabmodel.TabModel.TabSelectionType;
...@@ -59,7 +59,6 @@ public class ContextReporter { ...@@ -59,7 +59,6 @@ public class ContextReporter {
private final GSAContextReportDelegate mDelegate; private final GSAContextReportDelegate mDelegate;
private TabModelSelectorTabObserver mSelectorTabObserver; private TabModelSelectorTabObserver mSelectorTabObserver;
private TabModelSelectorTabModelObserver mModelObserver; private TabModelSelectorTabModelObserver mModelObserver;
private ContextualSearchObserver mContextualSearchObserver;
private boolean mLastContextWasTitleChange; private boolean mLastContextWasTitleChange;
private String mLastUrl; private String mLastUrl;
private String mLastTitle; private String mLastTitle;
...@@ -110,19 +109,10 @@ public class ContextReporter { ...@@ -110,19 +109,10 @@ public class ContextReporter {
} }
}; };
} }
if (mContextualSearchObserver == null && mActivity.getContextualSearchManager() != null) { ContextualSearchManager manager = mActivity.getContextualSearchManager();
mContextualSearchObserver = new ContextualSearchObserver() { if (manager != null) {
@Override manager.enableContextReporting(
public void onShowContextualSearch(GSAContextDisplaySelection contextSelection) { (selection) -> ContextReporter.this.reportDisplaySelection(selection));
if (contextSelection != null) reportDisplaySelection(contextSelection);
}
@Override
public void onHideContextualSearch() {
reportDisplaySelection(null);
}
};
mActivity.getContextualSearchManager().addObserver(mContextualSearchObserver);
} }
} }
...@@ -140,9 +130,8 @@ public class ContextReporter { ...@@ -140,9 +130,8 @@ public class ContextReporter {
mModelObserver.destroy(); mModelObserver.destroy();
mModelObserver = null; mModelObserver = null;
} }
if (mContextualSearchObserver != null && mActivity.getContextualSearchManager() != null) { if (mActivity.getContextualSearchManager() != null) {
mActivity.getContextualSearchManager().removeObserver(mContextualSearchObserver); mActivity.getContextualSearchManager().disableContextReporting();
mContextualSearchObserver = null;
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment