PageInfo subpage improvements

Reduce dependencies on main PageInfo controller by only passing a simple
interface to subpage controllers.

Improve layout of subpage to have a proper back button and match the
paddings.

Bug: 1077766
Change-Id: I2922908e4eb44b6cff7c28290fdc7a9c82aeae2c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2275884
Reviewed-by: Mugdha Lakhani <nator@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Ehimare Okoyomon <eokoyomon@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#785027}

chrome/android/java/src/org/chromium/chrome/browser/page_info/ChromePageInfoControllerDelegate.java

@@ -31,10 +31,9 @@
 import org.chromium.chrome.browser.vr.VrModuleProvider;
 import org.chromium.components.content_settings.CookieControlsBridge;
 import org.chromium.components.content_settings.CookieControlsObserver;
+import org.chromium.components.embedder_support.browser_context.BrowserContextHandle;
 import org.chromium.components.feature_engagement.EventConstants;
 import org.chromium.components.page_info.PageInfoControllerDelegate;
-import org.chromium.components.page_info.PageInfoControllerDelegate.OfflinePageState;
-import org.chromium.components.page_info.PageInfoControllerDelegate.PreviewPageState;
 import org.chromium.components.page_info.PageInfoView.PageInfoViewParams;
 import org.chromium.components.security_state.ConnectionSecurityLevel;
 import org.chromium.components.security_state.SecurityStateModel;
@@ -54,6 +53,7 @@
 public class ChromePageInfoControllerDelegate extends PageInfoControllerDelegate {
     private final WebContents mWebContents;
     private final Context mContext;
+    private final Profile mProfile;
     private String mOfflinePageCreationDate;
     private OfflinePageLoadUrlDelegate mOfflinePageLoadUrlDelegate;
 
@@ -69,16 +69,14 @@ public ChromePageInfoControllerDelegate(Context context, WebContents webContents
                 CookieControlsBridge.isCookieControlsEnabled(Profile.fromWebContents(webContents)));
         mContext = context;
         mWebContents = webContents;
+        mProfile = Profile.fromWebContents(mWebContents);
+
         mPreviewPageState = getPreviewPageStateAndRecordUma();
         initOfflinePageParams();
         mOfflinePageLoadUrlDelegate = offlinePageLoadUrlDelegate;
         initHttpsImageCompressionStateAndRecordUMA();
     }
 
-    private Profile profile() {
-        return Profile.fromWebContents(mWebContents);
-    }
-
     /**
      * Return the state of the webcontents showing the preview.
      */
@@ -93,7 +91,7 @@ private Profile profile() {
                     : PreviewPageState.INSECURE_PAGE_PREVIEW;
 
             PreviewsUma.recordPageInfoOpened(bridge.getPreviewsType(mWebContents));
-            TrackerFactory.getTrackerForProfile(profile()).notifyEvent(
+            TrackerFactory.getTrackerForProfile(mProfile).notifyEvent(
                     EventConstants.PREVIEWS_VERBOSE_STATUS_OPENED);
         }
         return previewPageState;
@@ -254,9 +252,17 @@ public void showSiteSettings(String url) {
     @Override
     @NonNull
     public CookieControlsBridge createCookieControlsBridge(CookieControlsObserver observer) {
-        Profile profile = Profile.fromWebContents(mWebContents);
         return new CookieControlsBridge(observer, mWebContents,
-                profile.isOffTheRecord() ? profile.getOriginalProfile() : null);
+                mProfile.isOffTheRecord() ? mProfile.getOriginalProfile() : null);
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    @NonNull
+    public BrowserContextHandle getBrowserContext() {
+        return mProfile;
     }
 
     @VisibleForTesting

components/page_info/android/BUILD.gn

@@ -82,6 +82,7 @@ android_library("java") {
     "java/src/org/chromium/components/page_info/PageInfoCookiesController.java",
     "java/src/org/chromium/components/page_info/PageInfoDialog.java",
     "java/src/org/chromium/components/page_info/PageInfoFeatureList.java",
+    "java/src/org/chromium/components/page_info/PageInfoMainPageController.java",
     "java/src/org/chromium/components/page_info/PageInfoPermissionsController.java",
     "java/src/org/chromium/components/page_info/PageInfoRowView.java",
     "java/src/org/chromium/components/page_info/PageInfoSubpage.java",

components/page_info/android/java/res/layout/page_info_subpage.xml

@@ -11,57 +11,50 @@
     xmlns:tools="http://schemas.android.com/tools"
     android:layout_width="match_parent"
     android:layout_height="match_parent"
-    android:paddingBottom="8dp"
-    android:orientation="vertical"
-    android:background="@color/sheet_bg_color">
+    android:background="@color/sheet_bg_color"
+    android:orientation="vertical">
 
-    <RelativeLayout
+    <view
+        android:id="@+id/subpage_url"
+        class="org.chromium.components.page_info.PageInfoView$ElidedUrlTextView"
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
-        android:paddingBottom="12dp"
-        android:paddingEnd="@dimen/page_info_popup_padding_sides"
-        android:paddingStart="@dimen/page_info_popup_padding_sides" >
+        android:ellipsize="end"
+        android:lineSpacingExtra="6dp"
+        android:paddingVertical="16dp"
+        android:textAlignment="center"
+        android:textAppearance="@style/TextAppearance.TextLarge.Primary" />
 
-        <view class="org.chromium.components.page_info.PageInfoView$ElidedUrlTextView"
-            android:id="@+id/subpage_url"
-            android:layout_width="match_parent"
-            android:layout_height="wrap_content"
-            android:ellipsize="end"
-            android:lineSpacingExtra="6dp"
-            android:paddingTop="16dp"
-            android:paddingBottom="16dp"
-            android:textAlignment="center"
-            android:textAppearance="@style/TextAppearance.TextLarge.Primary" />
+    <LinearLayout
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:orientation="horizontal"
+        android:gravity="center_vertical"
+        android:paddingHorizontal="16dp">
 
-        <ImageView
+        <org.chromium.ui.widget.ChromeImageButton
             android:id="@+id/subpage_back_button"
             android:layout_width="wrap_content"
             android:layout_height="wrap_content"
-            android:layout_below="@id/subpage_url"
-            tools:ignore="ContentDescription"
-            android:layout_marginEnd="16dp"
-            android:layout_centerVertical="true"
-            android:layout_alignParentStart="true"
+            android:padding="12dp"
+            android:layout_marginStart="-12dp"
+            android:layout_marginEnd="4dp"
+            android:background="?attr/selectableItemBackgroundBorderless"
             android:src="@drawable/ic_arrow_back_white_24dp"
             app:tint="@color/default_icon_color" />
 
         <TextView
             android:id="@+id/subpage_title"
             android:layout_width="match_parent"
             android:layout_height="wrap_content"
-            android:layout_below="@id/subpage_url"
-            android:layout_toEndOf="@id/subpage_back_button"
-            android:gravity="center_vertical"
-            android:paddingBottom="12dp"
-            app:chromeDrawableTint="@color/default_icon_color"
-            android:textAppearance="@style/TextAppearance.TextLarge.Primary"/>
-
-    </RelativeLayout>
+            android:layout_marginVertical="12dp"
+            android:textAppearance="@style/TextAppearance.TextLarge.Primary" />
+    </LinearLayout>
 
     <!-- Programmatically add page specific inner view here -->
     <FrameLayout
         android:id="@+id/placeholder"
         android:layout_width="match_parent"
-        android:layout_height="match_parent"/>
+        android:layout_height="match_parent" />
 
 </LinearLayout>

components/page_info/android/java/src/org/chromium/components/page_info/PageInfoConnectionController.java

@@ -11,13 +11,14 @@
  * Class for controlling the page info connection section.
  */
 public class PageInfoConnectionController implements PageInfoSubpageController {
-    private PageInfoController mMainController;
-    private PageInfoViewV2 mView;
+    private PageInfoMainPageController mMainController;
+    private PageInfoRowView mRowView;
     private String mTitle;
 
-    public PageInfoConnectionController(PageInfoController mainController, PageInfoViewV2 view) {
+    public PageInfoConnectionController(
+            PageInfoMainPageController mainController, PageInfoRowView view) {
         mMainController = mainController;
-        mView = view;
+        mRowView = view;
     }
 
     private void launchSubpage() {
@@ -36,7 +37,10 @@ public View createViewForSubpage(ViewGroup parent) {
     }
 
     @Override
-    public void willRemoveSubpage() {}
+    public void onSubPageAttached() {}
+
+    @Override
+    public void onSubpageRemoved() {}
 
     public void setConnectionInfo(PageInfoView.ConnectionInfoParams params) {
         mTitle = params.summary != null ? params.summary.toString() : null;
@@ -45,6 +49,6 @@ public void setConnectionInfo(PageInfoView.ConnectionInfoParams params) {
         rowParams.subtitle = params.message != null ? params.message.toString() : null;
         rowParams.visible = rowParams.title != null || rowParams.subtitle != null;
         rowParams.clickCallback = this::launchSubpage;
-        mView.getConnectionRowView().setParams(rowParams);
+        mRowView.setParams(rowParams);
     }
 }

components/page_info/android/java/src/org/chromium/components/page_info/PageInfoController.java

@@ -27,7 +27,6 @@
 import androidx.core.view.ViewCompat;
 
 import org.chromium.base.ApiCompatibilityUtils;
-import org.chromium.base.Consumer;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.NativeMethods;
 import org.chromium.base.metrics.RecordUserAction;
@@ -37,6 +36,7 @@
 import org.chromium.components.content_settings.CookieControlsObserver;
 import org.chromium.components.content_settings.CookieControlsStatus;
 import org.chromium.components.dom_distiller.core.DomDistillerUrlUtils;
+import org.chromium.components.embedder_support.browser_context.BrowserContextHandle;
 import org.chromium.components.embedder_support.util.UrlUtilities;
 import org.chromium.components.omnibox.AutocompleteSchemeClassifier;
 import org.chromium.components.omnibox.OmniboxUrlEmphasizer;
@@ -65,9 +65,9 @@
 /**
  * Java side of Android implementation of the page info UI.
  */
-public class PageInfoController implements ModalDialogProperties.Controller,
-                                           SystemSettingsActivityRequiredListener,
-                                           CookieControlsObserver {
+public class PageInfoController
+        implements PageInfoMainPageController, ModalDialogProperties.Controller,
+                   SystemSettingsActivityRequiredListener, CookieControlsObserver {
     @IntDef({OpenedFromSource.MENU, OpenedFromSource.TOOLBAR, OpenedFromSource.VR})
     @Retention(RetentionPolicy.SOURCE)
     public @interface OpenedFromSource {
@@ -117,8 +117,6 @@ public class PageInfoController implements ModalDialogProperties.Controller,
     // task is pending.
     private Runnable mPendingRunAfterDismissTask;
 
-    private Consumer<Runnable> mRunAfterDismissConsumer;
-
     // Reference to last created PageInfoController for testing.
     private static WeakReference<PageInfoController> sLastPageInfoControllerForTesting;
 
@@ -166,12 +164,6 @@ public PageInfoController(WebContents webContents, int securityLevel, String pub
         mDelegate = delegate;
         mIsV2Enabled = PageInfoFeatureList.isEnabled(PageInfoFeatureList.PAGE_INFO_V2);
         mPermissionParamsListBuilderDelegate = permissionParamsListBuilderDelegate;
-        mRunAfterDismissConsumer = new Consumer<Runnable>() {
-            @Override
-            public void accept(Runnable r) {
-                runAfterDismiss(r);
-            }
-        };
         PageInfoViewParams viewParams = new PageInfoViewParams();
 
         mWindowAndroid = webContents.getTopLevelNativeWindow();
@@ -249,8 +241,8 @@ public void accept(Runnable r) {
             if (mCookieBridge != null) mCookieBridge.onUiClosing();
         };
 
-        mDelegate.initPreviewUiParams(viewParams, mRunAfterDismissConsumer);
-        mDelegate.initOfflinePageUiParams(viewParams, mRunAfterDismissConsumer);
+        mDelegate.initPreviewUiParams(viewParams, this::runAfterDismiss);
+        mDelegate.initOfflinePageUiParams(viewParams, this::runAfterDismiss);
 
         if (!mIsInternalPage && !mDelegate.isShowingOfflinePage() && !mDelegate.isShowingPreview()
                 && mDelegate.isInstantAppAvailable(mFullUrl)) {
@@ -273,10 +265,12 @@ public void accept(Runnable r) {
         if (isSheet(mContext)) mView.setBackgroundColor(Color.WHITE);
         if (mIsV2Enabled) {
             PageInfoViewV2 view2 = (PageInfoViewV2) mView;
-            mConnectionController = new PageInfoConnectionController(this, view2);
-            mPermissionsController = new PageInfoPermissionsController(this, view2);
-            mCookiesController =
-                    new PageInfoCookiesController(this, view2, viewParams.cookieControlsShown);
+            mConnectionController =
+                    new PageInfoConnectionController(this, view2.getConnectionRowView());
+            mPermissionsController =
+                    new PageInfoPermissionsController(this, view2.getPermissionsRowView());
+            mCookiesController = new PageInfoCookiesController(
+                    this, view2.getCookiesRowView(), viewParams.cookieControlsShown, mFullUrl);
         } else {
             mView.showPerformanceInfo(mDelegate.shouldShowPerformanceBadge(mFullUrl));
             mView.showHttpsImageCompressionInfo(mDelegate.isHttpsImageCompressionApplied());
@@ -571,10 +565,16 @@ PageInfoControllerDelegate getDelegate() {
         return mDelegate;
     }
 
+    @Override
+    public BrowserContextHandle getBrowserContext() {
+        return mDelegate.getBrowserContext();
+    }
+
     /**
      * Launches a subpage with the specified params.
      */
-    void launchSubpage(PageInfoSubpageController controller) {
+    @Override
+    public void launchSubpage(PageInfoSubpageController controller) {
         mSubpageController = controller;
         PageInfoSubpage.Params subpageParams = new PageInfoSubpage.Params();
         subpageParams.url = mDisplayUrlBuilder;
@@ -583,10 +583,12 @@ void launchSubpage(PageInfoSubpageController controller) {
         mSubpage = new PageInfoSubpage(mContext, subpageParams);
         mSubpage.setBackButtonOnClickListener(view -> exitSubpage());
         View subview = mSubpageController.createViewForSubpage(mSubpage);
+
         if (subview != null) {
             ((FrameLayout) mSubpage.findViewById(R.id.placeholder)).addView(subview);
         }
         replaceView(mView, mSubpage);
+        controller.onSubPageAttached();
     }
 
     private ViewGroup getParent(View view) {
@@ -615,8 +617,8 @@ private void replaceView(View currentView, View newView) {
      * Switches back to the main page info view.
      */
     private void exitSubpage() {
-        mSubpageController.willRemoveSubpage();
         replaceView(mSubpage, mView);
+        mSubpageController.onSubpageRemoved();
         mSubpage = null;
         mSubpageController = null;
     }

components/page_info/android/java/src/org/chromium/components/page_info/PageInfoControllerDelegate.java

@@ -14,6 +14,7 @@
 import org.chromium.base.supplier.Supplier;
 import org.chromium.components.content_settings.CookieControlsBridge;
 import org.chromium.components.content_settings.CookieControlsObserver;
+import org.chromium.components.embedder_support.browser_context.BrowserContextHandle;
 import org.chromium.components.omnibox.AutocompleteSchemeClassifier;
 import org.chromium.components.page_info.PageInfoView.PageInfoViewParams;
 import org.chromium.ui.modaldialog.ModalDialogManager;
@@ -161,7 +162,7 @@ public boolean isShowingOfflinePage() {
     /**
      * Initialize viewParams with Offline Page UI info, if any.
      * @param viewParams The PageInfoViewParams to set state on.
-     * @param consumer Used to set "open Online" button callback for offline page.
+     * @param runAfterDismiss Used to set "open Online" button callback for offline page.
      */
     public void initOfflinePageUiParams(
             PageInfoViewParams viewParams, Consumer<Runnable> runAfterDismiss) {
@@ -205,4 +206,10 @@ public boolean isSiteSettingsAvailable() {
     @NonNull
     public abstract CookieControlsBridge createCookieControlsBridge(
             CookieControlsObserver observer);
+
+    /**
+     * @return Returns the browser context associated with this dialog.
+     */
+    @NonNull
+    public abstract BrowserContextHandle getBrowserContext();
 }

components/page_info/android/java/src/org/chromium/components/page_info/PageInfoCookiesController.java

@@ -11,20 +11,23 @@
  * Class for controlling the page info cookies section.
  */
 public class PageInfoCookiesController implements PageInfoSubpageController {
-    private PageInfoController mMainController;
-    private PageInfoViewV2 mView;
+    private PageInfoMainPageController mMainController;
+    private PageInfoRowView mRowView;
+    private String mFullUrl;
     private String mTitle;
 
-    public PageInfoCookiesController(
-            PageInfoController mainController, PageInfoViewV2 view, boolean isVisible) {
+    public PageInfoCookiesController(PageInfoMainPageController mainController,
+            PageInfoRowView rowView, boolean isVisible, String fullUrl) {
         mMainController = mainController;
-        mView = view;
-        mTitle = mView.getContext().getResources().getString(R.string.cookies_title);
+        mRowView = rowView;
+        mFullUrl = fullUrl;
+        mTitle = mRowView.getContext().getResources().getString(R.string.cookies_title);
+
         PageInfoRowView.ViewParams rowParams = new PageInfoRowView.ViewParams();
         rowParams.visible = isVisible;
         rowParams.title = mTitle;
         rowParams.clickCallback = this::launchSubpage;
-        mView.getCookiesRowView().setParams(rowParams);
+        mRowView.setParams(rowParams);
     }
 
     private void launchSubpage() {
@@ -43,11 +46,14 @@ public View createViewForSubpage(ViewGroup parent) {
     }
 
     @Override
-    public void willRemoveSubpage() {}
+    public void onSubPageAttached() {}
+
+    @Override
+    public void onSubpageRemoved() {}
 
     public void onBlockedCookiesCountChanged(int blockedCookies) {
-        String subtitle = mView.getContext().getResources().getQuantityString(
+        String subtitle = mRowView.getContext().getResources().getQuantityString(
                 R.plurals.cookie_controls_blocked_cookies, blockedCookies, blockedCookies);
-        mView.getCookiesRowView().updateSubtitle(subtitle);
+        mRowView.updateSubtitle(subtitle);
     }
 }