Implement additional parts of the new ui for CBD.

chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataTabsFragment.java

Index: chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataTabsFragment.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataTabsFragment.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataTabsFragment.java
index cec1dc5ef39c4e72f7e70dae2ee6e9b16988d965..133c4a9f90801dc643791f4f4d7c24137c6cebd0 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataTabsFragment.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataTabsFragment.java
@@ -10,6 +10,8 @@ import android.content.Context;
 import android.os.Bundle;
 import android.support.design.widget.TabLayout;
 import android.support.v13.app.FragmentPagerAdapter;
+import android.support.v4.text.TextUtilsCompat;
+import android.support.v4.view.ViewCompat;
 import android.support.v4.view.ViewPager;
 import android.view.LayoutInflater;
 import android.view.View;
@@ -17,13 +19,16 @@ import android.view.ViewGroup;
 
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeFeatureList;
+import org.chromium.chrome.browser.preferences.PrefServiceBridge;
 import org.chromium.chrome.browser.preferences.Preferences;
 
+import java.util.Locale;
+
 /**
  * Fragment with a {@link TabLayout} containing a basic and an advanced version of the CBD dialog.
  */
 public class ClearBrowsingDataTabsFragment extends Fragment {
-    public static final String TAG = ClearBrowsingDataTabsFragment.class.getSimpleName();
+    public static final int CBD_TAB_COUNT = 2;
 
     public ClearBrowsingDataTabsFragment() {}
 
@@ -36,6 +41,19 @@ public class ClearBrowsingDataTabsFragment extends Fragment {
         super.onCreate(savedInstanceState);
     }
 
+    /*
+     * RTL is broken for ViewPager: https://code.google.com/p/android/issues/detail?id=56831
+     * This class works around this issue by inserting the tabs in inverse order if RTL is active.
+     * The TabLayout needs to be set to LTR for this to work.
+     */
+    private static int adjustIndexForDirectionality(int index) {
+        if (TextUtilsCompat.getLayoutDirectionFromLocale(Locale.getDefault())
+                == ViewCompat.LAYOUT_DIRECTION_RTL) {
+            return CBD_TAB_COUNT - 1 - index;
+        }
+        return index;
+    }
+
     @Override
     public View onCreateView(
             LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
@@ -50,6 +68,13 @@ public class ClearBrowsingDataTabsFragment extends Fragment {
         // Give the TabLayout the ViewPager.
         TabLayout tabLayout = (TabLayout) view.findViewById(R.id.clear_browsing_data_tabs);
         tabLayout.setupWithViewPager(viewPager);
+        tabLayout.addOnTabSelectedListener(new TabSelectListener());
+        int tabIndex = adjustIndexForDirectionality(
+                PrefServiceBridge.getInstance().getLastSelectedClearBrowsingDataTab());
+        TabLayout.Tab tab = tabLayout.getTabAt(tabIndex);
+        if (tab != null) {
+            tab.select();
+        }
 
         // Remove elevation to avoid shadow between title and tabs.
         Preferences activity = (Preferences) getActivity();
@@ -68,11 +93,12 @@ public class ClearBrowsingDataTabsFragment extends Fragment {
 
         @Override
         public int getCount() {
-            return 2;
+            return CBD_TAB_COUNT;
         }
 
         @Override
         public Fragment getItem(int position) {
+            position = adjustIndexForDirectionality(position);
             switch (position) {
                 case 0:
                     return new ClearBrowsingDataPreferencesBasic();
@@ -85,6 +111,7 @@ public class ClearBrowsingDataTabsFragment extends Fragment {
 
         @Override
         public CharSequence getPageTitle(int position) {
+            position = adjustIndexForDirectionality(position);
             switch (position) {
                 case 0:
                     return mContext.getString(R.string.clear_browsing_data_basic_tab_title);
@@ -95,4 +122,18 @@ public class ClearBrowsingDataTabsFragment extends Fragment {
             }
         }
     }
+
+    private static class TabSelectListener implements TabLayout.OnTabSelectedListener {
+        @Override
+        public void onTabSelected(TabLayout.Tab tab) {
+            int tabIndex = adjustIndexForDirectionality(tab.getPosition());
+            PrefServiceBridge.getInstance().setLastSelectedClearBrowsingDataTab(tabIndex);

[Theresa, 2017/01/27 03:30:11]

To check my understanding here:

LTR: Tab 0 = Basic   Tab 1 = Advanced
RTL: Tab 0 = Advanced   Tab 1 = Basic

Here's a scenario:
In LTR mode I select the "basic" tab (0); that gets stored. I close Chrome,
change my layout to RTL and reload. The selected index is adjusted: 2 - 1 - 0 =
1, so the "basic" tab is still selected. I toggle selection between "advanced"
and "basic"; an adjusted index gets stored: 2 - 1 - 1 = 0 is stored.

Is that correct?

[dullweber, 2017/01/30 11:36:44]

That's right. adjustIndexForDirectionality will make sure that we always store 0
for the basic tab and 1 for advanced. This was the most reliable solution I
found. 

I also tried several wrapper classes for the ViewPager that try to fix this but
they all had either problems to work together with the TabLayout, didn't show
the right tab when tab.select() was called or had broken scrolling. My attempts
at fixing this were not very successful, probably because I didn't find all
places that required a translation for RTL.
I tried this one
(https://android.googlesource.com/platform/packages/apps/DeskClock/+/master/sr...)
and a few others I found here: go/dkrjr

This project seems to have a proper fix but I don't know how to include it
properly without licensing issues:
https://github.com/diego-gomez-olvera/RtlViewPager

In general I think it is sad that the ViewPager doesn't work correctly although
a ticket was filed in 2013
(https://code.google.com/p/android/issues/detail?id=56831). Do you know if there
is anyone that could be asked about this?

[Theresa, 2017/01/30 16:21:05]

There are a couple of internal bugs that reference that external bug, as well as
others for ViewPager in RTL. It looks like the Android folks are aware but it
hasn't been a high priority. We could ping one of the internal bugs.


I think ideally, we would have a ChromeViewPagerCompat (or something like that)
in our ui/android/java/src/org/chromium/ui/widget/  that encapsulates this work
around. It doesn't look like we currently use have any other ViewPagers that
support swipe gesture in our code, so this could be a TODO for later.

[dullweber, 2017/02/01 11:47:57]

Ok, I added a TODO

+        }
+
+        @Override
+        public void onTabUnselected(TabLayout.Tab tab) {}
+
+        @Override
+        public void onTabReselected(TabLayout.Tab tab) {}
+    }
 }