Commit 757e8982 authored by Jinsuk Kim's avatar Jinsuk Kim Committed by Commit Bot

Tidy up null checking against WebContents

WebContentsUserData-backed objects requires its WebContents instance from
CVC be non-null, which causes numerous null checking statments. This CL
examines callsites to the methods that contains accesses to those
objects, adds null-checking statements if missing, and removes it
if redundant.

Some observations the changes are based on:

- private native method doesn't need null check; WebContents is
    certainly valid at that point.
- CVC.destroy() itself or calls from it (before it nulls out WebContents)
    doesn't need null check since WebContent is valid up to that point.

Added null check:
   ... since clusterfuzz results warn that these need it:
    - setTouchScrollInProgress
    - destroyPastePopup

   ... by moving it over from the 2 callsites:
    - updateTextSelectionUI (onAttachedToWindow, onDetachedFromWindow)

Removed null check as callsites already have null checking/assert,
      or assure WebContents is not null:
 - hidePopupsAndClearSelection (WebContentObserver.resetPopupsAndInput,
                                onFocusChanged, showSelectPopup)
 - hidePopupsAndPreserveSelection (destroy, onHide, updateTextSelectionUI,
                                   onFocusChanged, onRotationChanged)
 - hidePopups (above 2)
 - onAttachedToWindow, onDetachedFromWindow: moved to updateTextSelectionUI
 - onRotationChanged: consolidated null checks

Inlined one-liner restoreSelectionPopupsIfNecessary into callsites
 to eliminate the concern on null check on it (much clearer when inlined).

Bug: 810179
Change-Id: I55aab2b1f7917a9af4522a1bcf8d3e7d9b172f2a
Reviewed-on: https://chromium-review.googlesource.com/907770
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: default avatarBo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536016}
parent aa854d18
...@@ -527,7 +527,7 @@ public class ContentViewCoreImpl ...@@ -527,7 +527,7 @@ public class ContentViewCoreImpl
assert mWebContents != null; assert mWebContents != null;
mWebContents.onShow(); mWebContents.onShow();
getWebContentsAccessibility().refreshState(); getWebContentsAccessibility().refreshState();
restoreSelectionPopupsIfNecessary(); getSelectionPopupController().restoreSelectionPopupsIfNecessary();
} }
@Override @Override
...@@ -543,34 +543,24 @@ public class ContentViewCoreImpl ...@@ -543,34 +543,24 @@ public class ContentViewCoreImpl
} }
private void hidePopupsAndClearSelection() { private void hidePopupsAndClearSelection() {
if (mWebContents != null) { getSelectionPopupController().destroyActionModeAndUnselect();
getSelectionPopupController().destroyActionModeAndUnselect(); mWebContents.dismissTextHandles();
mWebContents.dismissTextHandles();
}
hidePopups(); hidePopups();
} }
@CalledByNative @CalledByNative
private void hidePopupsAndPreserveSelection() { private void hidePopupsAndPreserveSelection() {
if (mWebContents != null) { getSelectionPopupController().destroyActionModeAndKeepSelection();
getSelectionPopupController().destroyActionModeAndKeepSelection();
}
hidePopups(); hidePopups();
} }
private void hidePopups() { private void hidePopups() {
if (mWebContents != null) { destroyPastePopup();
destroyPastePopup(); getTapDisambiguator().hidePopup(false);
getTapDisambiguator().hidePopup(false); getTextSuggestionHost().hidePopups();
getTextSuggestionHost().hidePopups();
}
hideSelectPopupWithCancelMessage(); hideSelectPopupWithCancelMessage();
} }
private void restoreSelectionPopupsIfNecessary() {
getSelectionPopupController().restoreSelectionPopupsIfNecessary();
}
private void resetGestureDetection() { private void resetGestureDetection() {
if (mNativeContentViewCore == 0) return; if (mNativeContentViewCore == 0) return;
nativeResetGestureDetection(mNativeContentViewCore); nativeResetGestureDetection(mNativeContentViewCore);
...@@ -587,18 +577,18 @@ public class ContentViewCoreImpl ...@@ -587,18 +577,18 @@ public class ContentViewCoreImpl
mAttachedToWindow = true; mAttachedToWindow = true;
for (WindowEventObserver observer : mWindowEventObservers) observer.onAttachedToWindow(); for (WindowEventObserver observer : mWindowEventObservers) observer.onAttachedToWindow();
addDisplayAndroidObserverIfNeeded(); addDisplayAndroidObserverIfNeeded();
if (mWebContents != null) { updateTextSelectionUI(true);
updateTextSelectionUI(true);
}
GamepadList.onAttachedToWindow(mContext); GamepadList.onAttachedToWindow(mContext);
mSystemCaptioningBridge.addListener(this); mSystemCaptioningBridge.addListener(this);
} }
@Override @Override
public void updateTextSelectionUI(boolean focused) { public void updateTextSelectionUI(boolean focused) {
// TODO(jinsukkim): Replace all the null checks against mWebContents to assert stmt.
if (mWebContents == null) return;
setTextHandlesTemporarilyHidden(!focused); setTextHandlesTemporarilyHidden(!focused);
if (focused) { if (focused) {
restoreSelectionPopupsIfNecessary(); getSelectionPopupController().restoreSelectionPopupsIfNecessary();
} else { } else {
hidePopupsAndPreserveSelection(); hidePopupsAndPreserveSelection();
} }
...@@ -613,14 +603,12 @@ public class ContentViewCoreImpl ...@@ -613,14 +603,12 @@ public class ContentViewCoreImpl
removeDisplayAndroidObserver(); removeDisplayAndroidObserver();
GamepadList.onDetachedFromWindow(); GamepadList.onDetachedFromWindow();
if (mWebContents != null) { // WebView uses PopupWindows for handle rendering, which may remain
// WebView uses PopupWindows for handle rendering, which may remain // unintentionally visible even after the WebView has been detached.
// unintentionally visible even after the WebView has been detached. // Override the handle visibility explicitly to address this, but
// Override the handle visibility explicitly to address this, but // preserve the underlying selection for detachment cases like screen
// preserve the underlying selection for detachment cases like screen // locking and app switching.
// locking and app switching. updateTextSelectionUI(false);
updateTextSelectionUI(false);
}
mSystemCaptioningBridge.removeListener(this); mSystemCaptioningBridge.removeListener(this);
} }
...@@ -688,7 +676,7 @@ public class ContentViewCoreImpl ...@@ -688,7 +676,7 @@ public class ContentViewCoreImpl
gainFocus && !getSelectionPopupController().isFocusedNodeEditable(); gainFocus && !getSelectionPopupController().isFocusedNodeEditable();
if (gainFocus) { if (gainFocus) {
restoreSelectionPopupsIfNecessary(); getSelectionPopupController().restoreSelectionPopupsIfNecessary();
} else { } else {
cancelRequestToScrollFocusedEditableNodeIntoView(); cancelRequestToScrollFocusedEditableNodeIntoView();
if (mPreserveSelectionOnNextLossOfFocus) { if (mPreserveSelectionOnNextLossOfFocus) {
...@@ -1113,14 +1101,14 @@ public class ContentViewCoreImpl ...@@ -1113,14 +1101,14 @@ public class ContentViewCoreImpl
// ActionMode#invalidate() won't be able to re-layout the floating // ActionMode#invalidate() won't be able to re-layout the floating
// action mode menu items according to the new rotation. So Chrome // action mode menu items according to the new rotation. So Chrome
// has to re-create the action mode. // has to re-create the action mode.
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M if (mWebContents != null) {
&& getSelectionPopupController().isActionModeValid()) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M
hidePopupsAndPreserveSelection(); && getSelectionPopupController().isActionModeValid()) {
if (mWebContents != null) { hidePopupsAndPreserveSelection();
getSelectionPopupController().showActionModeOrClearOnFailure(); getSelectionPopupController().showActionModeOrClearOnFailure();
} }
getTextSuggestionHost().hidePopups();
} }
if (mWebContents != null) getTextSuggestionHost().hidePopups();
int rotationDegrees = 0; int rotationDegrees = 0;
switch (rotation) { switch (rotation) {
......
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