Remove LogoItem and SiteSection from NewTabPageAdapter

Bug: 805070
Change-Id: I60095497d0dcf5029737ae511be13bb8d9af55d8
Reviewed-on: https://chromium-review.googlesource.com/1097412
Commit-Queue: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566789}

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

@@ -340,9 +340,6 @@
     <dimen name="tile_view_title_margin_top_condensed">62dp</dimen>
     <dimen name="tile_view_title_margin_top_modern">61dp</dimen>
     <dimen name="ntp_logo_height">100dp</dimen>
-    <dimen name="ntp_logo_margin_top_modern">25dp</dimen>
-    <!-- bottom_control_container_height + 8 -->
-    <dimen name="ntp_logo_margin_bottom_modern">64dp</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>
@@ -372,7 +369,6 @@
     <dimen name="snippets_offline_icon_size">18sp</dimen>
     <dimen name="ntp_suggestions_footer_padding_top">40dp</dimen>
     <dimen name="ntp_suggestions_footer_padding_bottom">18dp</dimen>
-    <dimen name="chrome_home_logo_minimum_height">1dp</dimen>
     <dimen name="modern_suggestions_footer_padding_top">24dp</dimen>
     <dimen name="modern_suggestions_footer_padding_bottom">24dp</dimen>
     <dimen name="content_suggestions_card_modern_margin">16dp</dimen>

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

@@ -288,9 +288,8 @@ public class NewTabPageView
         mRecyclerView.init(mUiConfig, mContextMenuManager);
 
         // Set up snippets
-        NewTabPageAdapter newTabPageAdapter =
-                new NewTabPageAdapter(mManager, mNewTabPageLayout, /* logoView = */ null, mUiConfig,
-                        offlinePageBridge, mContextMenuManager, /* tileGroupDelegate = */ null);
+        NewTabPageAdapter newTabPageAdapter = new NewTabPageAdapter(
+                mManager, mNewTabPageLayout, mUiConfig, offlinePageBridge, mContextMenuManager);
         newTabPageAdapter.refreshSuggestions();
         mRecyclerView.setAdapter(newTabPageAdapter);
         mRecyclerView.getLinearLayoutManager().scrollToPosition(scrollPosition);

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/CardViewHolder.java

@@ -230,8 +230,6 @@ public abstract class CardViewHolder
             case ItemViewType.PROMO:
                 return true;
             case ItemViewType.ABOVE_THE_FOLD:
-            case ItemViewType.LOGO:
-            case ItemViewType.SITE_SECTION:
             case ItemViewType.HEADER:
             case ItemViewType.PROGRESS:
             case ItemViewType.FOOTER:

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ItemViewType.java

@@ -15,9 +15,9 @@ import java.lang.annotation.RetentionPolicy;
  *
  * @see Adapter#getItemViewType(int)
  */
-@IntDef({ItemViewType.ABOVE_THE_FOLD, ItemViewType.LOGO, ItemViewType.SITE_SECTION,
-        ItemViewType.HEADER, ItemViewType.SNIPPET, ItemViewType.STATUS, ItemViewType.PROGRESS,
-        ItemViewType.ACTION, ItemViewType.FOOTER, ItemViewType.PROMO, ItemViewType.ALL_DISMISSED})
+@IntDef({ItemViewType.ABOVE_THE_FOLD, ItemViewType.HEADER, ItemViewType.SNIPPET,
+        ItemViewType.STATUS, ItemViewType.PROGRESS, ItemViewType.ACTION, ItemViewType.FOOTER,
+        ItemViewType.PROMO, ItemViewType.ALL_DISMISSED})
 @Retention(RetentionPolicy.SOURCE)
 public @interface ItemViewType {
     /**
@@ -26,64 +26,52 @@ public @interface ItemViewType {
      * @see Adapter#getItemViewType(int)
      */
     int ABOVE_THE_FOLD = 1;
-    /**
-     * View type for the logo
-     *
-     * @see Adapter#getItemViewType(int)
-     */
-    int LOGO = 2;
-    /**
-     * View type for a {@code SiteSection}.
-     *
-     * @see Adapter#getItemViewType(int)
-     */
-    int SITE_SECTION = 3;
     /**
      * View type for card group headers
      *
      * @see Adapter#getItemViewType(int)
      */
-    int HEADER = 4;
+    int HEADER = 2;
     /**
      * View type for snippet cards
      *
      * @see Adapter#getItemViewType(int)
      */
-    int SNIPPET = 5;
+    int SNIPPET = 3;
     /**
      * View type for a {@link StatusItem}, the card displaying status information
      *
      * @see Adapter#getItemViewType(int)
      */
-    int STATUS = 6;
+    int STATUS = 4;
     /**
      * View type for a {@link ProgressItem}, the progress indicator.
      *
      * @see Adapter#getItemViewType(int)
      */
-    int PROGRESS = 7;
+    int PROGRESS = 5;
     /**
      * View type for a {@link ActionItem}, an action button.
      *
      * @see Adapter#getItemViewType(int)
      */
-    int ACTION = 8;
+    int ACTION = 6;
     /**
      * View type for a {@link Footer}.
      *
      * @see Adapter#getItemViewType(int)
      */
-    int FOOTER = 9;
+    int FOOTER = 7;
     /**
      * View type for a {@link SignInPromo}.
      *
      * @see Adapter#getItemViewType(int)
      */
-    int PROMO = 10;
+    int PROMO = 8;
     /**
      * View type for a {@link AllDismissedItem}.
      *
      * @see Adapter#getItemViewType(int)
      */
-    int ALL_DISMISSED = 11;
+    int ALL_DISMISSED = 9;
 }

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java

@@ -15,7 +15,6 @@ import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.modelutil.ListObservable;
 import org.chromium.chrome.browser.ntp.ContextMenuManager;
-import org.chromium.chrome.browser.ntp.LogoView;
 import org.chromium.chrome.browser.ntp.cards.NewTabPageViewHolder.PartialBindCallback;
 import org.chromium.chrome.browser.ntp.snippets.CategoryInt;
 import org.chromium.chrome.browser.ntp.snippets.CategoryStatus;
@@ -26,12 +25,9 @@ import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge;
 import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource;
 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.suggestions.DestructionObserver;
-import org.chromium.chrome.browser.suggestions.LogoItem;
-import org.chromium.chrome.browser.suggestions.SiteSection;
 import org.chromium.chrome.browser.suggestions.SuggestionsConfig;
 import org.chromium.chrome.browser.suggestions.SuggestionsRecyclerView;
 import org.chromium.chrome.browser.suggestions.SuggestionsUiDelegate;
-import org.chromium.chrome.browser.suggestions.TileGroup;
 import org.chromium.chrome.browser.widget.displaystyle.UiConfig;
 
 import java.util.List;
@@ -50,13 +46,11 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder>
     private final OfflinePageBridge mOfflinePageBridge;
 
     private final @Nullable View mAboveTheFoldView;
-    private final @Nullable LogoView mLogoView;
     private final UiConfig mUiConfig;
     private SuggestionsRecyclerView mRecyclerView;
 
     private final InnerNode mRoot;
 
-    private final @Nullable SiteSection mSiteSection;
     private final SectionList mSections;
     private final @Nullable SignInPromo mSigninPromo;
     private final AllDismissedItem mAllDismissed;
@@ -68,24 +62,17 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder>
      * @param aboveTheFoldView the layout encapsulating all the above-the-fold elements
      *         (logo, search box, most visited tiles), or null if only suggestions should
      *         be displayed.
-     * @param logoView the view for the logo, which may be provided when {@code aboveTheFoldView} is
-     *         null. They are not expected to be both non-null as that would lead to showing the
-     *         logo twice.
      * @param uiConfig the NTP UI configuration, to be passed to created views.
      * @param offlinePageBridge used to determine if articles are available.
      * @param contextMenuManager used to build context menus.
-     * @param tileGroupDelegate if not null this is used to build a {@link SiteSection}.
      */
     public NewTabPageAdapter(SuggestionsUiDelegate uiDelegate, @Nullable View aboveTheFoldView,
-            @Nullable LogoView logoView, UiConfig uiConfig, OfflinePageBridge offlinePageBridge,
-            ContextMenuManager contextMenuManager, @Nullable TileGroup.Delegate tileGroupDelegate) {
-        assert !(aboveTheFoldView != null && logoView != null);
-
+            UiConfig uiConfig, OfflinePageBridge offlinePageBridge,
+            ContextMenuManager contextMenuManager) {
         mUiDelegate = uiDelegate;
         mContextMenuManager = contextMenuManager;
 
         mAboveTheFoldView = aboveTheFoldView;
-        mLogoView = logoView;
         mUiConfig = uiConfig;
         mRoot = new InnerNode();
         mSections = new SectionList(mUiDelegate, offlinePageBridge);
@@ -94,16 +81,6 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder>
 
         if (mAboveTheFoldView != null) mRoot.addChildren(new AboveTheFoldItem());
 
-        if (mLogoView != null) mRoot.addChildren(new LogoItem());
-
-        if (tileGroupDelegate == null) {
-            mSiteSection = null;
-        } else {
-            mSiteSection = new SiteSection(uiDelegate, mContextMenuManager, tileGroupDelegate,
-                    offlinePageBridge, uiConfig);
-            mRoot.addChildren(mSiteSection);
-        }
-
         if (SuggestionsConfig.scrollToLoad()) {
             // If scroll-to-load is enabled, show the sign-in promo above suggested content.
             if (mSigninPromo != null) mRoot.addChildren(mSigninPromo);
@@ -134,20 +111,13 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder>
     }
 
     @Override
-    public NewTabPageViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
+    public NewTabPageViewHolder onCreateViewHolder(ViewGroup parent, @ItemViewType int viewType) {
         assert parent == mRecyclerView;
 
         switch (viewType) {
             case ItemViewType.ABOVE_THE_FOLD:
                 return new NewTabPageViewHolder(mAboveTheFoldView);
 
-            case ItemViewType.LOGO:
-                return new LogoItem.ViewHolder(mLogoView);
-
-            case ItemViewType.SITE_SECTION:
-                return SiteSection.createViewHolder(
-                        SiteSection.inflateSiteSection(parent), mUiConfig);
-
             case ItemViewType.HEADER:
                 return new SectionHeaderViewHolder(mRecyclerView, mUiConfig);
 
@@ -204,10 +174,6 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder>
     /** Resets suggestions, pulling the current state as known by the backend. */
     public void refreshSuggestions() {
         mSections.refreshSuggestions();
-
-        if (mSiteSection != null) {
-            mSiteSection.getTileGroup().onSwitchToForeground(/* trackLoadTask = */ true);
-        }
     }
 
     public int getFirstHeaderPosition() {

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NodeVisitor.java

@@ -57,14 +57,4 @@ public class NodeVisitor {
      * Visits a header.
      */
     public void visitHeader() {}
-
-    /**
-     * Visits the tile grid.
-     */
-    public void visitTileGrid() {}
-
-    /**
-     * Visits a logo.
-     */
-    public void visitLogo() {}
 }

chrome/android/java/src/org/chromium/chrome/browser/suggestions/LogoItem.java

-// Copyright 2017 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-package org.chromium.chrome.browser.suggestions;
-
-import android.content.res.Resources;
-import android.view.View;
-import android.view.ViewGroup;
-import android.view.ViewGroup.MarginLayoutParams;
-
-import org.chromium.chrome.R;
-import org.chromium.chrome.browser.ntp.LogoView;
-import org.chromium.chrome.browser.ntp.cards.ItemViewType;
-import org.chromium.chrome.browser.ntp.cards.Leaf;
-import org.chromium.chrome.browser.ntp.cards.NewTabPageViewHolder;
-import org.chromium.chrome.browser.ntp.cards.NodeVisitor;
-
-/**
- * A logo for the search engine provider.
- */
-public class LogoItem extends Leaf {
-    private boolean mVisible;
-
-    @Override
-    @ItemViewType
-    protected int getItemViewType() {
-        return ItemViewType.LOGO;
-    }
-
-    @Override
-    protected void onBindViewHolder(NewTabPageViewHolder holder) {
-        ((ViewHolder) holder).setVisible(mVisible);
-    }
-
-    @Override
-    public void visitItems(NodeVisitor visitor) {
-        visitor.visitLogo();
-    }
-
-    public void setVisible(boolean visible) {
-        mVisible = visible;
-        notifyItemChanged(0, holder -> ((ViewHolder) holder).setVisible(visible));
-    }
-
-    /**
-     * The {@code ViewHolder} for the {@link LogoItem}.
-     */
-    public static class ViewHolder extends NewTabPageViewHolder {
-        public ViewHolder(LogoView logoView) {
-            super(logoView);
-        }
-
-        @Override
-        public void recycle() {
-            if (itemView.getParent() != null) {
-                ((ViewGroup) itemView.getParent()).removeView(itemView);
-            }
-            super.recycle();
-        }
-
-        public void setVisible(boolean visible) {
-            itemView.setVisibility(visible ? View.VISIBLE : View.INVISIBLE);
-            Resources resources = itemView.getResources();
-            MarginLayoutParams params = (MarginLayoutParams) itemView.getLayoutParams();
-            params.height = visible
-                    ? resources.getDimensionPixelSize(R.dimen.ntp_logo_height)
-                    : resources.getDimensionPixelSize(R.dimen.chrome_home_logo_minimum_height);
-            int top = visible ? resources.getDimensionPixelSize(R.dimen.ntp_logo_margin_top_modern)
-                              : 0;
-            int bottom = visible
-                    ? resources.getDimensionPixelSize(R.dimen.ntp_logo_margin_bottom_modern)
-                    : 0;
-            params.setMargins(0, top, 0, bottom);
-            itemView.setLayoutParams(params);
-        }
-    }
-}

chrome/android/java/src/org/chromium/chrome/browser/suggestions/SiteSection.java

@@ -21,13 +21,18 @@ import org.chromium.chrome.browser.widget.displaystyle.UiConfig;
 
 /**
  * The model and controller for a group of site suggestions.
+ * @deprecated This class is still being used, but not in the New Tab Page RecyclerView
+ *         anymore. It still uses the latter's base classes until SiteSection is migrated to the new
+ *         UI architecture.
  */
+@Deprecated
 public class SiteSection extends OptionalLeaf implements TileGroup.Observer {
     /**
      * The maximum number of tiles to try and fit in a row. On smaller screens, there may not be
      * enough space to fit all of them.
      */
     private static final int MAX_TILE_COLUMNS = 4;
+    private static final int TILE_TITLE_LINES = 1;
 
     private final TileGroup mTileGroup;
     private final TileRenderer mTileRenderer;
@@ -45,7 +50,7 @@ public class SiteSection extends OptionalLeaf implements TileGroup.Observer {
             TileGroup.Delegate tileGroupDelegate, OfflinePageBridge offlinePageBridge,
             UiConfig uiConfig) {
         mTileRenderer = new TileRenderer(ContextUtils.getApplicationContext(),
-                SuggestionsConfig.getTileStyle(uiConfig), getTileTitleLines(),
+                SuggestionsConfig.getTileStyle(uiConfig), TILE_TITLE_LINES,
                 uiDelegate.getImageFetcher());
         mTileGroup = new TileGroup(mTileRenderer, uiDelegate, contextMenuManager, tileGroupDelegate,
                 /* observer = */ this, offlinePageBridge);
@@ -55,7 +60,9 @@ public class SiteSection extends OptionalLeaf implements TileGroup.Observer {
     @Override
     @ItemViewType
     protected int getItemViewType() {
-        return ItemViewType.SITE_SECTION;
+        // Throw an exception instead of just `assert false` to avoid compiler warnings about the
+        // return value.
+        throw new IllegalStateException();
     }
 
     @Override
@@ -67,7 +74,7 @@ public class SiteSection extends OptionalLeaf implements TileGroup.Observer {
 
     @Override
     protected void visitOptionalItem(NodeVisitor visitor) {
-        visitor.visitTileGrid();
+        assert false;
     }
 
     @Override
@@ -94,7 +101,7 @@ public class SiteSection extends OptionalLeaf implements TileGroup.Observer {
         notifyItemChanged(0, (holder) -> ((SiteSectionViewHolder) holder).updateOfflineBadge(tile));
     }
 
-    public TileGroup getTileGroup() {
+    TileGroup getTileGroupForTesting() {
         return mTileGroup;
     }
 
@@ -105,10 +112,6 @@ public class SiteSection extends OptionalLeaf implements TileGroup.Observer {
         return 2;
     }
 
-    private static int getTileTitleLines() {
-        return 1;
-    }
-
     @LayoutRes
     private static int getLayout() {
         if (SuggestionsConfig.useModernLayout()) {

chrome/android/java_sources.gni

@@ -1224,7 +1224,6 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/ssl/SecurityStateModel.java",
   "java/src/org/chromium/chrome/browser/suggestions/DestructionObserver.java",
   "java/src/org/chromium/chrome/browser/suggestions/ImageFetcher.java",
-  "java/src/org/chromium/chrome/browser/suggestions/LogoItem.java",
   "java/src/org/chromium/chrome/browser/suggestions/MostVisitedSites.java",
   "java/src/org/chromium/chrome/browser/suggestions/MostVisitedSitesBridge.java",
   "java/src/org/chromium/chrome/browser/suggestions/SiteSection.java",

chrome/android/javatests/src/org/chromium/chrome/browser/suggestions/TileGridLayoutTest.java

@@ -324,7 +324,7 @@ public class TileGridLayoutTest {
             uiConfig.updateDisplayStyle();
 
             SiteSection siteSection = createSiteSection(viewHolder, uiConfig, offlineUrls);
-            siteSection.getTileGroup().onSwitchToForeground(false);
+            siteSection.getTileGroupForTesting().onSwitchToForeground(false);
             assertTrue("Tile Data should be visible.", siteSection.isVisible());
 
             siteSection.onBindViewHolder(viewHolder, 0);

chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java

@@ -1242,9 +1242,9 @@ public class NewTabPageAdapterTest {
 
     private void reloadNtp() {
         mSource.removeObservers();
-        mAdapter = new NewTabPageAdapter(mUiDelegate, mock(View.class), /* logoView = */ null,
-                makeUiConfig(), mOfflinePageBridge, mock(ContextMenuManager.class),
-                /* tileGroupDelegate = */ null);
+        mAdapter = new NewTabPageAdapter(mUiDelegate, mock(View.class), /* logoView = */
+                makeUiConfig(), mOfflinePageBridge, mock(ContextMenuManager.class)
+                /* tileGroupDelegate = */);
         mAdapter.refreshSuggestions();
     }
 

chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/suggestions/ContentSuggestionsTestUtils.java

@@ -187,16 +187,6 @@ public final class ContentSuggestionsTestUtils {
                 describeItem("HEADER");
             }
 
-            @Override
-            public void visitTileGrid() {
-                describeItem("SITE_SECTION");
-            }
-
-            @Override
-            public void visitLogo() {
-                describeItem("LOGO");
-            }
-
             private void describeItem(String description) {
                 stringBuilder.append(
                         String.format(Locale.US, "%s - %s%n", mPosition++, description));