Replaced webPageOrderedClose with WebStateDelegate API.

Replaced webPageOrderedClose with WebStateDelegate API.

New WebStateDelegate::CloseWebState mirrors content's
WebContents::CloseContents.

BUG=674991
Review-Url: https://codereview.chromium.org/2737353006
Cr-Commit-Position: refs/heads/master@{#456588}
Committed: https://chromium.googlesource.com/chromium/src/+/b46b2120bd9c8b260513015717a9a09792f71714

[Eugene But (OOO till 7-30), 2017-03-10 19:49:29]

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

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

lgtm with one question about dangling pointers

https://codereview.chromium.org/2737353006/diff/1/ios/web/public/test/fakes/t...
File ios/web/public/test/fakes/test_web_state_delegate.h (right):

https://codereview.chromium.org/2737353006/diff/1/ios/web/public/test/fakes/t...
ios/web/public/test/fakes/test_web_state_delegate.h:29: WebState* web_state =
nullptr;
It looks like we sometimes destroy the WebState when CloseWebState() is called,
so this may be a dangling pointer?  Is it worth making this a void* or
something, to make sure no one actually uses it?  Or should it keep the WebState
alive?

https://codereview.chromium.org/2737353006/diff/1/ios/web/public/test/fakes/t...
File ios/web/public/test/fakes/test_web_state_delegate.mm (right):

https://codereview.chromium.org/2737353006/diff/1/ios/web/public/test/fakes/t...
ios/web/public/test/fakes/test_web_state_delegate.mm:74:
child_windows_.erase(child_windows_.begin() + i);
If the WebState is owned by child_windows_, we destroy it here.

https://codereview.chromium.org/2737353006/diff/1/ios/web/web_state/web_state...
File ios/web/web_state/web_state_impl.mm (right):

https://codereview.chromium.org/2737353006/diff/1/ios/web/web_state/web_state...
ios/web/web_state/web_state_impl.mm:519: return delegate_->CloseWebState(this);
Any need for "return" if this method returns void?

[Eugene But (OOO till 7-30), 2017-03-14 01:20:24]

Thanks!

https://codereview.chromium.org/2737353006/diff/1/ios/web/public/test/fakes/t...
File ios/web/public/test/fakes/test_web_state_delegate.h (right):

https://codereview.chromium.org/2737353006/diff/1/ios/web/public/test/fakes/t...
ios/web/public/test/fakes/test_web_state_delegate.h:29: WebState* web_state =
nullptr;
On 2017/03/13 12:30:02, rohitrao wrote:
> It looks like we sometimes destroy the WebState when CloseWebState() is
called,
> so this may be a dangling pointer?  Is it worth making this a void* or
> something, to make sure no one actually uses it?  Or should it keep the
WebState
> alive?
We can't compare void* with WebState*, so that's not going to work for
WebStateImplTest::DelegateTest. Keeping it alive sounds like a reasonable thing
to do.

https://codereview.chromium.org/2737353006/diff/1/ios/web/public/test/fakes/t...
File ios/web/public/test/fakes/test_web_state_delegate.mm (right):

https://codereview.chromium.org/2737353006/diff/1/ios/web/public/test/fakes/t...
ios/web/public/test/fakes/test_web_state_delegate.mm:74:
child_windows_.erase(child_windows_.begin() + i);
On 2017/03/13 12:30:02, rohitrao wrote:
> If the WebState is owned by child_windows_, we destroy it here.
Good catch. Fixed by having |closed_child_windows_| vector.

https://codereview.chromium.org/2737353006/diff/1/ios/web/web_state/web_state...
File ios/web/web_state/web_state_impl.mm (right):

https://codereview.chromium.org/2737353006/diff/1/ios/web/web_state/web_state...
ios/web/web_state/web_state_impl.mm:519: return delegate_->CloseWebState(this);
On 2017/03/13 12:30:02, rohitrao wrote:
> Any need for "return" if this method returns void?

Done.

[Eugene But (OOO till 7-30), 2017-03-14 01:20:29]

The CQ bit was checked by eugenebut@chromium.org

[Eugene But (OOO till 7-30), 2017-03-14 01:20:29]

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/2737353006/#ps20001
(title: "Addressed review comments")

[commit-bot: I haz the power, 2017-03-14 01:20: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 02:43:32]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489454429463570,
"parent_rev": "ff263391641f59daed81c273d9eb28a390759239", "commit_rev":
"b46b2120bd9c8b260513015717a9a09792f71714"}

[commit-bot: I haz the power, 2017-03-14 02:44:10]

Description was changed from

==========
Replaced webPageOrderedClose with WebStateDelegate API.

New WebStateDelegate::CloseWebState mirrors content's
WebContents::CloseContents.

BUG=674991
==========

to

==========
Replaced webPageOrderedClose with WebStateDelegate API.

New WebStateDelegate::CloseWebState mirrors content's
WebContents::CloseContents.

BUG=674991

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

[commit-bot: I haz the power, 2017-03-14 02:44:11]

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