diff options
author | Ben Murdoch <benm@google.com> | 2014-11-13 22:56:35 +0000 |
---|---|---|
committer | Ben Murdoch <benm@google.com> | 2014-11-13 22:56:35 +0000 |
commit | d9512fb2dc45a909a2812842735be59e78b5171d (patch) | |
tree | b9ce47ff9e391d15f992b904889d80779da64442 /content | |
parent | 3eaa0a6864d789c352be4a8216f2797a475e01e0 (diff) | |
download | chromium_org-d9512fb2dc45a909a2812842735be59e78b5171d.tar.gz |
Cherry pick [Android] Override text handle visibility when the view is
detached
Bug: 18373995
Original Description:
PopupWindows are used to render text handles in WebView. As these popups
are drawn outside of the web contents View hierarchy, it's possible that
their visibility becomes unsynchronized with the WebView itself. For
example, after the WebView has been detached from the window but
before it has been properly disposed of. Address this by overriding
handle visibility when the (content) View is detached, restoring it
only after re-attachment.
BUG=430859
(internal b/18066784)
Change-Id: I43b5ff63368384c3ff30ceece7ddd86aa0a4b738
Committed: https://crrev.com/0563c2785d0b1e62793ffb0f9dc3ac178559aeaa
Cr-Commit-Position: refs/heads/master@{#303987}
Diffstat (limited to 'content')
8 files changed, 93 insertions, 19 deletions
diff --git a/content/browser/android/content_view_core_impl.cc b/content/browser/android/content_view_core_impl.cc index 9cd9679d40..fece49f90f 100644 --- a/content/browser/android/content_view_core_impl.cc +++ b/content/browser/android/content_view_core_impl.cc @@ -1077,15 +1077,23 @@ void ContentViewCoreImpl::SelectBetweenCoordinates(JNIEnv* env, jobject obj, void ContentViewCoreImpl::MoveCaret(JNIEnv* env, jobject obj, jfloat x, jfloat y) { - if (GetRenderWidgetHostViewAndroid()) { - GetRenderWidgetHostViewAndroid()->MoveCaret( - gfx::Point(x / dpi_scale_, y / dpi_scale_)); - } + RenderWidgetHostViewAndroid* rwhv = GetRenderWidgetHostViewAndroid(); + if (rwhv) + rwhv->MoveCaret(gfx::Point(x / dpi_scale_, y / dpi_scale_)); } -void ContentViewCoreImpl::HideTextHandles(JNIEnv* env, jobject obj) { - if (GetRenderWidgetHostViewAndroid()) - GetRenderWidgetHostViewAndroid()->HideTextHandles(); +void ContentViewCoreImpl::DismissTextHandles(JNIEnv* env, jobject obj) { + RenderWidgetHostViewAndroid* rwhv = GetRenderWidgetHostViewAndroid(); + if (rwhv) + rwhv->DismissTextHandles(); +} + +void ContentViewCoreImpl::SetTextHandlesTemporarilyHidden(JNIEnv* env, + jobject obj, + jboolean hidden) { + RenderWidgetHostViewAndroid* rwhv = GetRenderWidgetHostViewAndroid(); + if (rwhv) + rwhv->SetTextHandlesTemporarilyHidden(hidden); } void ContentViewCoreImpl::ResetGestureDetection(JNIEnv* env, jobject obj) { diff --git a/content/browser/android/content_view_core_impl.h b/content/browser/android/content_view_core_impl.h index 9887c0e404..d7f2711c10 100644 --- a/content/browser/android/content_view_core_impl.h +++ b/content/browser/android/content_view_core_impl.h @@ -143,7 +143,10 @@ class ContentViewCoreImpl : public ContentViewCore, jfloat x1, jfloat y1, jfloat x2, jfloat y2); void MoveCaret(JNIEnv* env, jobject obj, jfloat x, jfloat y); - void HideTextHandles(JNIEnv* env, jobject obj); + void DismissTextHandles(JNIEnv* env, jobject obj); + void SetTextHandlesTemporarilyHidden(JNIEnv* env, + jobject obj, + jboolean hidden); void ResetGestureDetection(JNIEnv* env, jobject obj); void SetDoubleTapSupportEnabled(JNIEnv* env, jobject obj, jboolean enabled); diff --git a/content/browser/renderer_host/input/touch_selection_controller.cc b/content/browser/renderer_host/input/touch_selection_controller.cc index d57b96f0aa..e904e283ee 100644 --- a/content/browser/renderer_host/input/touch_selection_controller.cc +++ b/content/browser/renderer_host/input/touch_selection_controller.cc @@ -160,6 +160,20 @@ void TouchSelectionController::HideAndDisallowShowingAutomatically() { activate_selection_automatically_ = false; } +void TouchSelectionController::SetTemporarilyHidden(bool hidden) { + if (temporarily_hidden_ == hidden) + return; + temporarily_hidden_ = hidden; + + TouchHandle::AnimationStyle animation_style = GetAnimationStyle(true); + if (is_selection_active_) { + start_selection_handle_->SetVisible(GetStartVisible(), animation_style); + end_selection_handle_->SetVisible(GetEndVisible(), animation_style); + } + if (is_insertion_active_) + insertion_handle_->SetVisible(GetStartVisible(), animation_style); +} + void TouchSelectionController::OnSelectionEditable(bool editable) { if (selection_editable_ == editable) return; diff --git a/content/browser/renderer_host/input/touch_selection_controller.h b/content/browser/renderer_host/input/touch_selection_controller.h index 452dde0cc3..13031e12b9 100644 --- a/content/browser/renderer_host/input/touch_selection_controller.h +++ b/content/browser/renderer_host/input/touch_selection_controller.h @@ -69,6 +69,9 @@ class CONTENT_EXPORT TouchSelectionController : public TouchHandleClient { // showing allowance. void HideAndDisallowShowingAutomatically(); + // Override the handle visibility according to |hidden|. + void SetTemporarilyHidden(bool hidden); + // To be called when the editability of the focused region changes. void OnSelectionEditable(bool editable); diff --git a/content/browser/renderer_host/input/touch_selection_controller_unittest.cc b/content/browser/renderer_host/input/touch_selection_controller_unittest.cc index c3422077dd..5bdf57273f 100644 --- a/content/browser/renderer_host/input/touch_selection_controller_unittest.cc +++ b/content/browser/renderer_host/input/touch_selection_controller_unittest.cc @@ -632,6 +632,31 @@ TEST_F(TouchSelectionControllerTest, Animation) { EXPECT_FALSE(GetAndResetNeedsAnimate()); } +TEST_F(TouchSelectionControllerTest, TemporarilyHidden) { + controller().OnTapEvent(); + controller().OnSelectionEditable(true); + + gfx::RectF insertion_rect(5, 5, 0, 10); + + bool visible = true; + ChangeInsertion(insertion_rect, visible); + EXPECT_FALSE(GetAndResetNeedsAnimate()); + + controller().SetTemporarilyHidden(true); + EXPECT_TRUE(GetAndResetNeedsAnimate()); + + visible = false; + ChangeInsertion(insertion_rect, visible); + EXPECT_FALSE(GetAndResetNeedsAnimate()); + + visible = true; + ChangeInsertion(insertion_rect, visible); + EXPECT_FALSE(GetAndResetNeedsAnimate()); + + controller().SetTemporarilyHidden(false); + EXPECT_TRUE(GetAndResetNeedsAnimate()); +} + TEST_F(TouchSelectionControllerTest, SelectionClearOnTap) { gfx::RectF start_rect(5, 5, 0, 10); gfx::RectF end_rect(50, 5, 0, 10); diff --git a/content/browser/renderer_host/render_widget_host_view_android.cc b/content/browser/renderer_host/render_widget_host_view_android.cc index 66c9fcdd74..a20a7da14e 100644 --- a/content/browser/renderer_host/render_widget_host_view_android.cc +++ b/content/browser/renderer_host/render_widget_host_view_android.cc @@ -1523,11 +1523,16 @@ void RenderWidgetHostViewAndroid::MoveCaret(const gfx::Point& point) { host_->MoveCaret(point); } -void RenderWidgetHostViewAndroid::HideTextHandles() { +void RenderWidgetHostViewAndroid::DismissTextHandles() { if (selection_controller_) selection_controller_->HideAndDisallowShowingAutomatically(); } +void RenderWidgetHostViewAndroid::SetTextHandlesTemporarilyHidden(bool hidden) { + if (selection_controller_) + selection_controller_->SetTemporarilyHidden(hidden); +} + void RenderWidgetHostViewAndroid::OnShowingPastePopup( const gfx::PointF& point) { if (!selection_controller_) @@ -1542,7 +1547,7 @@ void RenderWidgetHostViewAndroid::OnShowingPastePopup( insertion_bound.visible = true; insertion_bound.edge_top = point; insertion_bound.edge_bottom = point; - HideTextHandles(); + DismissTextHandles(); ShowSelectionHandlesAutomatically(); selection_controller_->OnSelectionEditable(true); selection_controller_->OnSelectionEmpty(true); diff --git a/content/browser/renderer_host/render_widget_host_view_android.h b/content/browser/renderer_host/render_widget_host_view_android.h index 5393856a92..22bb69446c 100644 --- a/content/browser/renderer_host/render_widget_host_view_android.h +++ b/content/browser/renderer_host/render_widget_host_view_android.h @@ -259,7 +259,8 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid bool HasValidFrame() const; void MoveCaret(const gfx::Point& point); - void HideTextHandles(); + void DismissTextHandles(); + void SetTextHandlesTemporarilyHidden(bool hidden); void OnShowingPastePopup(const gfx::PointF& point); void SynchronousFrameMetadata( diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java index 234bc47654..196f83d9de 100644 --- a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java +++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java @@ -545,7 +545,7 @@ public class ContentViewCore public void onImeEvent() { mPopupZoomer.hide(true); getContentViewClient().onImeEvent(); - if (mFocusedNodeEditable) hideTextHandles(); + if (mFocusedNodeEditable) dismissTextHandles(); } @Override @@ -1316,7 +1316,7 @@ public class ContentViewCore hidePastePopup(); hideSelectPopup(); mPopupZoomer.hide(false); - if (mUnselectAllOnActionModeDismiss) hideTextHandles(); + if (mUnselectAllOnActionModeDismiss) dismissTextHandles(); } private void restoreSelectionPopupsIfNecessary() { @@ -1345,6 +1345,7 @@ public class ContentViewCore @SuppressWarnings("javadoc") public void onAttachedToWindow() { setAccessibilityState(mAccessibilityManager.isEnabled()); + setTextHandlesTemporarilyHidden(false); restoreSelectionPopupsIfNecessary(); ScreenOrientationListener.getInstance().addObserver(this, mContext); GamepadList.onAttachedToWindow(mContext); @@ -1357,12 +1358,19 @@ public class ContentViewCore @SuppressLint("MissingSuperCall") public void onDetachedFromWindow() { setInjectedAccessibility(false); - hidePopupsAndPreserveSelection(); mZoomControlsDelegate.dismissZoomPicker(); unregisterAccessibilityContentObserver(); ScreenOrientationListener.getInstance().removeObserver(this); GamepadList.onDetachedFromWindow(); + + // WebView uses PopupWindows for handle rendering, which may remain + // unintentionally visible even after the WebView has been detached. + // Override the handle visibility explicitly to address this, but + // preserve the underlying selection for detachment cases like screen + // locking and app switching. + setTextHandlesTemporarilyHidden(true); + hidePopupsAndPreserveSelection(); } /** @@ -1923,7 +1931,7 @@ public class ContentViewCore public void onDestroyActionMode() { mActionMode = null; if (mUnselectAllOnActionModeDismiss) { - hideTextHandles(); + dismissTextHandles(); clearUserSelection(); } getContentViewClient().onContextualActionBarHidden(); @@ -2053,10 +2061,15 @@ public class ContentViewCore getContentViewClient().onSelectionEvent(eventType, posXDip * scale, posYDip * scale); } - private void hideTextHandles() { + private void dismissTextHandles() { mHasSelection = false; mHasInsertion = false; - if (mNativeContentViewCore != 0) nativeHideTextHandles(mNativeContentViewCore); + if (mNativeContentViewCore != 0) nativeDismissTextHandles(mNativeContentViewCore); + } + + private void setTextHandlesTemporarilyHidden(boolean hide) { + if (mNativeContentViewCore == 0) return; + nativeSetTextHandlesTemporarilyHidden(mNativeContentViewCore, hide); } /** @@ -2306,7 +2319,7 @@ public class ContentViewCore @Override public void paste() { mImeAdapter.paste(); - hideTextHandles(); + dismissTextHandles(); } }); } @@ -3019,7 +3032,9 @@ public class ContentViewCore private native void nativeMoveCaret(long nativeContentViewCoreImpl, float x, float y); - private native void nativeHideTextHandles(long nativeContentViewCoreImpl); + private native void nativeDismissTextHandles(long nativeContentViewCoreImpl); + private native void nativeSetTextHandlesTemporarilyHidden( + long nativeContentViewCoreImpl, boolean hidden); private native void nativeResetGestureDetection(long nativeContentViewCoreImpl); |