Commit 5a639726 authored by Patrick Noland's avatar Patrick Noland Committed by Commit Bot

[ToolbarMVC] Clean up LocationBar destruction and re-creation

Testing a smaller (included) fix for a crash in updateMicButtonState
revealed a few other post-destruction issues that were previously
hidden by LocationBarLayout never being nulled out. So this CL
* Null checks VoiceRecognitionHandler in updateMicButtonState
* Guards against post-destruction callback/delegate invocation in
URLBar
* Prevents a post-destruction call to onSuggestionsReceived in
AutocompleteMediator (and fixes incorrect documentation)

Bug: 1151225, 1151080, 1146585
Change-Id: Iae295a7629c35aeae8ce357e414b05e6847d62e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2552570Reviewed-by: default avatarFilip Gorski <fgorski@chromium.org>
Reviewed-by: default avatarTomasz Wiszkowski <ender@google.com>
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829885}
parent 4da74cad
...@@ -155,6 +155,7 @@ public final class LocationBarCoordinator ...@@ -155,6 +155,7 @@ public final class LocationBarCoordinator
mSubCoordinator.destroy(); mSubCoordinator.destroy();
mSubCoordinator = null; mSubCoordinator = null;
} }
mUrlCoordinator.destroy();
mUrlCoordinator = null; mUrlCoordinator = null;
mLocationBarLayout.removeUrlFocusChangeListener(mAutocompleteCoordinator); mLocationBarLayout.removeUrlFocusChangeListener(mAutocompleteCoordinator);
mAutocompleteCoordinator.destroy(); mAutocompleteCoordinator.destroy();
......
...@@ -669,7 +669,8 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener { ...@@ -669,7 +669,8 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
* need to be updated. * need to be updated.
*/ */
/* package */ void updateMicButtonState() { /* package */ void updateMicButtonState() {
mVoiceSearchEnabled = mVoiceRecognitionHandler.isVoiceSearchEnabled(); mVoiceSearchEnabled =
mVoiceRecognitionHandler != null && mVoiceRecognitionHandler.isVoiceSearchEnabled();
updateButtonVisibility(); updateButtonVisibility();
} }
......
...@@ -222,12 +222,14 @@ public abstract class UrlBar extends AutocompleteEditText { ...@@ -222,12 +222,14 @@ public abstract class UrlBar extends AutocompleteEditText {
new GestureDetector(getContext(), new GestureDetector.SimpleOnGestureListener() { new GestureDetector(getContext(), new GestureDetector.SimpleOnGestureListener() {
@Override @Override
public void onLongPress(MotionEvent e) { public void onLongPress(MotionEvent e) {
if (mUrlBarDelegate == null) return;
mUrlBarDelegate.gestureDetected(true); mUrlBarDelegate.gestureDetected(true);
performLongClick(); performLongClick();
} }
@Override @Override
public boolean onSingleTapUp(MotionEvent e) { public boolean onSingleTapUp(MotionEvent e) {
if (mUrlBarDelegate == null) return true;
requestFocus(); requestFocus();
mUrlBarDelegate.gestureDetected(false); mUrlBarDelegate.gestureDetected(false);
return true; return true;
...@@ -244,6 +246,15 @@ public abstract class UrlBar extends AutocompleteEditText { ...@@ -244,6 +246,15 @@ public abstract class UrlBar extends AutocompleteEditText {
ApiCompatibilityUtils.disableSmartSelectionTextClassifier(this); ApiCompatibilityUtils.disableSmartSelectionTextClassifier(this);
} }
public void destroy() {
setAllowFocus(false);
mUrlBarDelegate = null;
setOnFocusChangeListener(null);
mTextContextMenuDelegate = null;
mUrlTextChangeListener = null;
mTextChangedListener = null;
}
/** /**
* Initialize the delegate that allows interaction with the Window. * Initialize the delegate that allows interaction with the Window.
*/ */
...@@ -340,7 +351,8 @@ public abstract class UrlBar extends AutocompleteEditText { ...@@ -340,7 +351,8 @@ public abstract class UrlBar extends AutocompleteEditText {
@Override @Override
public View focusSearch(int direction) { public View focusSearch(int direction) {
if (direction == View.FOCUS_BACKWARD && mUrlBarDelegate.getViewForUrlBackFocus() != null) { if (mUrlBarDelegate != null && direction == View.FOCUS_BACKWARD
&& mUrlBarDelegate.getViewForUrlBackFocus() != null) {
return mUrlBarDelegate.getViewForUrlBackFocus(); return mUrlBarDelegate.getViewForUrlBackFocus();
} else { } else {
return super.focusSearch(direction); return super.focusSearch(direction);
......
...@@ -37,8 +37,8 @@ public class UrlBarCoordinator implements UrlBarEditingTextStateProvider { ...@@ -37,8 +37,8 @@ public class UrlBarCoordinator implements UrlBarEditingTextStateProvider {
int SELECT_END = 1; int SELECT_END = 1;
} }
private final UrlBar mUrlBar; private UrlBar mUrlBar;
private final UrlBarMediator mMediator; private UrlBarMediator mMediator;
/** /**
* Constructs a coordinator for the given UrlBar view. * Constructs a coordinator for the given UrlBar view.
...@@ -67,6 +67,13 @@ public class UrlBarCoordinator implements UrlBarEditingTextStateProvider { ...@@ -67,6 +67,13 @@ public class UrlBarCoordinator implements UrlBarEditingTextStateProvider {
mMediator = new UrlBarMediator(model, focusChangeCallback); mMediator = new UrlBarMediator(model, focusChangeCallback);
} }
public void destroy() {
mMediator.destroy();
mMediator = null;
mUrlBar.destroy();
mUrlBar = null;
}
/** @see UrlBarMediator#addUrlTextChangeListener(UrlTextChangeListener) */ /** @see UrlBarMediator#addUrlTextChangeListener(UrlTextChangeListener) */
public void addUrlTextChangeListener(UrlTextChangeListener listener) { public void addUrlTextChangeListener(UrlTextChangeListener listener) {
mMediator.addUrlTextChangeListener(listener); mMediator.addUrlTextChangeListener(listener);
......
...@@ -79,6 +79,12 @@ class UrlBarMediator ...@@ -79,6 +79,12 @@ class UrlBarMediator
setUseDarkTextColors(true); setUseDarkTextColors(true);
} }
public void destroy() {
mUrlTextChangeListeners.clear();
mTextChangedListeners.clear();
mOnFocusChangeCallback = (unused) -> {};
}
/** Adds a listener for url text changes. */ /** Adds a listener for url text changes. */
public void addUrlTextChangeListener(UrlTextChangeListener listener) { public void addUrlTextChangeListener(UrlTextChangeListener listener) {
mUrlTextChangeListeners.add(listener); mUrlTextChangeListeners.add(listener);
......
...@@ -198,7 +198,7 @@ public class AutocompleteController { ...@@ -198,7 +198,7 @@ public class AutocompleteController {
* {@link #start(Profile,String, String, boolean)}. * {@link #start(Profile,String, String, boolean)}.
* *
* <p> * <p>
* Calling this method with {@code false}, will result in * Calling this method with {@code true}, will result in
* {@link #onSuggestionsReceived(AutocompleteResult, String, long)} being called with an empty * {@link #onSuggestionsReceived(AutocompleteResult, String, long)} being called with an empty
* result set. * result set.
* *
......
...@@ -223,7 +223,7 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi ...@@ -223,7 +223,7 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi
} }
public void destroy() { public void destroy() {
stopAutocomplete(true); stopAutocomplete(false);
mDropdownViewInfoListBuilder.destroy(); mDropdownViewInfoListBuilder.destroy();
if (mTabObserver != null) { if (mTabObserver != null) {
mTabObserver.destroy(); mTabObserver.destroy();
......
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