From 8ce6fe330ebf9e56a14d97662ac13d136326acd5 Mon Sep 17 00:00:00 2001
From: Michael Comella <michael.l.comella@gmail.com>
Date: Fri, 26 Jun 2020 18:31:02 -0700
Subject: [PATCH] [fenix] For
 https://github.com/mozilla-mobile/fenix/issues/8795: remove redundant
 ConstraintLayout around BrowserToolbar.
 (https://github.com/mozilla-mobile/fenix/pull/12014)

This is functionally equivalent to the code before this patch but should
be slightly more performant in theory because ConstraintLayout is
expensive to inflate.

The elevation and layoutParams set dynamically appeared to have no effect
with the wrapping view but broke the view when used by itself so I had
to remove them. I also updated a few other unnecessary params.

Theoretically this may have some perf benefits but I didn't see anything
outside noise levels after I took the numbers (but I didn't try very
hard).
---
 .../mozilla/fenix/search/SearchFragment.kt    |  2 -
 .../fenix/search/toolbar/ToolbarView.kt       | 10 -----
 app/src/main/res/layout/fragment_search.xml   | 44 +++++++------------
 3 files changed, 15 insertions(+), 41 deletions(-)

diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt b/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt
index 2190be332b..4f5ef089e6 100644
--- a/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt
@@ -294,8 +294,6 @@ class SearchFragment : Fragment(), UserInteractionHandler {
 
         view.search_suggestions_onboarding.setOnInflateListener((stubListener))
 
-        view.toolbar_wrapper.clipToOutline = false
-
         fill_link_from_clipboard.setOnClickListener {
             (activity as HomeActivity)
                 .openToBrowserAndLoad(
diff --git a/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt b/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt
index bf997e4138..31369ed68f 100644
--- a/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt
+++ b/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt
@@ -8,7 +8,6 @@ import android.content.Context
 import android.graphics.Bitmap
 import android.graphics.drawable.BitmapDrawable
 import androidx.appcompat.content.res.AppCompatResources
-import androidx.coordinatorlayout.widget.CoordinatorLayout
 import androidx.core.content.ContextCompat
 import mozilla.components.browser.domains.autocomplete.ShippedDomainsProvider
 import mozilla.components.browser.toolbar.BrowserToolbar
@@ -16,7 +15,6 @@ import mozilla.components.concept.engine.Engine
 import mozilla.components.concept.storage.HistoryStorage
 import mozilla.components.feature.toolbar.ToolbarAutocompleteFeature
 import mozilla.components.support.ktx.android.content.getColorFromAttr
-import mozilla.components.support.ktx.android.util.dpToPx
 import mozilla.components.support.ktx.android.view.hideKeyboard
 import org.mozilla.fenix.R
 import org.mozilla.fenix.search.SearchFragmentState
@@ -64,8 +62,6 @@ class ToolbarView(
         view.apply {
             editMode()
 
-            elevation = TOOLBAR_ELEVATION_IN_DP.dpToPx(resources.displayMetrics).toFloat()
-
             setOnUrlCommitListener {
                 // We're hiding the keyboard as early as possible to prevent the engine view
                 // from resizing in case the BrowserFragment is being displayed before the
@@ -80,8 +76,6 @@ class ToolbarView(
                     context, ThemeManager.resolveAttribute(R.attr.foundation, context)
                 )
 
-            layoutParams.height = CoordinatorLayout.LayoutParams.MATCH_PARENT
-
             edit.hint = context.getString(R.string.search_hint)
 
             edit.colors = edit.colors.copy(
@@ -156,8 +150,4 @@ class ToolbarView(
 
         view.edit.setIcon(icon, searchState.searchEngineSource.searchEngine.name)
     }
-
-    companion object {
-        private const val TOOLBAR_ELEVATION_IN_DP = 16
-    }
 }
diff --git a/app/src/main/res/layout/fragment_search.xml b/app/src/main/res/layout/fragment_search.xml
index d9d325649a..c2a77fa70a 100644
--- a/app/src/main/res/layout/fragment_search.xml
+++ b/app/src/main/res/layout/fragment_search.xml
@@ -11,38 +11,24 @@
     android:background="?foundation"
     tools:context=".search.SearchFragment">
 
-    <androidx.constraintlayout.widget.ConstraintLayout
-        android:id="@+id/toolbar_wrapper"
+    <mozilla.components.browser.toolbar.BrowserToolbar
+        android:id="@+id/toolbar"
         android:layout_width="0dp"
         android:layout_height="@dimen/browser_toolbar_height"
-        android:layout_margin="0dp"
-        android:outlineProvider="paddedBounds"
+        android:background="@drawable/toolbar_background_top"
+        android:clickable="true"
+        android:focusable="true"
+        android:focusableInTouchMode="true"
+        app:layout_scrollFlags="scroll|enterAlways|snap|exitUntilCollapsed"
+        app:browserToolbarClearColor="?primaryText"
+        app:browserToolbarInsecureColor="?primaryText"
+        app:browserToolbarMenuColor="?primaryText"
+        app:browserToolbarProgressBarGravity="bottom"
+        app:browserToolbarSecureColor="?primaryText"
+        app:browserToolbarTrackingProtectionAndSecurityIndicatorSeparatorColor="?toolbarDivider"
         app:layout_constraintEnd_toEndOf="parent"
         app:layout_constraintStart_toStartOf="parent"
-        app:layout_constraintTop_toTopOf="parent">
-
-        <mozilla.components.browser.toolbar.BrowserToolbar
-            android:id="@+id/toolbar"
-            android:layout_width="match_parent"
-            android:layout_height="@dimen/browser_toolbar_height"
-            android:layout_gravity="top"
-            android:background="@drawable/toolbar_background_top"
-            android:clickable="true"
-            android:focusable="true"
-            android:focusableInTouchMode="true"
-            app:layout_scrollFlags="scroll|enterAlways|snap|exitUntilCollapsed"
-            app:browserToolbarClearColor="?primaryText"
-            app:browserToolbarInsecureColor="?primaryText"
-            app:browserToolbarMenuColor="?primaryText"
-            app:browserToolbarProgressBarGravity="bottom"
-            app:browserToolbarSecureColor="?primaryText"
-            app:browserToolbarTrackingProtectionAndSecurityIndicatorSeparatorColor="?toolbarDivider"
-            app:layout_constraintBottom_toBottomOf="parent"
-            app:layout_constraintEnd_toEndOf="parent"
-            app:layout_constraintStart_toStartOf="parent"
-            app:layout_constraintTop_toTopOf="parent"/>
-
-    </androidx.constraintlayout.widget.ConstraintLayout>
+        app:layout_constraintTop_toTopOf="parent"/>
 
     <androidx.core.widget.NestedScrollView
         android:layout_width="0dp"
@@ -51,7 +37,7 @@
         app:layout_constraintBottom_toBottomOf="@id/search_divider"
         app:layout_constraintEnd_toEndOf="parent"
         app:layout_constraintStart_toStartOf="parent"
-        app:layout_constraintTop_toBottomOf="@id/toolbar_wrapper">
+        app:layout_constraintTop_toBottomOf="@id/toolbar">
 
         <androidx.constraintlayout.widget.ConstraintLayout
             android:id="@+id/scrollable_area"