Clean up scrim logic in LocationBarLayout

Scrim visibility in the location bar is now completely controlled by
a single function. The scrim observer is now added and removed based
on the scrim visibility to avoid conflict with other observers (like
the bottom sheet).

Bug: 834927
Change-Id: I172ca637dde5bc026b1b7f629bc34c6d939dff2d
Reviewed-on: https://chromium-review.googlesource.com/1026168
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558785}

chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java

@@ -188,7 +188,7 @@ public class LocationBarLayout
     private Runnable mRequestSuggestions;
 
     private ViewGroup mOmniboxResultsContainer;
-    protected FadingBackgroundView mFadingView;
+    private FadingBackgroundView mFadingView;
 
     private boolean mSuggestionsShown;
     private boolean mUrlHasFocus;
@@ -1108,8 +1108,8 @@ public class LocationBarLayout
             listener.onUrlFocusChange(hasFocus);
         }
 
-        updateOmniboxResultsContainer();
-        if (hasFocus) updateFadingBackgroundView(true);
+        maybeShowOmniboxResultsContainer();
+        updateFadingBackgroundView(hasFocus, false);
     }
 
     /**
@@ -1713,7 +1713,7 @@ public class LocationBarLayout
                 mSuggestionList.setVisibility(GONE);
             }
         }
-        updateOmniboxResultsContainer();
+        maybeShowOmniboxResultsContainer();
     }
 
     /**
@@ -2291,12 +2291,10 @@ public class LocationBarLayout
         mOmniboxResultsContainer = (ViewGroup) overlayStub.inflate();
     }
 
-    private void updateOmniboxResultsContainer() {
+    private void maybeShowOmniboxResultsContainer() {
         if (mSuggestionsShown || mUrlHasFocus) {
             initOmniboxResultsContainer();
             updateOmniboxResultsContainerVisibility(true);
-        } else if (mOmniboxResultsContainer != null) {
-            updateFadingBackgroundView(false);
         }
     }
 
@@ -2313,21 +2311,10 @@ public class LocationBarLayout
         }
     }
 
-    /**
-     * Initialize the fading background for when the omnibox is focused.
-     */
-    protected void initFadingOverlayView() {
-        mFadingView =
-                (FadingBackgroundView) getRootView().findViewById(R.id.fading_focus_target);
-        mFadingView.addObserver(this);
-    }
-
     @Override
     public void onFadingViewClick() {
         setUrlBarFocus(false);
-
-        // If the bottom sheet is used, it will control the fading view.
-        if (mBottomSheet == null) updateFadingBackgroundView(false);
+        updateFadingBackgroundView(false, false);
     }
 
     @Override
@@ -2348,23 +2335,22 @@ public class LocationBarLayout
      * Update the fading background view that shows when the omnibox is focused. If Chrome Home is
      * enabled, this method is a no-op.
      * @param visible Whether the background should be made visible.
+     * @param ignoreNtpChecks Whether the checks for the ntp should be considered when updating the
+     *                        scrim.
      */
-    private void updateFadingBackgroundView(boolean visible) {
-        if (mFadingView == null) initFadingOverlayView();
-
-        // If Chrome Home is enabled (the bottom sheet is not null), it will be controlling the
-        // fading view, so block any updating here.
-        if (mToolbarDataProvider == null || mBottomSheet != null) return;
-
+    protected void updateFadingBackgroundView(boolean visible, boolean ignoreNtpChecks) {
+        if (mFadingView == null) mFadingView = getRootView().findViewById(R.id.fading_focus_target);
         NewTabPage ntp = mToolbarDataProvider.getNewTabPageForCurrentTab();
         boolean locationBarShownInNTP = ntp != null && ntp.isLocationBarShownInNTP();
 
-        if (visible && !locationBarShownInNTP) {
+        if (visible && (!locationBarShownInNTP || ignoreNtpChecks)) {
+            mFadingView.addObserver(this);
             // If the location bar is shown in the NTP, the toolbar will eventually trigger a
             // fade in.
             mFadingView.showFadingOverlay();
         } else {
             mFadingView.hideFadingOverlay(!locationBarShownInNTP);
+            mFadingView.removeObserver(this);
         }
     }
 

chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarPhone.java

@@ -188,8 +188,7 @@ public class LocationBarPhone extends LocationBarLayout {
 
         NewTabPage ntp = getToolbarDataProvider().getNewTabPageForCurrentTab();
         if (hasFocus && ntp != null && ntp.isLocationBarShownInNTP() && mBottomSheet == null) {
-            if (mFadingView == null) initFadingOverlayView();
-            mFadingView.showFadingOverlay();
+            updateFadingBackgroundView(true, true);
         }
     }
 

chrome/android/java/src/org/chromium/chrome/browser/widget/FadingBackgroundView.java

@@ -172,6 +172,14 @@ public class FadingBackgroundView extends View implements View.OnClickListener {
         mObservers.addObserver(observer);
     }
 
+    /**
+     * Removes an observer to this fading view.
+     * @param observer The observer to be removed.
+     */
+    public void removeObserver(FadingViewObserver observer) {
+        mObservers.removeObserver(observer);
+    }
+
     @Override
     public void onClick(View view) {
         for (FadingViewObserver o : mObservers) o.onFadingViewClick();