[ios] Add a delegate to WebStateList class.

[ios] Add a delegate to WebStateList class.

In order to attach the tab helpers to WebState when they are inserted
in the WebStateList, without having the list of tab helpers known to
the WebStateList.

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

[sdefresne, 2017-03-13 17:11:23]

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

[commit-bot: I haz the power, 2017-03-13 17:11:46]

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

[sdefresne, 2017-03-13 17:15:30]

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

[sdefresne, 2017-03-13 17:15:31]

Please take a look.

https://codereview.chromium.org/2748793002/diff/1/ios/chrome/browser/tabs/tab...
File ios/chrome/browser/tabs/tab_model_web_state_list_delegate.h (right):

https://codereview.chromium.org/2748793002/diff/1/ios/chrome/browser/tabs/tab...
ios/chrome/browser/tabs/tab_model_web_state_list_delegate.h:18: void
WillAddWebState(web::WebState* web_state) override;
Upstream TabStripModelDelegate does not receive the TabStripModel so I don't
pass it either, but I'm wondering whether I should pass it. WDYT?

[commit-bot: I haz the power, 2017-03-13 18:47:40]

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

[commit-bot: I haz the power, 2017-03-13 18:47:41]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[rohitrao (ping after 24h), 2017-03-14 11:51:12]

https://codereview.chromium.org/2748793002/diff/1/ios/chrome/browser/tabs/tab...
File ios/chrome/browser/tabs/tab_model_web_state_list_delegate.h (right):

https://codereview.chromium.org/2748793002/diff/1/ios/chrome/browser/tabs/tab...
ios/chrome/browser/tabs/tab_model_web_state_list_delegate.h:18: void
WillAddWebState(web::WebState* web_state) override;
On 2017/03/13 17:15:30, sdefresne wrote:
> Upstream TabStripModelDelegate does not receive the TabStripModel so I don't
> pass it either, but I'm wondering whether I should pass it. WDYT?

I'm fine either way.  I think this kind of API should pass the model as an
argument, but we'll also never use it for anything.

https://codereview.chromium.org/2748793002/diff/1/ios/clean/chrome/browser/mo...
File ios/clean/chrome/browser/model/browser.h (right):

https://codereview.chromium.org/2748793002/diff/1/ios/clean/chrome/browser/mo...
ios/clean/chrome/browser/model/browser.h:24: WebStateListDelegate*
web_state_list_delegate);
Why is this owned by the BrowserList and passed into the Browser, instead of
just being created and owned by the Browser?

[sdefresne, 2017-03-14 12:07:19]

On 2017/03/14 11:51:12, rohitrao wrote:
>
https://codereview.chromium.org/2748793002/diff/1/ios/chrome/browser/tabs/tab...
> File ios/chrome/browser/tabs/tab_model_web_state_list_delegate.h (right):
> 
>
https://codereview.chromium.org/2748793002/diff/1/ios/chrome/browser/tabs/tab...
> ios/chrome/browser/tabs/tab_model_web_state_list_delegate.h:18: void
> WillAddWebState(web::WebState* web_state) override;
> On 2017/03/13 17:15:30, sdefresne wrote:
> > Upstream TabStripModelDelegate does not receive the TabStripModel so I don't
> > pass it either, but I'm wondering whether I should pass it. WDYT?
> 
> I'm fine either way.  I think this kind of API should pass the model as an
> argument, but we'll also never use it for anything.

This is mostly because Delegate in Objective-C always receive the delegatee when
invoked, but I don't think it make sense in C++. I also don't think we'll use
it, so I'm not going to pass it for the moment and I'll change the API if we
ever need access to it.
 
>
https://codereview.chromium.org/2748793002/diff/1/ios/clean/chrome/browser/mo...
> File ios/clean/chrome/browser/model/browser.h (right):
> 
>
https://codereview.chromium.org/2748793002/diff/1/ios/clean/chrome/browser/mo...
> ios/clean/chrome/browser/model/browser.h:24: WebStateListDelegate*
> web_state_list_delegate);
> Why is this owned by the BrowserList and passed into the Browser, instead of
> just being created and owned by the Browser?

I expected the WebStateListDelegate to have no state, thus it would be better to
share the same WebStateList for all BrowserList (I even though of attaching it
to the BrowserState). But when implementing AttachTabHelpers, I saw that we may
want to move the SessionId from TabModel to WebStateListDelegate, thus have
state. So I'm going to upload a second patchset that move this object from
BrowserList to Browser.

[sdefresne, 2017-03-14 13:53:40]

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

[sdefresne, 2017-03-14 13:53:49]

Please take another look.

[commit-bot: I haz the power, 2017-03-14 13:53:57]

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

[rohitrao (ping after 24h), 2017-03-14 13:57:28]

lgtm

https://codereview.chromium.org/2748793002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/model/browser.h (right):

https://codereview.chromium.org/2748793002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/model/browser.h:26: WebStateList& web_state_list() {
return *web_state_list_.get(); }
Why this change?

[sdefresne, 2017-03-14 13:59:10]

On 2017/03/14 13:57:28, rohitrao wrote:
> lgtm
> 
>
https://codereview.chromium.org/2748793002/diff/20001/ios/clean/chrome/browse...
> File ios/clean/chrome/browser/model/browser.h (right):
> 
>
https://codereview.chromium.org/2748793002/diff/20001/ios/clean/chrome/browse...
> ios/clean/chrome/browser/model/browser.h:26: WebStateList& web_state_list() {
> return *web_state_list_.get(); }
> Why this change?

To avoid in-flight conflict with https://codereview.chromium.org/2741883002/
that expect Browser::web_state_list to return a reference, not a pointer.

[sdefresne, 2017-03-14 13:59:39]

The CQ bit was unchecked by sdefresne@chromium.org

[sdefresne, 2017-03-14 13:59:45]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2017-03-14 13:59:55]

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-03-14 15:07:17]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489499985472530,
"parent_rev": "5cda81b4d16e323763e51f22582a9144dc64c30e", "commit_rev":
"365a73ab9da546dc6898bef703b688c82c292527"}

[commit-bot: I haz the power, 2017-03-14 15:07:58]

Description was changed from

==========
[ios] Add a delegate to WebStateList class.

In order to attach the tab helpers to WebState when they are inserted
in the WebStateList, without having the list of tab helpers known to
the WebStateList.

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

to

==========
[ios] Add a delegate to WebStateList class.

In order to attach the tab helpers to WebState when they are inserted
in the WebStateList, without having the list of tab helpers known to
the WebStateList.

BUG=687207

Review-Url: https://codereview.chromium.org/2748793002
Cr-Commit-Position: refs/heads/master@{#456705}
Committed:
https://chromium.googlesource.com/chromium/src/+/365a73ab9da546dc6898bef703b6...
==========

[commit-bot: I haz the power, 2017-03-14 15:08:00]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/365a73ab9da546dc6898bef703b6...