Skip to content

Commit

Permalink
[TTS] reland mojo-ify ShowUnhandleTapUIIfNeeded.
Browse files Browse the repository at this point in the history
Replaces the whole call chain for ShowUnhandledTapUIIfNeeded
between Blink and Chrome/WebView with Mojo messaging.

Removes the Blink WebTappedInfo class in favor of a new UnhandledTapInfo
that sends a Mojo construct of the same name to the Browser which
has a service installed via a RenderHostFrame observer.

All functionality should remain unchanged.

NEW CODE:
The CSTabHelper now creates an UnhandledTapWebContentsObserver that
connects the Mojo service via UnhandledTapNotifierImpl.  That Impl
calls back through Java to the CSTabHelper to the CSManager to notify
that a Tap has occurred.  The CSTabHelper ignores these notifications
when CS is disabled.

The mojo message is only sent to the browser if the tap is unhandled
and the other required conditions are met.

UPDATED TESTS:
Test notification is now done through mojo messaging, so we now only
know if the tap was unhandled, not whether the page changed or other
details about why the tap was unhandled.

Updated the test page by adding nodes that do not trigger, and removed
the mousemove handler since it's unrelated to tap handling.

BUG=754862

TBR=dcheng@chromium, yfriedman@chromium.org, kinuko@chromium.org

Original CL:

Reviewed-on: https://chromium-review.googlesource.com/841544
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Change-Id: I824837419cd4116b26b32170009e7a7c3308d109
Reviewed-on: https://chromium-review.googlesource.com/952679
Commit-Queue: Donn Denman <donnd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541569}
  • Loading branch information
Donn Denman authored and Commit Bot committed Mar 7, 2018
1 parent a7596aa commit ba4d1c4
Show file tree
Hide file tree
Showing 45 changed files with 513 additions and 369 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1292,13 +1292,6 @@ public void onSelectionEvent(int eventType, float posXPix, float posYPix) {
}
}

@Override
public void showUnhandledTapUIIfNeeded(final int x, final int y) {
if (!isOverlayVideoMode()) {
mSelectionController.handleShowUnhandledTapUIIfNeeded(x, y);
}
}

@Override
public void selectWordAroundCaretAck(boolean didSelect, int startAdjust, int endAdjust) {
if (mSelectWordAroundCaretCounter > 0) mSelectWordAroundCaretCounter--;
Expand Down Expand Up @@ -1327,6 +1320,13 @@ public boolean requestSelectionPopupUpdates(boolean shouldSuggest) {
public void cancelAllRequests() {}
}

/** Shows the Unhandled Tap UI. Called by {@link ContextualSearchTabHelper}. */
void onShowUnhandledTapUIIfNeeded(int x, int y) {
if (!isOverlayVideoMode()) {
mSelectionController.handleShowUnhandledTapUIIfNeeded(x, y);
}
}

/**
* @return Whether the display is in a full-screen video overlay mode.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ public class ContextualSearchTabHelper
/** The Tab that this helper tracks. */
private final Tab mTab;

// Device scale factor.
private final float mPxToDp;

/** Notification handler for Contextual Search events. */
private TemplateUrlServiceObserver mTemplateUrlObserver;

Expand Down Expand Up @@ -73,6 +76,11 @@ private ContextualSearchTabHelper(Tab tab) {
if (NetworkChangeNotifier.isInitialized()) {
NetworkChangeNotifier.addConnectionTypeObserver(this);
}
float scaleFactor = 1.f;
if (tab != null && tab.getActivity() != null && tab.getActivity().getResources() != null) {
scaleFactor /= tab.getActivity().getResources().getDisplayMetrics().density;
}
mPxToDp = scaleFactor;
}

// ============================================================================================
Expand Down Expand Up @@ -219,6 +227,7 @@ private void addContextualSearchHooks(WebContents webContents) {
contextualSearchManager.getContextualSearchSelectionClient()));
contextualSearchManager.suppressContextualSearchForSmartSelection(
mSelectionClientManager.isSmartSelectionEnabledInChrome());
nativeInstallUnhandledTapNotifierIfNeeded(mNativeHelper, webContents, mPxToDp);
}
}

Expand Down Expand Up @@ -303,6 +312,20 @@ void onContextualSearchPrefChanged() {
}
}

/**
* Notifies this helper to show the Unhandled Tap UI due to a tap at the given pixel
* coordinates.
*/
@CalledByNative
void onShowUnhandledTapUIIfNeeded(int x, int y) {
// Only notify the manager if we currently have a valid listener.
if (mGestureStateListener != null && getContextualSearchManager(mTab) != null) {
getContextualSearchManager(mTab).onShowUnhandledTapUIIfNeeded(x, y);
}
}

private native long nativeInit(Profile profile);
private native void nativeInstallUnhandledTapNotifierIfNeeded(
long nativeContextualSearchTabHelper, WebContents webContents, float pxToDpScaleFactor);
private native void nativeDestroy(long nativeContextualSearchTabHelper);
}
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,6 @@ public void onSelectionEvent(int eventType, float posXPix, float posYPix) {
mContextualSearchSelectionClient.onSelectionEvent(eventType, posXPix, posYPix);
}

@Override
public void showUnhandledTapUIIfNeeded(int x, int y) {
mSmartSelectionClient.showUnhandledTapUIIfNeeded(x, y);
mContextualSearchSelectionClient.showUnhandledTapUIIfNeeded(x, y);
}

@Override
public void selectWordAroundCaretAck(boolean didSelect, int startAdjust, int endAdjust) {
mSmartSelectionClient.selectWordAroundCaretAck(didSelect, startAdjust, endAdjust);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ private void mockTapText(String text) {
@Override
public void run() {
mContextualSearchManager.getGestureStateListener().onTouchDown();
mContextualSearchClient.showUnhandledTapUIIfNeeded(0, 0);
mContextualSearchManager.onShowUnhandledTapUIIfNeeded(0, 0);
}
});
}
Expand All @@ -212,7 +212,7 @@ private void mockTapEmptySpace() {
ThreadUtils.runOnUiThreadBlocking(new Runnable() {
@Override
public void run() {
mContextualSearchClient.showUnhandledTapUIIfNeeded(0, 0);
mContextualSearchManager.onShowUnhandledTapUIIfNeeded(0, 0);
mContextualSearchClient.onSelectionEvent(
SelectionEventType.SELECTION_HANDLES_CLEARED, 0, 0);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ public boolean requestSelectionPopupUpdates(boolean shouldSuggest) {
@Override
public void onSelectionEvent(int eventType, float posXPix, float posYPix) {}

@Override
public void showUnhandledTapUIIfNeeded(int x, int y) {}

@Override
public void selectWordAroundCaretAck(boolean didSelect, int startAdjust, int endAdjust) {}

Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1931,6 +1931,10 @@ jumbo_split_static_library("browser") {
"android/contextualsearch/ctr_suppression.h",
"android/contextualsearch/resolved_search_term.cc",
"android/contextualsearch/resolved_search_term.h",
"android/contextualsearch/unhandled_tap_notifier_impl.cc",
"android/contextualsearch/unhandled_tap_notifier_impl.h",
"android/contextualsearch/unhandled_tap_web_contents_observer.cc",
"android/contextualsearch/unhandled_tap_web_contents_observer.h",
"android/cookies/cookies_fetcher_util.cc",
"android/crash/pure_java_exception_handler.cc",
"android/crash/pure_java_exception_handler.h",
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/android/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ include_rules = [
"+sandbox/sandbox_features.h",
"+third_party/gvr-android-sdk",
"+third_party/WebKit/public/platform/media_download_in_product_help.mojom.h",
"+third_party/WebKit/public/platform/unhandled_tap_notifier.mojom.h",
]

specific_include_rules = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "base/time/time.h"
#include "chrome/browser/android/contextualsearch/contextual_search_delegate.h"
#include "chrome/browser/android/contextualsearch/resolved_search_term.h"
#include "chrome/browser/android/tab_android.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "components/contextual_search/browser/contextual_search_js_api_service_impl.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@

#include "base/android/jni_android.h"
#include "base/android/jni_string.h"
#include "build/build_config.h"
#include "chrome/browser/android/contextualsearch/unhandled_tap_web_contents_observer.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_android.h"
#include "chrome/common/pref_names.h"
#include "components/prefs/pref_change_registrar.h"
#include "content/public/browser/web_contents.h"
#include "jni/ContextualSearchTabHelper_jni.h"

using base::android::JavaParamRef;
Expand All @@ -24,8 +27,9 @@ ContextualSearchTabHelper::ContextualSearchTabHelper(JNIEnv* env,
pref_change_registrar_->Init(profile->GetPrefs());
pref_change_registrar_->Add(
prefs::kContextualSearchEnabled,
base::Bind(&ContextualSearchTabHelper::OnContextualSearchPrefChanged,
weak_factory_.GetWeakPtr()));
base::BindRepeating(
&ContextualSearchTabHelper::OnContextualSearchPrefChanged,
weak_factory_.GetWeakPtr()));
}

ContextualSearchTabHelper::~ContextualSearchTabHelper() {
Expand All @@ -38,6 +42,33 @@ void ContextualSearchTabHelper::OnContextualSearchPrefChanged() {
Java_ContextualSearchTabHelper_onContextualSearchPrefChanged(env, jobj);
}

void ContextualSearchTabHelper::OnShowUnhandledTapUIIfNeeded(int x_px,
int y_px) {
JNIEnv* env = base::android::AttachCurrentThread();
ScopedJavaLocalRef<jobject> jobj = weak_java_ref_.get(env);
Java_ContextualSearchTabHelper_onShowUnhandledTapUIIfNeeded(env, jobj, x_px,
y_px);
}

void ContextualSearchTabHelper::InstallUnhandledTapNotifierIfNeeded(
JNIEnv* env,
jobject obj,
const JavaParamRef<jobject>& j_base_web_contents,
jfloat device_scale_factor) {
DCHECK(j_base_web_contents);
content::WebContents* base_web_contents =
content::WebContents::FromJavaWebContents(j_base_web_contents);
DCHECK(base_web_contents);
if (!unhandled_tap_web_contents_observer_) {
unhandled_tap_web_contents_observer_.reset(
new contextual_search::UnhandledTapWebContentsObserver(
base_web_contents, device_scale_factor,
base::BindRepeating(
&ContextualSearchTabHelper::OnShowUnhandledTapUIIfNeeded,
weak_factory_.GetWeakPtr())));
}
}

void ContextualSearchTabHelper::Destroy(JNIEnv* env,
const JavaParamRef<jobject>& obj) {
delete this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,42 @@

class Profile;

namespace contextual_search {
class UnhandledTapWebContentsObserver;
}

class ContextualSearchTabHelper {
public:
ContextualSearchTabHelper(JNIEnv* env, jobject obj, Profile* profile);
void Destroy(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);

// Installs the UnhandledTapNotifier Mojo handler if needed.
// The |j_base_web_contents| is a java WebContents of the base page tab.
// The |device_scale_factor| is the ratio of pixels to dips.
void InstallUnhandledTapNotifierIfNeeded(
JNIEnv* env,
jobject obj,
const base::android::JavaParamRef<jobject>& j_base_web_contents,
jfloat device_scale_factor);

private:
~ContextualSearchTabHelper();

// Methods that call back to Java.
// Call when the preferences change.
void OnContextualSearchPrefChanged();
// Call when an unhandled tap needs to show the UI for a tap at the given
// position.
void OnShowUnhandledTapUIIfNeeded(int x_px, int y_px);

JavaObjectWeakGlobalRef weak_java_ref_;
std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_;

// The unhandled tap WebContentsObserver for the current tab.
// Installs a mojo handler for ShowUnhandledTapUIIfNeeded.
std::unique_ptr<contextual_search::UnhandledTapWebContentsObserver>
unhandled_tap_web_contents_observer_;

base::WeakPtrFactory<ContextualSearchTabHelper> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(ContextualSearchTabHelper);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// 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/android/contextualsearch/unhandled_tap_notifier_impl.h"

#include <utility>

#include "base/memory/ptr_util.h"
#include "content/public/common/use_zoom_for_dsf_policy.h"
#include "mojo/public/cpp/bindings/strong_binding.h"

namespace contextual_search {

UnhandledTapNotifierImpl::UnhandledTapNotifierImpl(
float device_scale_factor,
UnhandledTapCallback callback)
: device_scale_factor_(device_scale_factor),
unhandled_tap_callback_(std::move(callback)) {}

UnhandledTapNotifierImpl::~UnhandledTapNotifierImpl() {}

void UnhandledTapNotifierImpl::ShowUnhandledTapUIIfNeeded(
blink::mojom::UnhandledTapInfoPtr unhandled_tap_info) {
float x_px = unhandled_tap_info->tapped_position_in_viewport.x();
float y_px = unhandled_tap_info->tapped_position_in_viewport.y();

if (!content::IsUseZoomForDSFEnabled()) {
x_px /= device_scale_factor_;
y_px /= device_scale_factor_;
}
// Call back through the callback if possible. (The callback uses a weakptr
// that might make this a NOP).
unhandled_tap_callback_.Run(x_px, y_px);
}

// static
void CreateUnhandledTapNotifierImpl(
float device_scale_factor,
UnhandledTapCallback callback,
blink::mojom::UnhandledTapNotifierRequest request) {
mojo::MakeStrongBinding(std::make_unique<UnhandledTapNotifierImpl>(
device_scale_factor, std::move(callback)),
std::move(request));
}

} // namespace contextual_search
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// 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_ANDROID_CONTEXTUALSEARCH_UNHANDLED_TAP_NOTIFIER_IMPL_H_
#define CHROME_BROWSER_ANDROID_CONTEXTUALSEARCH_UNHANDLED_TAP_NOTIFIER_IMPL_H_

#include "base/macros.h"
#include "chrome/browser/android/contextualsearch/unhandled_tap_web_contents_observer.h"
#include "third_party/WebKit/public/platform/unhandled_tap_notifier.mojom.h"

namespace contextual_search {

// Implements a Mojo service endpoint for the mojo unhandled-tap notifier
// message.
class UnhandledTapNotifierImpl : public blink::mojom::UnhandledTapNotifier {
public:
// Creates an implementation that will scale tap locations by the given
// |scale_factor| (when needed) and call the given |callback| when Mojo
// ShowUnhandledTapUIIfNeeded messages are received for the
// unhandled_tap_notifier service.
UnhandledTapNotifierImpl(float device_scale_factor,
UnhandledTapCallback callback);

~UnhandledTapNotifierImpl() override;

// Mojo UnhandledTapNotifier implementation.
void ShowUnhandledTapUIIfNeeded(
blink::mojom::UnhandledTapInfoPtr unhandled_tap_info) override;

private:
// Scale factor between pixels and DPs.
float device_scale_factor_;

// Callback to call when an unhandled tap notification takes place.
UnhandledTapCallback unhandled_tap_callback_;

DISALLOW_COPY_AND_ASSIGN(UnhandledTapNotifierImpl);
};

// static
void CreateUnhandledTapNotifierImpl(
float device_scale_factor,
UnhandledTapCallback callback,
blink::mojom::UnhandledTapNotifierRequest request);

} // namespace contextual_search

#endif // CHROME_BROWSER_ANDROID_CONTEXTUALSEARCH_UNHANDLED_TAP_NOTIFIER_IMPL_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// 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/android/contextualsearch/unhandled_tap_web_contents_observer.h"

#include <utility>

#include "base/memory/ptr_util.h"
#include "build/build_config.h"
#include "third_party/WebKit/public/public_features.h"

#if BUILDFLAG(ENABLE_UNHANDLED_TAP)
#include "chrome/browser/android/contextualsearch/unhandled_tap_notifier_impl.h"
#endif // BUILDFLAG(ENABLE_UNHANDLED_TAP)

namespace contextual_search {

UnhandledTapWebContentsObserver::UnhandledTapWebContentsObserver(
content::WebContents* web_contents,
float device_scale_factor,
UnhandledTapCallback callback)
: content::WebContentsObserver(web_contents) {
#if BUILDFLAG(ENABLE_UNHANDLED_TAP)
registry_.AddInterface(
base::BindRepeating(&contextual_search::CreateUnhandledTapNotifierImpl,
device_scale_factor, std::move(callback)));
#endif // BUILDFLAG(ENABLE_UNHANDLED_TAP)
}

UnhandledTapWebContentsObserver::~UnhandledTapWebContentsObserver() {}

void UnhandledTapWebContentsObserver::OnInterfaceRequestFromFrame(
content::RenderFrameHost* render_frame_host,
const std::string& interface_name,
mojo::ScopedMessagePipeHandle* interface_pipe) {
#if BUILDFLAG(ENABLE_UNHANDLED_TAP)
registry_.TryBindInterface(interface_name, interface_pipe);
#endif // BUILDFLAG(ENABLE_UNHANDLED_TAP)
}

} // namespace contextual_search
Loading

0 comments on commit ba4d1c4

Please sign in to comment.