Removing iPad Tab Switcher experimental flag.

Removing iPad Tab Switcher experimental flag.

This CL should cause no behavior change compared to having
|IsTabSwitcherEnabled| always return true.

Note:
tabstrip_switch*.png , IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO,
and IDC_SWITCH_BROWSER_MODES will be removed when I'll clean up the
tab_strip_controller, in a separate CL.

BUG=687132
Review-Url: https://codereview.chromium.org/2660143002
Cr-Commit-Position: refs/heads/master@{#448972}
Committed: https://chromium.googlesource.com/chromium/src/+/7fed8128470bd4107aedd4faa9fbb0d319b355af

[jif-google, 2017-01-30 13:11:57]

Description was changed from

==========
Removing iPad Tab Switcher experimental flag.

BUG=None
==========

to

==========
Removing iPad Tab Switcher experimental flag.

Changing the code as if |IsTabSwitcherEnabled| always returned true.

BUG=None
==========

[jif-google, 2017-01-30 15:24:37]

Description was changed from

==========
Removing iPad Tab Switcher experimental flag.

Changing the code as if |IsTabSwitcherEnabled| always returned true.

BUG=None
==========

to

==========
Removing iPad Tab Switcher experimental flag.

This CL simplifies the code by assuming |IsTabSwitcherEnabled| always returns
true.

BUG=None
==========

[jif-google, 2017-01-30 15:25:24]

Description was changed from

==========
Removing iPad Tab Switcher experimental flag.

This CL simplifies the code by assuming |IsTabSwitcherEnabled| always returns
true.

BUG=None
==========

to

==========
Removing iPad Tab Switcher experimental flag.

The CL should be equivalent to having |IsTabSwitcherEnabled| always return true.

BUG=None
==========

[jif, 2017-01-30 18:00:27]

Description was changed from

==========
Removing iPad Tab Switcher experimental flag.

The CL should be equivalent to having |IsTabSwitcherEnabled| always return true.

BUG=None
==========

to

==========
Removing iPad Tab Switcher experimental flag.

The CL should be equivalent to having |IsTabSwitcherEnabled| always return true.
Notes:
IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO is still used.

BUG=None
==========

[jif, 2017-01-31 10:50:46]

Description was changed from

==========
Removing iPad Tab Switcher experimental flag.

The CL should be equivalent to having |IsTabSwitcherEnabled| always return true.
Notes:
IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO is still used.

BUG=None
==========

to

==========
Removing iPad Tab Switcher experimental flag.

The CL should be equivalent to having |IsTabSwitcherEnabled| always return true.
Notes:
IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO is still used.
Removing the Tab Strip's incognito toggle button will be done in a separate CL.


BUG=None
==========

[jif, 2017-01-31 12:16:59]

Description was changed from

==========
Removing iPad Tab Switcher experimental flag.

The CL should be equivalent to having |IsTabSwitcherEnabled| always return true.
Notes:
IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO is still used.
Removing the Tab Strip's incognito toggle button will be done in a separate CL.


BUG=None
==========

to

==========
Removing iPad Tab Switcher experimental flag.

The CL should be equivalent to having |IsTabSwitcherEnabled| always return true.
Notes:
IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO is still used.
Removing the Tab Strip's incognito toggle button will be done in a separate CL.


BUG=687132
==========

[jif-google, 2017-01-31 12:40:11]

Description was changed from

==========
Removing iPad Tab Switcher experimental flag.

The CL should be equivalent to having |IsTabSwitcherEnabled| always return true.
Notes:
IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO is still used.
Removing the Tab Strip's incognito toggle button will be done in a separate CL.


BUG=687132
==========

to

==========
Removing iPad Tab Switcher experimental flag.

The CL should be equivalent to having |IsTabSwitcherEnabled| always return true.

IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO,
IDC_SWITCH_BROWSER_MODES, and tabstrip_switch*.png will be removed when I'll
clean up the tab_strip_controller, in a separate CL.

BUG=687132
==========

[jif-google, 2017-01-31 12:41:14]

Description was changed from

==========
Removing iPad Tab Switcher experimental flag.

The CL should be equivalent to having |IsTabSwitcherEnabled| always return true.

IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO,
IDC_SWITCH_BROWSER_MODES, and tabstrip_switch*.png will be removed when I'll
clean up the tab_strip_controller, in a separate CL.

BUG=687132
==========

to

==========
Removing iPad Tab Switcher experimental flag.

This CL should cause no behavior change compared to having
|IsTabSwitcherEnabled| always return true.

tabstrip_switch*.png , IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO,
and IDC_SWITCH_BROWSER_MODES will be removed when I'll clean up the
tab_strip_controller, in a separate CL.

BUG=687132
==========

[jif-google, 2017-01-31 12:42:18]

Description was changed from

==========
Removing iPad Tab Switcher experimental flag.

This CL should cause no behavior change compared to having
|IsTabSwitcherEnabled| always return true.

tabstrip_switch*.png , IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO,
and IDC_SWITCH_BROWSER_MODES will be removed when I'll clean up the
tab_strip_controller, in a separate CL.

BUG=687132
==========

to

==========
Removing iPad Tab Switcher experimental flag.

This CL should cause no behavior change compared to having
|IsTabSwitcherEnabled| always return true.

tabstrip_switch*.png , IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO,
and IDC_SWITCH_BROWSER_MODES will be removed when I'll clean up the
tab_strip_controller, in a separate CL.

BUG=687132, 437418
==========

[jif-google, 2017-01-31 12:43:01]

Description was changed from

==========
Removing iPad Tab Switcher experimental flag.

This CL should cause no behavior change compared to having
|IsTabSwitcherEnabled| always return true.

tabstrip_switch*.png , IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO,
and IDC_SWITCH_BROWSER_MODES will be removed when I'll clean up the
tab_strip_controller, in a separate CL.

BUG=687132, 437418
==========

to

==========
Removing iPad Tab Switcher experimental flag.

This CL should cause no behavior change compared to having
|IsTabSwitcherEnabled| always return true.

tabstrip_switch*.png , IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO,
and IDC_SWITCH_BROWSER_MODES will be removed when I'll clean up the
tab_strip_controller, in a separate CL.

BUG=687132
==========

[jif, 2017-01-31 12:48:15]

Patchset #1 (id:1) has been deleted

[jif, 2017-01-31 12:48:23]

Patchset #1 (id:20001) has been deleted

[jif, 2017-01-31 12:49:26]

Description was changed from

==========
Removing iPad Tab Switcher experimental flag.

This CL should cause no behavior change compared to having
|IsTabSwitcherEnabled| always return true.

tabstrip_switch*.png , IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO,
and IDC_SWITCH_BROWSER_MODES will be removed when I'll clean up the
tab_strip_controller, in a separate CL.

BUG=687132
==========

to

==========
Removing iPad Tab Switcher experimental flag.

This CL should cause no behavior change compared to having
|IsTabSwitcherEnabled| always return true.

Note:
tabstrip_switch*.png , IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO,
and IDC_SWITCH_BROWSER_MODES will be removed when I'll clean up the
tab_strip_controller, in a separate CL.

BUG=687132
==========

[jif, 2017-01-31 12:49:33]

jif@chromium.org changed reviewers:
+ rohitrao@chromium.org

[jif, 2017-01-31 12:49:34]

PTAL (and don't miss the note in the description).

[jif-google, 2017-01-31 12:49:57]

The CQ bit was checked by jif@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 12:50:05]

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

[jif, 2017-01-31 12:53:53]

On 2017/01/31 12:49:34, jif wrote:
> PTAL (and don't miss the note in the description).

Also, please take special care about making sure I didn't do mistakes in the
logic changes :-)
If you feel like we need a 3rd pair of eyes looking at this CL, don't hesitate
to add a reviewer.

Thanks!

[commit-bot: I haz the power, 2017-01-31 13:51:06]

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

[commit-bot: I haz the power, 2017-01-31 13:51:07]

Dry run: This issue passed the CQ dry run.

[jif, 2017-01-31 19:39:43]

jif@chromium.org changed reviewers:
- jif@google.com

[rohitrao (ping after 24h), 2017-02-01 18:34:35]

https://codereview.chromium.org/2660143002/diff/40001/ios/chrome/browser/snap...
File ios/chrome/browser/snapshots/snapshot_cache.mm (right):

https://codereview.chromium.org/2660143002/diff/40001/ios/chrome/browser/snap...
ios/chrome/browser/snapshots/snapshot_cache.mm:408: return !IsIPadIdiom();
It looks like the old implementation always returned true, so this is a change
in behavior?

https://codereview.chromium.org/2660143002/diff/40001/ios/chrome/browser/snap...
ios/chrome/browser/snapshots/snapshot_cache.mm:413: return
experimental_flags::IsLRUSnapshotCacheEnabled();
Same here.

https://codereview.chromium.org/2660143002/diff/40001/ios/chrome/browser/ui/b...
File ios/chrome/browser/ui/browser_view_controller.mm (right):

https://codereview.chromium.org/2660143002/diff/40001/ios/chrome/browser/ui/b...
ios/chrome/browser/ui/browser_view_controller.mm:1131: if (!IsIPadIdiom() ||
!isChromeScheme) {
I believe under the old logic this was always true.

https://codereview.chromium.org/2660143002/diff/40001/ios/chrome/browser/ui/b...
ios/chrome/browser/ui/browser_view_controller.mm:3993: // TODO(xxx): Evaluate if
a screenshot of the tab is needed on iPad.
Is there a bug for this?

https://codereview.chromium.org/2660143002/diff/40001/ios/chrome/browser/ui/t...
File ios/chrome/browser/ui/tabs/tab_strip_controller.mm (left):

https://codereview.chromium.org/2660143002/diff/40001/ios/chrome/browser/ui/t...
ios/chrome/browser/ui/tabs/tab_strip_controller.mm:1094: [_modeToggleButton
setTag:IDC_SWITCH_BROWSER_MODES];
Can this constant can be deleted from
ios/chrome/browser/ui/commands/ios_command_ids.h now?

[jif, 2017-02-03 12:55:51]

Thanks! ptal

https://codereview.chromium.org/2660143002/diff/40001/ios/chrome/browser/snap...
File ios/chrome/browser/snapshots/snapshot_cache.mm (right):

https://codereview.chromium.org/2660143002/diff/40001/ios/chrome/browser/snap...
ios/chrome/browser/snapshots/snapshot_cache.mm:408: return !IsIPadIdiom();
On 2017/02/01 18:34:34, rohitrao wrote:
> It looks like the old implementation always returned true, so this is a change
> in behavior?

It is! Thank you.
Done.

https://codereview.chromium.org/2660143002/diff/40001/ios/chrome/browser/snap...
ios/chrome/browser/snapshots/snapshot_cache.mm:413: return
experimental_flags::IsLRUSnapshotCacheEnabled();
On 2017/02/01 18:34:34, rohitrao wrote:
> Same here.

Done.

https://codereview.chromium.org/2660143002/diff/40001/ios/chrome/browser/ui/b...
File ios/chrome/browser/ui/browser_view_controller.mm (right):

https://codereview.chromium.org/2660143002/diff/40001/ios/chrome/browser/ui/b...
ios/chrome/browser/ui/browser_view_controller.mm:1131: if (!IsIPadIdiom() ||
!isChromeScheme) {
On 2017/02/01 18:34:34, rohitrao wrote:
> I believe under the old logic this was always true.

Done.

https://codereview.chromium.org/2660143002/diff/40001/ios/chrome/browser/ui/b...
ios/chrome/browser/ui/browser_view_controller.mm:3993: // TODO(xxx): Evaluate if
a screenshot of the tab is needed on iPad.
On 2017/02/01 18:34:34, rohitrao wrote:
> Is there a bug for this?

Done.

https://codereview.chromium.org/2660143002/diff/40001/ios/chrome/browser/ui/t...
File ios/chrome/browser/ui/tabs/tab_strip_controller.mm (left):

https://codereview.chromium.org/2660143002/diff/40001/ios/chrome/browser/ui/t...
ios/chrome/browser/ui/tabs/tab_strip_controller.mm:1094: [_modeToggleButton
setTag:IDC_SWITCH_BROWSER_MODES];
On 2017/02/01 18:34:34, rohitrao wrote:
> Can this constant can be deleted from
> ios/chrome/browser/ui/commands/ios_command_ids.h now?

It's still used in the tab_strip_controller, so I'd rather do it in the
CL that does the clean up of the tab_strip_controller.
See CL description.

[jif, 2017-02-06 08:58:08]

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

[commit-bot: I haz the power, 2017-02-06 08:58:23]

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-02-06 09:54:34]

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

[commit-bot: I haz the power, 2017-02-06 09:54:35]

Dry run: This issue passed the CQ dry run.

[rohitrao (ping after 24h), 2017-02-06 14:21:09]

lgtm

[jif, 2017-02-07 15:02:51]

The CQ bit was checked by jif@chromium.org

[commit-bot: I haz the power, 2017-02-07 15:03:13]

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-02-07 15:05:19]

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

[commit-bot: I haz the power, 2017-02-07 15:05:20]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[jif, 2017-02-08 12:13:58]

The CQ bit was checked by jif@chromium.org

[jif, 2017-02-08 12:13:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
rohitrao@chromium.org
Link to the patchset: https://codereview.chromium.org/2660143002/#ps80001
(title: "Rebased.")

[commit-bot: I haz the power, 2017-02-08 12:14:12]

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-02-08 13:15:29]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1486556038906610,
"parent_rev": "2c41fd7827b4aa5a485f5ffbb8b6a26014374e6c", "commit_rev":
"7fed8128470bd4107aedd4faa9fbb0d319b355af"}

[commit-bot: I haz the power, 2017-02-08 13:16:05]

Description was changed from

==========
Removing iPad Tab Switcher experimental flag.

This CL should cause no behavior change compared to having
|IsTabSwitcherEnabled| always return true.

Note:
tabstrip_switch*.png , IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO,
and IDC_SWITCH_BROWSER_MODES will be removed when I'll clean up the
tab_strip_controller, in a separate CL.

BUG=687132
==========

to

==========
Removing iPad Tab Switcher experimental flag.

This CL should cause no behavior change compared to having
|IsTabSwitcherEnabled| always return true.

Note:
tabstrip_switch*.png , IDS_IOS_SWITCH_BROWSER_MODE_LEAVE_INCOGNITO,
and IDC_SWITCH_BROWSER_MODES will be removed when I'll clean up the
tab_strip_controller, in a separate CL.

BUG=687132

Review-Url: https://codereview.chromium.org/2660143002
Cr-Commit-Position: refs/heads/master@{#448972}
Committed:
https://chromium.googlesource.com/chromium/src/+/7fed8128470bd4107aedd4faa9fb...
==========

[commit-bot: I haz the power, 2017-02-08 13:16:06]

Committed patchset #3 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/7fed8128470bd4107aedd4faa9fb...