Add WebStateListOrderController to control WebState insertion.

Add WebStateListOrderController to control WebState insertion.

Introduce WebStateListOrderController to control the position of
insertion of WebState and refactor TabModel to use (using new API
of WebStateList).

BUG=687207
Review-Url: https://codereview.chromium.org/2699833004
Cr-Commit-Position: refs/heads/master@{#452496}
Committed: https://chromium.googlesource.com/chromium/src/+/ef097a7cd0861251cd05ee6adfe0293eeb56197e

[sdefresne, 2017-02-17 11:48:01]

Description was changed from

==========
Add WebStateListOrderController to control WebState insertion.

BUG=687207
==========

to

==========
Add WebStateListOrderController to control WebState insertion.

Introduce WebStateListOrderController to control the position of
insertion of WebState and refactor TabModel to use (using new API
of WebStateList).

BUG=687207
==========

[sdefresne, 2017-02-17 11:48:01]

sdefresne@chromium.org changed reviewers:
+ marq@chromium.org, rohitrao@chromium.org

[sdefresne, 2017-02-17 11:49:33]

Patchset #2 (id:20001) has been deleted

[sdefresne, 2017-02-17 11:49:39]

Patchset #1 (id:1) has been deleted

[sdefresne, 2017-02-17 11:50:15]

Please both take a look.

[sdefresne, 2017-02-17 11:50:23]

sdefresne@chromium.org changed reviewers:
+ thakis@chromium.org

[sdefresne, 2017-02-17 11:51:18]

Nico, can you take a look at the new DEPS on ui/base.

Note that the code is refactored from ios/chrome/browser to be shared between
old and new architecture for Chrome on iOS. So this is not a new DEPS but just a
DEPS that is moved.

[Nico, 2017-02-17 12:44:44]

Deps lgtm

[rohitrao (ping after 24h), 2017-02-17 13:04:12]

lgtm

https://codereview.chromium.org/2699833004/diff/40001/ios/shared/chrome/brows...
File ios/shared/chrome/browser/tabs/web_state_list.mm (right):

https://codereview.chromium.org/2699833004/diff/40001/ios/shared/chrome/brows...
ios/shared/chrome/browser/tabs/web_state_list.mm:171: // creats a New Tab (e.g.
Ctrl+T, or clicks the New Tab button), or types
creates

https://codereview.chromium.org/2699833004/diff/40001/ios/shared/chrome/brows...
File ios/shared/chrome/browser/tabs/web_state_list_order_controller_unittest.mm
(right):

https://codereview.chromium.org/2699833004/diff/40001/ios/shared/chrome/brows...
ios/shared/chrome/browser/tabs/web_state_list_order_controller_unittest.mm:65:
// no other childs.
children.

[sdefresne, 2017-02-17 13:32:18]

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

[commit-bot: I haz the power, 2017-02-17 13:32:34]

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

[marq (ping after 24h), 2017-02-17 13:33:00]

https://codereview.chromium.org/2699833004/diff/40001/ios/shared/chrome/brows...
File ios/shared/chrome/browser/tabs/web_state_list.h (right):

https://codereview.chromium.org/2699833004/diff/40001/ios/shared/chrome/brows...
ios/shared/chrome/browser/tabs/web_state_list.h:80: // WebState may be passed.
Pass kInvalidIndex for |index| to append it to the
I would have a mild preference for a separate AppendWebState method instead of
attaching magical behavior to specific input values.

https://codereview.chromium.org/2699833004/diff/40001/ios/shared/chrome/brows...
File ios/shared/chrome/browser/tabs/web_state_list.mm (right):

https://codereview.chromium.org/2699833004/diff/40001/ios/shared/chrome/brows...
ios/shared/chrome/browser/tabs/web_state_list.mm:161: if (index ==
kInvalidIndex)
The header says to pass kInvalidIndex to have the webstate appended, but this
does something different.

https://codereview.chromium.org/2699833004/diff/40001/ios/shared/chrome/brows...
ios/shared/chrome/browser/tabs/web_state_list.mm:173: // Tab to quickly look up
something. When this Tab is closed, the old one
Nit: do we need a comment in our iOS code that describes Windows keyboard
commands? Is the usage pattern described here sensible for us at all?

[commit-bot: I haz the power, 2017-02-17 13:35:08]

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

[commit-bot: I haz the power, 2017-02-17 13:35:09]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[sdefresne, 2017-02-20 15:56:57]

Patchset #2 (id:60001) has been deleted

[sdefresne, 2017-02-20 16:12:56]

marq: PTAL

https://codereview.chromium.org/2699833004/diff/40001/ios/shared/chrome/brows...
File ios/shared/chrome/browser/tabs/web_state_list.h (right):

https://codereview.chromium.org/2699833004/diff/40001/ios/shared/chrome/brows...
ios/shared/chrome/browser/tabs/web_state_list.h:80: // WebState may be passed.
Pass kInvalidIndex for |index| to append it to the
On 2017/02/17 13:33:00, marq wrote:
> I would have a mild preference for a separate AppendWebState method instead of
> attaching magical behavior to specific input values.

Done.

https://codereview.chromium.org/2699833004/diff/40001/ios/shared/chrome/brows...
File ios/shared/chrome/browser/tabs/web_state_list.mm (right):

https://codereview.chromium.org/2699833004/diff/40001/ios/shared/chrome/brows...
ios/shared/chrome/browser/tabs/web_state_list.mm:161: if (index ==
kInvalidIndex)
On 2017/02/17 13:33:00, marq wrote:
> The header says to pass kInvalidIndex to have the webstate appended, but this
> does something different.

Addressed.

https://codereview.chromium.org/2699833004/diff/40001/ios/shared/chrome/brows...
ios/shared/chrome/browser/tabs/web_state_list.mm:171: // creats a New Tab (e.g.
Ctrl+T, or clicks the New Tab button), or types
On 2017/02/17 13:04:12, rohitrao wrote:
> creates

Removed.

https://codereview.chromium.org/2699833004/diff/40001/ios/shared/chrome/brows...
ios/shared/chrome/browser/tabs/web_state_list.mm:173: // Tab to quickly look up
something. When this Tab is closed, the old one
On 2017/02/17 13:33:00, marq wrote:
> Nit: do we need a comment in our iOS code that describes Windows keyboard
> commands? Is the usage pattern described here sensible for us at all?

Removed.

[marq (ping after 24h), 2017-02-21 10:13:02]

lgtm

[sdefresne, 2017-02-23 14:45:09]

The CQ bit was checked by sdefresne@chromium.org

[sdefresne, 2017-02-23 14:45:09]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org,
rohitrao@chromium.org
Link to the patchset: https://codereview.chromium.org/2699833004/#ps90001
(title: "Address comments.")

[commit-bot: I haz the power, 2017-02-23 14:45:31]

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-23 15:41:08]

CQ is committing da patch.

Bot data: {"patchset_id": 90001, "attempt_start_ts": 1487861109033340,
"parent_rev": "ef704d88025002793b942e1df5b57e312ada65a0", "commit_rev":
"ef097a7cd0861251cd05ee6adfe0293eeb56197e"}

[commit-bot: I haz the power, 2017-02-23 15:41:48]

Description was changed from

==========
Add WebStateListOrderController to control WebState insertion.

Introduce WebStateListOrderController to control the position of
insertion of WebState and refactor TabModel to use (using new API
of WebStateList).

BUG=687207
==========

to

==========
Add WebStateListOrderController to control WebState insertion.

Introduce WebStateListOrderController to control the position of
insertion of WebState and refactor TabModel to use (using new API
of WebStateList).

BUG=687207

Review-Url: https://codereview.chromium.org/2699833004
Cr-Commit-Position: refs/heads/master@{#452496}
Committed:
https://chromium.googlesource.com/chromium/src/+/ef097a7cd0861251cd05ee6adfe0...
==========

[commit-bot: I haz the power, 2017-02-23 15:41:49]

Committed patchset #3 (id:90001) as
https://chromium.googlesource.com/chromium/src/+/ef097a7cd0861251cd05ee6adfe0...