Removed CRWWebController dependency from NativeAppNavigationController.

Removed CRWWebController dependency from NativeAppNavigationController.

Conform NativeAppNavigationController to CRWWebStateObserver instead of
CRWWebControllerObserver.

BUG=675005, 619984
Review-Url: https://codereview.chromium.org/2731443003
Cr-Commit-Position: refs/heads/master@{#454751}
Committed: https://chromium.googlesource.com/chromium/src/+/4736030020471fa4dca4eb223925cfdac92267df

[Eugene But (OOO till 7-30), 2017-03-02 22:20:53]

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

[commit-bot: I haz the power, 2017-03-02 22:21:35]

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-03-02 22:28:57]

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

[commit-bot: I haz the power, 2017-03-02 22:28:58]

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...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Eugene But (OOO till 7-30), 2017-03-02 23:00:22]

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

[commit-bot: I haz the power, 2017-03-02 23:01:03]

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-03-02 23:54:49]

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

[commit-bot: I haz the power, 2017-03-02 23:54:50]

Dry run: This issue passed the CQ dry run.

[Eugene But (OOO till 7-30), 2017-03-02 23:56:10]

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

[pkl (ping after 24h if needed), 2017-03-03 16:45:31]

LGTM with comments

https://codereview.chromium.org/2731443003/diff/20001/ios/chrome/browser/nati...
File ios/chrome/browser/native_app_launcher/native_app_navigation_controller.h
(right):

https://codereview.chromium.org/2731443003/diff/20001/ios/chrome/browser/nati...
ios/chrome/browser/native_app_launcher/native_app_navigation_controller.h:10:
#import
"ios/chrome/browser/native_app_launcher/native_app_navigation_controller_protocol.h"
Can this be moved to the .mm file too?

https://codereview.chromium.org/2731443003/diff/20001/ios/chrome/browser/nati...
ios/chrome/browser/native_app_launcher/native_app_navigation_controller.h:11:
#import "ios/web/public/web_state/crw_web_controller_observer.h"
Can you remove this #import too?

https://codereview.chromium.org/2731443003/diff/20001/ios/chrome/browser/nati...
File ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm
(right):

https://codereview.chromium.org/2731443003/diff/20001/ios/chrome/browser/nati...
ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm:227:
#pragma mark Private methods
nit: Can we just use one line? Like this:
#pragma mark - Private methods

[Eugene But (OOO till 7-30), 2017-03-03 19:42:06]

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

[Eugene But (OOO till 7-30), 2017-03-03 19:42:07]

Thanks! +Mike for committer review

https://codereview.chromium.org/2731443003/diff/20001/ios/chrome/browser/nati...
File ios/chrome/browser/native_app_launcher/native_app_navigation_controller.h
(right):

https://codereview.chromium.org/2731443003/diff/20001/ios/chrome/browser/nati...
ios/chrome/browser/native_app_launcher/native_app_navigation_controller.h:10:
#import
"ios/chrome/browser/native_app_launcher/native_app_navigation_controller_protocol.h"
On 2017/03/03 16:45:30, pklpkl wrote:
> Can this be moved to the .mm file too?

Done.

https://codereview.chromium.org/2731443003/diff/20001/ios/chrome/browser/nati...
ios/chrome/browser/native_app_launcher/native_app_navigation_controller.h:11:
#import "ios/web/public/web_state/crw_web_controller_observer.h"
On 2017/03/03 16:45:30, pklpkl wrote:
> Can you remove this #import too?

Done.

https://codereview.chromium.org/2731443003/diff/20001/ios/chrome/browser/nati...
File ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm
(right):

https://codereview.chromium.org/2731443003/diff/20001/ios/chrome/browser/nati...
ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm:227:
#pragma mark Private methods
On 2017/03/03 16:45:30, pklpkl wrote:
> nit: Can we just use one line? Like this:
> #pragma mark - Private methods
Done. Fixed in other 2 places as well.

[michaeldo, 2017-03-04 00:35:38]

https://codereview.chromium.org/2731443003/diff/40001/ios/chrome/browser/nati...
File ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm
(right):

https://codereview.chromium.org/2731443003/diff/40001/ios/chrome/browser/nati...
ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm:85:
_webStateObserver.reset(new web::WebStateObserverBridge(webState, self));
Use base::MakeUnique here instead of new?

_webStateObserver = base::MakeUnique<web::WebStateObserverBridge>(webState,
self);

https://codereview.chromium.org/2731443003/diff/40001/ios/chrome/browser/nati...
ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm:221:
_webState = nullptr;
Shouldn't we also cleanup/reset |_webStateObserver| here?

However, in order to retain similar behavior, do we want to change the value of
|_webState| at all?

[Eugene But (OOO till 7-30), 2017-03-04 01:01:36]

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

[Eugene But (OOO till 7-30), 2017-03-04 01:01:50]

Thanks!

https://codereview.chromium.org/2731443003/diff/40001/ios/chrome/browser/nati...
File ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm
(right):

https://codereview.chromium.org/2731443003/diff/40001/ios/chrome/browser/nati...
ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm:85:
_webStateObserver.reset(new web::WebStateObserverBridge(webState, self));
On 2017/03/04 00:35:38, michaeldo wrote:
> Use base::MakeUnique here instead of new?
> 
> _webStateObserver = base::MakeUnique<web::WebStateObserverBridge>(webState,
> self);

Done.

https://codereview.chromium.org/2731443003/diff/40001/ios/chrome/browser/nati...
ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm:221:
_webState = nullptr;
On 2017/03/04 00:35:38, michaeldo wrote:
> Shouldn't we also cleanup/reset |_webStateObserver| here?
> 
> However, in order to retain similar behavior, do we want to change the value
of
> |_webState| at all?
Added _webStateObserver.reset(). I don't really want to keep the old behavior
where |_webState| is a dangling pointer :)

[commit-bot: I haz the power, 2017-03-04 01:02:00]

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-03-04 01:10:34]

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

[commit-bot: I haz the power, 2017-03-04 01:10:36]

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

[michaeldo, 2017-03-04 01:26:40]

lgtm

https://codereview.chromium.org/2731443003/diff/40001/ios/chrome/browser/nati...
File ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm
(right):

https://codereview.chromium.org/2731443003/diff/40001/ios/chrome/browser/nati...
ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm:221:
_webState = nullptr;
On 2017/03/04 01:01:50, Eugene But wrote:
> On 2017/03/04 00:35:38, michaeldo wrote:
> > Shouldn't we also cleanup/reset |_webStateObserver| here?
> > 
> > However, in order to retain similar behavior, do we want to change the value
> of
> > |_webState| at all?
> Added _webStateObserver.reset(). I don't really want to keep the old behavior
> where |_webState| is a dangling pointer :)

good call :)

[Eugene But (OOO till 7-30), 2017-03-04 01:38:54]

The CQ bit was checked by eugenebut@chromium.org

[Eugene But (OOO till 7-30), 2017-03-04 01:38:54]

The patchset sent to the CQ was uploaded after l-g-t-m from pkl@chromium.org
Link to the patchset: https://codereview.chromium.org/2731443003/#ps80001
(title: "Added missing import")

[commit-bot: I haz the power, 2017-03-04 01:39:05]

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-04 02:34:04]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1488591534294630,
"parent_rev": "eacf0f430d80615f07da8bc4094bb981771a155c", "commit_rev":
"4736030020471fa4dca4eb223925cfdac92267df"}

[commit-bot: I haz the power, 2017-03-04 02:34:40]

Description was changed from

==========
Removed CRWWebController dependency from NativeAppNavigationController.

Conform NativeAppNavigationController to CRWWebStateObserver instead of
CRWWebControllerObserver.

BUG=675005,619984
==========

to

==========
Removed CRWWebController dependency from NativeAppNavigationController.

Conform NativeAppNavigationController to CRWWebStateObserver instead of
CRWWebControllerObserver.

BUG=675005,619984

Review-Url: https://codereview.chromium.org/2731443003
Cr-Commit-Position: refs/heads/master@{#454751}
Committed:
https://chromium.googlesource.com/chromium/src/+/4736030020471fa4dca4eb223925...
==========

[commit-bot: I haz the power, 2017-03-04 02:34:41]

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