Initial version of the split CBD dialog

Create an initial version of the split CBD dialog. It contains a TabLayout with a basic and and advanced version of the CBD view. The basic version only has history, cache and cookies.
All changes are behind the new flag "TABS_IN_CBD".

BUG=681523
Review-Url: https://codereview.chromium.org/2644573003
Cr-Commit-Position: refs/heads/master@{#445691}
Committed: https://chromium.googlesource.com/chromium/src/+/9da80076a5063f0f2d9c0dfb609a0948c841f4c3

[dullweber, 2017-01-18 16:10:25]

The CQ bit was checked by dullweber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 16:10:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dullweber, 2017-01-18 16:11:52]

Description was changed from

==========
Create an initial version of the splitted CBD dialog. It contains a TabLayout
with a basic and and advanced version of the CBD view. The basic version only
has history, cache and cookies.
All changes are behind the new flag "TABS_IN_CBD".

BUG=681523
==========

to

==========
Create an initial version of the split CBD dialog. It contains a TabLayout with
a basic and and advanced version of the CBD view. The basic version only has
history, cache and cookies.
All changes are behind the new flag "TABS_IN_CBD".

BUG=681523
==========

[dullweber, 2017-01-18 16:12:59]

dullweber@chromium.org changed reviewers:
+ msramek@chromium.org

[dullweber, 2017-01-18 16:13:00]

Martin, please have a look at the first version of the new CBD dialog

[msramek, 2017-01-18 17:06:39]

LGTM % comments.

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/res/lay...
File chrome/android/java/res/layout/clear_browsing_data_tabs.xml (right):

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/clear_browsing_data_tabs.xml:17:
android:id="@+id/clear_browser_data_tabs"
Let's use *browsing* instead of *browser* data to be consistent with the rest of
our codebase. The fact that the old WebUI CBD dialog was called
ClearBrowserDataHandler often made it difficult to grep things in the code
search.

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/clear_browsing_data_tabs.xml:26:
android:id="@+id/clear_browser_data_viewpager"
Also here.

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/res/xml...
File chrome/android/java/res/xml/clear_browsing_data_preferences.xml (right):

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/res/xml...
chrome/android/java/res/xml/clear_browsing_data_preferences.xml:25:
android:summaryOff="@string/clear_cookies_and_site_data_summary" />
In the old dialog, there should be no "off" summary. In the new dialog, we will
have a cookie counter.

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java
(right):

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java:75:
mCheckbox.setSummaryOff(""); // No summary when unchecked.
nit: Two spaces before an inline comment. Even better, expand the comment to
explain that our redesign uses always-shown summaries.

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesAdvanced.java
(right):

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesAdvanced.java:11:
public class ClearBrowsingDataPreferencesAdvanced extends
ClearBrowsingDataPreferences {}
nit: I suggest adding some sort of // TODO: Add more dialog options into the
class body, otherwise the comment above is blatantly false :)

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataTabsFragment.java
(right):

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataTabsFragment.java:41:
// Inflate the layout for this fragment
nit: Periods after sentences plz :) Also below.

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataTabsFragment.java:44:
// Get the ViewPager and set it's PagerAdapter so that it can display items
typo: its

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataTabsFragment.java:83:
// TODO(dullweber): Use translated strings.
I know this is still behind a flag, but we generally don't land untranslated UI
strings. Please resolve the TODO in this CL.

https://codereview.chromium.org/2644573003/diff/1/chrome/app/generated_resour...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2644573003/diff/1/chrome/app/generated_resour...
chrome/app/generated_resources.grd:6947: +      <if expr="is_android">
Let's place this next to IDS_FLAGS_ENABLE_CLEAR_BROWSING_DATA_COUNTERS_* strings
so it's easier to find.

https://codereview.chromium.org/2644573003/diff/1/chrome/app/generated_resour...
chrome/app/generated_resources.grd:6948: +        <message
name="IDS_FLAGS_TABS_IN_CBD_NAME" desc="Name of the flag to enable a tabbed
version of the Clear Browsing Data dialog on android.">
nit: Android.

https://codereview.chromium.org/2644573003/diff/1/chrome/app/generated_resour...
chrome/app/generated_resources.grd:6949: +          Enable tabs for clear
browsing data dialog.
nit: Please use "Clear Browsing Data" (title case). It also makes the string
more understandable for translators, since "for clear" (preposition + active
verb) is a syntax error.

[commit-bot: I haz the power, 2017-01-18 17:52:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-18 17:52:48]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[dullweber, 2017-01-19 12:21:52]

The CQ bit was checked by dullweber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 12:22:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-19 13:09:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 13:09:29]

Dry run: This issue passed the CQ dry run.

[dullweber, 2017-01-19 13:16:15]

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/res/lay...
File chrome/android/java/res/layout/clear_browsing_data_tabs.xml (right):

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/clear_browsing_data_tabs.xml:17:
android:id="@+id/clear_browser_data_tabs"
On 2017/01/18 17:06:38, msramek wrote:
> Let's use *browsing* instead of *browser* data to be consistent with the rest
of
> our codebase. The fact that the old WebUI CBD dialog was called
> ClearBrowserDataHandler often made it difficult to grep things in the code
> search.

Done.

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/clear_browsing_data_tabs.xml:26:
android:id="@+id/clear_browser_data_viewpager"
On 2017/01/18 17:06:38, msramek wrote:
> Also here.

Done.

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/res/xml...
File chrome/android/java/res/xml/clear_browsing_data_preferences.xml (right):

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/res/xml...
chrome/android/java/res/xml/clear_browsing_data_preferences.xml:25:
android:summaryOff="@string/clear_cookies_and_site_data_summary" />
On 2017/01/18 17:06:39, msramek wrote:
> In the old dialog, there should be no "off" summary. In the new dialog, we
will
> have a cookie counter.

ok, I removed it here.

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java
(right):

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java:75:
mCheckbox.setSummaryOff(""); // No summary when unchecked.
On 2017/01/18 17:06:39, msramek wrote:
> nit: Two spaces before an inline comment. Even better, expand the comment to
> explain that our redesign uses always-shown summaries.

Done.
git cl format removes the additional space but the style guide is also using at
least two spaces (http://source.android.com/source/code-style.html). Should this
be reported as a bug?

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesAdvanced.java
(right):

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesAdvanced.java:11:
public class ClearBrowsingDataPreferencesAdvanced extends
ClearBrowsingDataPreferences {}
On 2017/01/18 17:06:39, msramek wrote:
> nit: I suggest adding some sort of // TODO: Add more dialog options into the
> class body, otherwise the comment above is blatantly false :)

yes :D

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataTabsFragment.java
(right):

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataTabsFragment.java:41:
// Inflate the layout for this fragment
On 2017/01/18 17:06:39, msramek wrote:
> nit: Periods after sentences plz :) Also below.

Done.

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataTabsFragment.java:44:
// Get the ViewPager and set it's PagerAdapter so that it can display items
On 2017/01/18 17:06:39, msramek wrote:
> typo: its

Done.

https://codereview.chromium.org/2644573003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataTabsFragment.java:83:
// TODO(dullweber): Use translated strings.
On 2017/01/18 17:06:39, msramek wrote:
> I know this is still behind a flag, but we generally don't land untranslated
UI
> strings. Please resolve the TODO in this CL.

Done.

https://codereview.chromium.org/2644573003/diff/1/chrome/app/generated_resour...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2644573003/diff/1/chrome/app/generated_resour...
chrome/app/generated_resources.grd:6947: +      <if expr="is_android">
On 2017/01/18 17:06:39, msramek wrote:
> Let's place this next to IDS_FLAGS_ENABLE_CLEAR_BROWSING_DATA_COUNTERS_*
strings
> so it's easier to find.

Done.

https://codereview.chromium.org/2644573003/diff/1/chrome/app/generated_resour...
chrome/app/generated_resources.grd:6948: +        <message
name="IDS_FLAGS_TABS_IN_CBD_NAME" desc="Name of the flag to enable a tabbed
version of the Clear Browsing Data dialog on android.">
On 2017/01/18 17:06:39, msramek wrote:
> nit: Android.

Done.

https://codereview.chromium.org/2644573003/diff/1/chrome/app/generated_resour...
chrome/app/generated_resources.grd:6949: +          Enable tabs for clear
browsing data dialog.
On 2017/01/18 17:06:39, msramek wrote:
> nit: Please use "Clear Browsing Data" (title case). It also makes the string
> more understandable for translators, since "for clear" (preposition + active
> verb) is a syntax error.

Done.

[dullweber, 2017-01-19 13:25:07]

dullweber@chromium.org changed reviewers:
+ twellington@chromium.org

[dullweber, 2017-01-19 13:25:07]

twellington@chromium.org: Please review these changes.

[msramek, 2017-01-19 13:28:40]

LGTM

https://codereview.chromium.org/2644573003/diff/20001/chrome/android/java/str...
File chrome/android/java/strings/android_chrome_strings.grd (right):

https://codereview.chromium.org/2644573003/diff/20001/chrome/android/java/str...
chrome/android/java/strings/android_chrome_strings.grd:547: basic
nit: Please use title case for titles. (It's probably going to be fully
capitalized anyway, but that's just style; if it wasn't, it certainly wouldn't
be all lowercase)

[Theresa, 2017-01-19 19:08:26]

Looks good overall

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/res...
File chrome/android/java/res/layout/clear_browsing_data_tabs.xml (right):

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/res...
chrome/android/java/res/layout/clear_browsing_data_tabs.xml:6:
<android.support.design.widget.CoordinatorLayout
xmlns:android="http://schemas.android.com/apk/res/android"
nit: put the xmlns on the next line

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/history/HistoryManager.java
(right):

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/history/HistoryManager.java:258:
: ClearBrowsingDataPreferences.class.getName();
Does it make sense to to pull this into a helper method (maybe in
PreferenceLauncher) rather than duplicating the logic?

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java
(right):

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java:77:
// views will always show the counter.
Should this comment say "The redesigned basic and advanced CBD views will always
show the checkbox summary"? Or "... will always show the checkbox summary set in
{@link #onCounterFinished()"?

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesBasic.java
(right):

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesBasic.java:8:
* A simpler version of {@link ClearBrowsingDataPreferences} with less dialog
options and more
s/less/fewer

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/str...
File chrome/android/java/strings/android_chrome_strings.grd (right):

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/str...
chrome/android/java/strings/android_chrome_strings.grd:550: Advanced
We should re-use the existing "Advanced" string, IDS_PREFS_SECTION_ADVANCED.

https://codereview.chromium.org/2644573003/diff/40001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2644573003/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:8004: +        <message
name="IDS_FLAGS_TABS_IN_CBD_NAME" desc="Name of the flag to enable a tabbed
version of the Clear Browsing Data dialog in android.">
chrome:// flags don't need to be translated, so this message and the one below
should have translateable="false".

[dullweber, 2017-01-20 09:27:46]

On 2017/01/19 19:08:26, Theresa wrote:
> Looks good overall
> 
>
https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/res...
> File chrome/android/java/res/layout/clear_browsing_data_tabs.xml (right):
> 
>
https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/res...
> chrome/android/java/res/layout/clear_browsing_data_tabs.xml:6:
> <android.support.design.widget.CoordinatorLayout
> xmlns:android="http://schemas.android.com/apk/res/android"
> nit: put the xmlns on the next line
> 
>
https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/history/HistoryManager.java
> (right):
> 
>
https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/history/HistoryManager.java:258:
> : ClearBrowsingDataPreferences.class.getName();
> Does it make sense to to pull this into a helper method (maybe in
> PreferenceLauncher) rather than duplicating the logic?
> 
>
https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java
> (right):
> 
>
https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java:77:
> // views will always show the counter.
> Should this comment say "The redesigned basic and advanced CBD views will
always
> show the checkbox summary"? Or "... will always show the checkbox summary set
in
> {@link #onCounterFinished()"?
> 
>
https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesBasic.java
> (right):
> 
>
https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesBasic.java:8:
> * A simpler version of {@link ClearBrowsingDataPreferences} with less dialog
> options and more
> s/less/fewer
> 
>
https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/str...
> File chrome/android/java/strings/android_chrome_strings.grd (right):
> 
>
https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/str...
> chrome/android/java/strings/android_chrome_strings.grd:550: Advanced
> We should re-use the existing "Advanced" string, IDS_PREFS_SECTION_ADVANCED.
> 
>
https://codereview.chromium.org/2644573003/diff/40001/chrome/app/generated_re...
> File chrome/app/generated_resources.grd (right):
> 
>
https://codereview.chromium.org/2644573003/diff/40001/chrome/app/generated_re...
> chrome/app/generated_resources.grd:8004: +        <message
> name="IDS_FLAGS_TABS_IN_CBD_NAME" desc="Name of the flag to enable a tabbed
> version of the Clear Browsing Data dialog in android.">
> chrome:// flags don't need to be translated, so this message and the one below
> should have translateable="false".

Here is a screenshot of the current state:
https://screenshot.googleplex.com/S6aJ9mCz5se.png
So far I implemented the tab bar and reduced the number of options shown in the
basic view. I still need to implement the other UI changes.

[dullweber, 2017-01-20 10:53:07]

Thanks for the review. I fixed the issues you mentioned

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/res...
File chrome/android/java/res/layout/clear_browsing_data_tabs.xml (right):

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/res...
chrome/android/java/res/layout/clear_browsing_data_tabs.xml:6:
<android.support.design.widget.CoordinatorLayout
xmlns:android="http://schemas.android.com/apk/res/android"
On 2017/01/19 19:08:26, Theresa wrote:
> nit: put the xmlns on the next line

Done.

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/history/HistoryManager.java
(right):

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/history/HistoryManager.java:258:
: ClearBrowsingDataPreferences.class.getName();
On 2017/01/19 19:08:26, Theresa wrote:
> Does it make sense to to pull this into a helper method (maybe in
> PreferenceLauncher) rather than duplicating the logic?

Sounds good. I created a helper in the PreferenceLauncher

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java
(right):

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java:77:
// views will always show the counter.
On 2017/01/19 19:08:26, Theresa wrote:
> Should this comment say "The redesigned basic and advanced CBD views will
always
> show the checkbox summary"? Or "... will always show the checkbox summary set
in
> {@link #onCounterFinished()"?

Yes, the first alternative sounds better.

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesBasic.java
(right):

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesBasic.java:8:
* A simpler version of {@link ClearBrowsingDataPreferences} with less dialog
options and more
On 2017/01/19 19:08:26, Theresa wrote:
> s/less/fewer

Done.

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/str...
File chrome/android/java/strings/android_chrome_strings.grd (right):

https://codereview.chromium.org/2644573003/diff/40001/chrome/android/java/str...
chrome/android/java/strings/android_chrome_strings.grd:550: Advanced
On 2017/01/19 19:08:26, Theresa wrote:
> We should re-use the existing "Advanced" string, IDS_PREFS_SECTION_ADVANCED.

Done.

https://codereview.chromium.org/2644573003/diff/40001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2644573003/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:8004: +        <message
name="IDS_FLAGS_TABS_IN_CBD_NAME" desc="Name of the flag to enable a tabbed
version of the Clear Browsing Data dialog in android.">
On 2017/01/19 19:08:26, Theresa wrote:
> chrome:// flags don't need to be translated, so this message and the one below
> should have translateable="false".

Done.

[Theresa, 2017-01-20 16:55:51]

lgtm

[dullweber, 2017-01-23 09:13:23]

The CQ bit was checked by dullweber@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-23 09:13:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dullweber, 2017-01-23 09:17:21]

dullweber@chromium.org changed reviewers:
+ bauerb@chromium.org, isherman@chromium.org

[dullweber, 2017-01-23 09:17:22]

bauerb@chromium.org: Please review changes in ChromeApplication.java and
android_chrome_strings.grd
isherman@chromium.org: Please review changes in histograms.xml

[Bernhard Bauer, 2017-01-23 09:53:33]

lgtm

[commit-bot: I haz the power, 2017-01-23 10:19:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-23 10:20:00]

Dry run: This issue passed the CQ dry run.

[Ilya Sherman, 2017-01-23 20:20:13]

histograms.xml lgtm

[Ilya Sherman, 2017-01-23 20:20:23]

The CQ bit was checked by isherman@chromium.org

[Ilya Sherman, 2017-01-23 20:20:24]

The patchset sent to the CQ was uploaded after l-g-t-m from msramek@chromium.org
Link to the patchset: https://codereview.chromium.org/2644573003/#ps60001
(title: "fix comments")

[commit-bot: I haz the power, 2017-01-23 20:20:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-23 20:29:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-23 20:29:58]

Failed to apply patch for chrome/browser/about_flags.cc:
While running git apply --index -p1;
  error: patch failed: chrome/browser/about_flags.cc:736
  error: chrome/browser/about_flags.cc: patch does not apply

Patch:       chrome/browser/about_flags.cc
Index: chrome/browser/about_flags.cc
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
index
6df2def717bdcc076f70cb57cd4201b194916c58..1620f3c844b59d1a64b5b817730ac568cd1780a2
100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -736,8 +736,7 @@ const FeatureEntry kFeatureEntries[] = {
     {"show-overdraw-feedback", IDS_FLAGS_SHOW_OVERDRAW_FEEDBACK,
      IDS_FLAGS_SHOW_OVERDRAW_FEEDBACK_DESCRIPTION, kOsAll,
      SINGLE_VALUE_TYPE(cc::switches::kShowOverdrawFeedback)},
-    {"ui-disable-partial-swap",
-     IDS_FLAGS_UI_PARTIAL_SWAP_NAME,
+    {"ui-disable-partial-swap", IDS_FLAGS_UI_PARTIAL_SWAP_NAME,
      IDS_FLAGS_UI_PARTIAL_SWAP_DESCRIPTION, kOsAll,
      SINGLE_DISABLE_VALUE_TYPE(switches::kUIDisablePartialSwap)},
 #if BUILDFLAG(ENABLE_WEBRTC)
@@ -1052,8 +1051,7 @@ const FeatureEntry kFeatureEntries[] = {
      IDS_FLAGS_MATERIAL_DESIGN_INK_DROP_ANIMATION_SPEED_NAME,
      IDS_FLAGS_MATERIAL_DESIGN_INK_DROP_ANIMATION_SPEED_DESCRIPTION, kOsCrOS,
      MULTI_VALUE_TYPE(kAshMaterialDesignInkDropAnimationSpeed)},
-    {"ui-slow-animations",
-     IDS_FLAGS_UI_SLOW_ANIMATIONS_NAME,
+    {"ui-slow-animations", IDS_FLAGS_UI_SLOW_ANIMATIONS_NAME,
      IDS_FLAGS_UI_SLOW_ANIMATIONS_DESCRIPTION, kOsCrOS,
      SINGLE_VALUE_TYPE(switches::kUISlowAnimations)},
     {"disable-cloud-import", IDS_FLAGS_CLOUD_IMPORT,
@@ -1990,6 +1988,9 @@ const FeatureEntry kFeatureEntries[] = {
     {"important-sites-in-cbd", IDS_FLAGS_IMPORTANT_SITES_IN_CBD_NAME,
      IDS_FLAGS_IMPORTANT_SITES_IN_CBD_DESCRIPTION, kOsAndroid,
      FEATURE_VALUE_TYPE(chrome::android::kImportantSitesInCBD)},
+    {"tabs-in-cbd", IDS_FLAGS_TABS_IN_CBD_NAME,
+     IDS_FLAGS_TABS_IN_CBD_DESCRIPTION, kOsAndroid,
+     FEATURE_VALUE_TYPE(chrome::android::kTabsInCBD)},
 #endif  // OS_ANDROID
     {"enable-pointer-events", IDS_FLAGS_EXPERIMENTAL_POINTER_EVENT_NAME,
      IDS_FLAGS_EXPERIMENTAL_POINTER_EVENT_DESCRIPTION, kOsAll,

[dullweber, 2017-01-24 09:24:55]

The CQ bit was checked by dullweber@chromium.org

[dullweber, 2017-01-24 09:24:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
msramek@chromium.org, twellington@chromium.org, isherman@chromium.org,
bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/2644573003/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2017-01-24 09:25:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 10:31:30]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1485249895841690,
"parent_rev": "a1baf0567e91b4db6634e4929335a9d6bb1a9d72", "commit_rev":
"9da80076a5063f0f2d9c0dfb609a0948c841f4c3"}

[commit-bot: I haz the power, 2017-01-24 10:32:47]

Description was changed from

==========
Create an initial version of the split CBD dialog. It contains a TabLayout with
a basic and and advanced version of the CBD view. The basic version only has
history, cache and cookies.
All changes are behind the new flag "TABS_IN_CBD".

BUG=681523
==========

to

==========
Create an initial version of the split CBD dialog. It contains a TabLayout with
a basic and and advanced version of the CBD view. The basic version only has
history, cache and cookies.
All changes are behind the new flag "TABS_IN_CBD".

BUG=681523

Review-Url: https://codereview.chromium.org/2644573003
Cr-Commit-Position: refs/heads/master@{#445691}
Committed:
https://chromium.googlesource.com/chromium/src/+/9da80076a5063f0f2d9c0dfb609a...
==========

[commit-bot: I haz the power, 2017-01-24 10:32:51]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/9da80076a5063f0f2d9c0dfb609a...