Cleanup location bar layout logic

Remove padding from modern_toolbar_background and update/simplify the
location bar layout logic to better match the UX spec.

BUG=815343

Change-Id: I24e0239f3763c6c2fd7bd1f8c5f286ebf4f0b78c
Reviewed-on: https://chromium-review.googlesource.com/1197902Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588163}

chrome/android/java/res/drawable/modern_toolbar_background.xml

@@ -11,9 +11,4 @@
         android:width="@dimen/modern_toolbar_background_size"
         android:height="@dimen/modern_toolbar_background_size" />
     <corners android:radius="@dimen/modern_toolbar_background_corner_radius" />
-    <padding
-        android:top="0dp"
-        android:bottom="0dp"
-        android:left="@dimen/modern_toolbar_background_lateral_inset"
-        android:right="@dimen/modern_toolbar_background_lateral_inset" />
 </shape>

chrome/android/java/res/drawable/modern_toolbar_background_white.xml

@@ -11,9 +11,4 @@
         android:width="@dimen/modern_toolbar_background_size"
         android:height="@dimen/modern_toolbar_background_size" />
     <corners android:radius="@dimen/modern_toolbar_background_corner_radius" />
-    <padding
-        android:top="0dp"
-        android:bottom="0dp"
-        android:left="@dimen/modern_toolbar_background_lateral_inset"
-        android:right="@dimen/modern_toolbar_background_lateral_inset" />
 </shape>

chrome/android/java/res/layout/location_bar_base.xml

@@ -13,6 +13,7 @@
     <FrameLayout android:id="@+id/location_bar_icon"
         android:layout_width="@dimen/location_bar_icon_width"
         android:layout_height="match_parent"
+        android:paddingEnd="4dp"
         android:visibility="gone" >
 
         <ImageView android:id="@+id/navigation_button"

chrome/android/java/res/layout/new_tab_page_layout.xml

@@ -36,18 +36,15 @@
         android:layout_marginBottom="1dp"
         android:gravity="center_vertical"
         android:addStatesFromChildren="true"
-        android:background="@drawable/card_single"
+        android:background="@drawable/ntp_search_box"
         android:orientation="horizontal"
-        android:paddingStart="2dp"
-        android:paddingEnd="2dp"
-        android:paddingTop="3dp"
-        android:paddingBottom="3dp" >
+        android:paddingStart="@dimen/location_bar_lateral_padding"
+        android:paddingEnd="@dimen/location_bar_lateral_padding" >
         <EditText
             style="@style/TextAppearance.NewTabPageSearchBoxText"
             android:id="@+id/search_box_text"
             android:layout_width="0dp"
             android:layout_height="match_parent"
-            android:layout_marginStart="12dp"
             android:layout_marginEnd="12dp"
             android:layout_weight="1"
             android:background="@null"
@@ -61,7 +58,6 @@
             android:id="@+id/voice_search_button"
             android:layout_width="wrap_content"
             android:layout_height="wrap_content"
-            android:layout_marginEnd="12dp"
             android:contentDescription="@string/accessibility_toolbar_btn_mic"
             android:src="@drawable/btn_mic"
             app:chrometint="@color/dark_mode_tint" />

chrome/android/java/res/layout/search_activity.xml

@@ -5,7 +5,6 @@
 
 <FrameLayout
     xmlns:android="http://schemas.android.com/apk/res/android"
-    xmlns:tools="http://schemas.android.com/tools"
     android:id="@+id/control_container"
     android:layout_width="match_parent"
     android:layout_height="match_parent" >
@@ -22,6 +21,8 @@
             android:layout_width="match_parent"
             android:layout_height="@dimen/toolbar_height_no_shadow"
             android:background="@android:color/white"
+            android:paddingStart="@dimen/toolbar_edge_padding"
+            android:paddingEnd="@dimen/toolbar_edge_padding"
             android:clickable="true" >
 
         <org.chromium.chrome.browser.searchwidget.SearchActivityLocationBarLayout
@@ -31,8 +32,8 @@
                 android:layout_gravity="center_vertical"
                 android:layout_marginTop="@dimen/location_bar_vertical_margin"
                 android:layout_marginBottom="@dimen/location_bar_vertical_margin"
-                android:layout_marginStart="@dimen/modern_toolbar_background_focused_left_margin"
-                android:layout_marginEnd="@dimen/modern_toolbar_background_focused_left_margin" />
+                android:paddingStart="@dimen/location_bar_lateral_padding"
+                android:paddingEnd="@dimen/location_bar_lateral_padding" />
 
     </FrameLayout>
 

chrome/android/java/res/layout/toolbar_phone.xml

@@ -43,11 +43,6 @@
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:layout_gravity="top|end" >
-
-        <Space
-            android:layout_width="4dp"
-            android:layout_height="match_parent" />
-
         <FrameLayout
             android:layout_width="wrap_content"
             android:layout_height="wrap_content"

chrome/android/java/res/values-v17/styles.xml

@@ -580,6 +580,7 @@
     <style name="LocationBarActionButton" parent="LocationBarButton">
         <item name="android:layout_width">@dimen/location_bar_icon_width</item>
         <item name="android:layout_height">match_parent</item>
+        <item name="android:paddingStart">4dp</item>
         <item name="android:scaleType">center</item>
         <item name="android:layout_gravity">end</item>
         <item name="chrometint">@color/dark_mode_tint</item>

chrome/android/java/res/values/dimens.xml

@@ -269,7 +269,8 @@
     <dimen name="toolbar_edge_padding">8dp</dimen>
     <dimen name="location_bar_vertical_margin">8dp</dimen>
     <dimen name="location_bar_url_text_size">16sp</dimen>
-    <dimen name="location_bar_icon_width">32dp</dimen>
+    <dimen name="location_bar_icon_width">28dp</dimen>
+    <dimen name="location_bar_lateral_padding">10dp</dimen>
 
     <dimen name="tablet_toolbar_start_padding">4dp</dimen>
     <dimen name="tablet_toolbar_start_padding_no_buttons">6dp</dimen>
@@ -277,12 +278,10 @@
     <dimen name="location_bar_status_separator_width">1dp</dimen>
 
     <!-- Modern toolbar dimensions -->
-    <dimen name="modern_toolbar_background_focused_left_margin">6dp</dimen>
     <dimen name="modern_toolbar_background_size">40dp</dimen>
     <!-- Sometimes the modern toolbar is shown at 48dp so a corner radius of
          24dp is needed to completely round the corners. -->
     <dimen name="modern_toolbar_background_corner_radius">24dp</dimen>
-    <dimen name="modern_toolbar_background_lateral_inset">2dp</dimen>
 
     <!-- Omnibox suggestions -->
     <dimen name="omnibox_suggestion_height">60dp</dimen>
@@ -301,8 +300,8 @@
     <dimen name="omnibox_suggestion_refine_width">48dp</dimen>
     <dimen name="omnibox_suggestion_text_vertical_padding">5dp</dimen>
     <dimen name="omnibox_suggestion_multiline_text_vertical_padding">10dp</dimen>
-    <dimen name="omnibox_suggestion_refine_view_modern_end_padding">2dp</dimen>
-    <dimen name="omnibox_suggestion_list_modern_offset">10dp</dimen>
+    <dimen name="omnibox_suggestion_refine_view_modern_end_padding">4dp</dimen>
+    <dimen name="omnibox_suggestion_list_modern_offset">8dp</dimen>
 
     <!-- NTP dimensions -->
     <dimen name="tile_grid_layout_max_width">504dp</dimen>
@@ -338,11 +337,8 @@
     <dimen name="tile_view_title_margin_top_modern">61dp</dimen>
     <dimen name="ntp_logo_height">100dp</dimen>
     <dimen name="duet_ntp_logo_top_padding">16dp</dimen>
-    <dimen name="ntp_search_box_height">62dp</dimen>
-    <dimen name="ntp_search_box_height_modern">48dp</dimen>
-    <dimen name="ntp_search_box_shadow_width">4dp</dimen>
+    <dimen name="ntp_search_box_height">48dp</dimen>
     <dimen name="ntp_search_box_transition_length">16dp</dimen>
-    <dimen name="ntp_search_box_voice_search_margin_end_modern">6dp</dimen>
     <dimen name="explore_sites_radius">8dp</dimen>
     <dimen name="explore_sites_padding">8dp</dimen>
     <dimen name="explore_sites_margin">16dp</dimen>
@@ -524,7 +520,7 @@
     <!--  Miscellaneous dimensions -->
     <dimen name="action_bar_shadow_height">10dp</dimen>
     <dimen name="card_corner_radius">2dp</dimen>
-    <dimen name="clear_text_button_end_padding">4dp</dimen>
+    <dimen name="clear_text_button_end_padding">10dp</dimen>
     <dimen name="sync_promo_view_padding">16dp</dimen>
     <dimen name="sad_tab_bullet_gap">12dp</dimen>
     <dimen name="sad_tab_bullet_leading_offset">1dp</dimen>

chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageLayout.java

@@ -12,7 +12,6 @@ import android.graphics.drawable.Drawable;
 import android.graphics.drawable.RippleDrawable;
 import android.os.Build;
 import android.support.annotation.Nullable;
-import android.support.v4.view.MarginLayoutParamsCompat;
 import android.text.Editable;
 import android.text.TextWatcher;
 import android.util.AttributeSet;
@@ -72,7 +71,6 @@ public class NewTabPageLayout extends LinearLayout implements TileGroup.Observer
     private static final String PARAM_SIMPLIFIED_NTP_ABLATION = "simplified_ntp_ablation";
 
     private final int mTileGridLayoutBleed;
-    private final int mSearchboxShadowWidth;
 
     private View mMiddleSpacer; // Spacer between toolbar and Most Likely.
 
@@ -178,7 +176,6 @@ public class NewTabPageLayout extends LinearLayout implements TileGroup.Observer
         super(context, attrs);
         Resources res = getResources();
         mTileGridLayoutBleed = res.getDimensionPixelSize(R.dimen.tile_grid_layout_bleed);
-        mSearchboxShadowWidth = res.getDimensionPixelOffset(R.dimen.ntp_search_box_shadow_width);
     }
 
     @Override
@@ -247,15 +244,10 @@ public class NewTabPageLayout extends LinearLayout implements TileGroup.Observer
                 mManager.getNavigationDelegate(), mSearchProviderLogoView, profile);
 
         mSearchBoxView = findViewById(R.id.search_box);
-        if (SuggestionsConfig.useModernLayout()) {
-            mSearchBoxView.setBackgroundResource(R.drawable.ntp_search_box);
-            mSearchBoxView.getLayoutParams().height =
-                    getResources().getDimensionPixelSize(R.dimen.ntp_search_box_height_modern);
-
-            if (!DeviceFormFactor.isWindowOnTablet(mTab.getWindowAndroid())) {
-                mSearchBoxBoundsVerticalInset = getResources().getDimensionPixelSize(
-                        R.dimen.ntp_search_box_bounds_vertical_inset_modern);
-            }
+        if (SuggestionsConfig.useModernLayout()
+                && !DeviceFormFactor.isWindowOnTablet(mTab.getWindowAndroid())) {
+            mSearchBoxBoundsVerticalInset = getResources().getDimensionPixelSize(
+                    R.dimen.ntp_search_box_bounds_vertical_inset_modern);
         }
         mNoSearchLogoSpacer = findViewById(R.id.no_search_logo_spacer);
 
@@ -348,14 +340,6 @@ public class NewTabPageLayout extends LinearLayout implements TileGroup.Observer
         mVoiceSearchButton = findViewById(R.id.voice_search_button);
         mVoiceSearchButton.setOnClickListener(v -> mManager.focusSearchBox(true, null));
 
-        if (SuggestionsConfig.useModernLayout()
-                && !DeviceFormFactor.isWindowOnTablet(mTab.getWindowAndroid())) {
-            MarginLayoutParamsCompat.setMarginEnd(
-                    (MarginLayoutParams) mVoiceSearchButton.getLayoutParams(),
-                    getResources().getDimensionPixelSize(
-                            R.dimen.ntp_search_box_voice_search_margin_end_modern));
-        }
-
         TraceEvent.end(TAG + ".initializeVoiceSearchButton()");
     }
 
@@ -694,10 +678,8 @@ public class NewTabPageLayout extends LinearLayout implements TileGroup.Observer
     void getSearchBoxBounds(Rect bounds, Point translation, View parentView) {
         int searchBoxX = (int) mSearchBoxView.getX();
         int searchBoxY = (int) mSearchBoxView.getY();
-        bounds.set(searchBoxX + mSearchBoxView.getPaddingLeft(),
-                searchBoxY + mSearchBoxView.getPaddingTop(),
-                searchBoxX + mSearchBoxView.getWidth() - mSearchBoxView.getPaddingRight(),
-                searchBoxY + mSearchBoxView.getHeight() - mSearchBoxView.getPaddingBottom());
+        bounds.set(searchBoxX, searchBoxY, searchBoxX + mSearchBoxView.getWidth(),
+                searchBoxY + mSearchBoxView.getHeight());
 
         translation.set(0, 0);
 
@@ -906,14 +888,12 @@ public class NewTabPageLayout extends LinearLayout implements TileGroup.Observer
     private void unifyElementWidths() {
         if (mSiteSectionView.getVisibility() != GONE) {
             final int width = mSiteSectionView.getMeasuredWidth() - mTileGridLayoutBleed;
-            measureExactly(mSearchBoxView,
-                    width + mSearchboxShadowWidth, mSearchBoxView.getMeasuredHeight());
+            measureExactly(mSearchBoxView, width, mSearchBoxView.getMeasuredHeight());
             measureExactly(mSearchProviderLogoView,
                     width, mSearchProviderLogoView.getMeasuredHeight());
         } else if (mExploreSectionView != null) {
             final int exploreWidth = mExploreSectionView.getMeasuredWidth() - mTileGridLayoutBleed;
-            measureExactly(mSearchBoxView, exploreWidth + mSearchboxShadowWidth,
-                    mSearchBoxView.getMeasuredHeight());
+            measureExactly(mSearchBoxView, exploreWidth, mSearchBoxView.getMeasuredHeight());
             measureExactly(mSearchProviderLogoView, exploreWidth,
                     mSearchProviderLogoView.getMeasuredHeight());
         }

chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchActivityLocationBarLayout.java

@@ -7,7 +7,6 @@ package org.chromium.chrome.browser.searchwidget;
 import android.content.Context;
 import android.os.Handler;
 import android.support.annotation.Nullable;
-import android.support.v4.view.ViewCompat;
 import android.text.TextUtils;
 import android.util.AttributeSet;
 import android.view.View;
@@ -46,23 +45,6 @@ public class SearchActivityLocationBarLayout extends LocationBarLayout {
         setUrlBarFocusable(true);
         setBackground(ToolbarPhone.createModernLocationBarBackground(getResources()));
 
-        // Now you might ask yourself what these paddings are doing? Great question, really glad
-        // you asked...OH MY...LOOK BEHIND YOU!!!
-        //
-        // <Sounds of original author running away>
-        //
-        // These numbers were chosen at a single snapshot in time where they roughly equated to
-        // the padding used in the modern tabbed mode focused omnibox.  There is minimal rhyme or
-        // reason to this, but using something that approximated the toolbar logic looked even
-        // worse.
-        //
-        // Shrug. Once that layout logic is cleaned up, this should be universally replaced with
-        // something sane.
-        int toolbarSidePadding = getResources().getDimensionPixelSize(R.dimen.toolbar_edge_padding);
-        int backgroundSidePadding = getResources().getDimensionPixelSize(
-                R.dimen.modern_toolbar_background_focused_left_margin);
-        ViewCompat.setPaddingRelative(this, backgroundSidePadding + toolbarSidePadding,
-                getPaddingTop(), backgroundSidePadding, getPaddingBottom());
         mPendingSearchPromoDecision = LocaleManager.getInstance().needToCheckForSearchEnginePromo();
     }