Removed use of CRWSessionController from NativeAppNavigationUtilsTest.

Removed use of CRWSessionController from NativeAppNavigationUtilsTest.

Previous implementation uses WebStateImpl, NavigationManagerImpl, and
CRWSessionController which are all private. This new version uses a
TestNavigationManager that is all "faked".

BUG=619984
Review-Url: https://codereview.chromium.org/2742173002
Cr-Commit-Position: refs/heads/master@{#456873}
Committed: https://chromium.googlesource.com/chromium/src/+/3d9e8afb33950f06b047aab296e143e45ab576ce

[pkl (ping after 24h if needed), 2017-03-13 13:49:03]

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

[Eugene But (OOO till 7-30), 2017-03-13 21:05:41]

Thank you for cleanup! lgtm

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

https://codereview.chromium.org/2742173002/diff/20001/ios/chrome/browser/nati...
ios/chrome/browser/native_app_launcher/native_app_navigation_util_unittest.mm:9:
#import "ios/web/public/navigation_item_list.h"
nit: s/import/include

https://codereview.chromium.org/2742173002/diff/20001/ios/chrome/browser/nati...
ios/chrome/browser/native_app_launcher/native_app_navigation_util_unittest.mm:30:
void AddItem(const std::string& url_spec, ui::PageTransition transition) {
This could be useful for other tests. Could you please move this to
web::TestNavigationManager

[pkl (ping after 24h if needed), 2017-03-14 02:12:48]

Thanks! PTAL.

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

https://codereview.chromium.org/2742173002/diff/20001/ios/chrome/browser/nati...
ios/chrome/browser/native_app_launcher/native_app_navigation_util_unittest.mm:9:
#import "ios/web/public/navigation_item_list.h"
On 2017/03/13 21:05:40, Eugene But wrote:
> nit: s/import/include

Acknowledged.

https://codereview.chromium.org/2742173002/diff/20001/ios/chrome/browser/nati...
ios/chrome/browser/native_app_launcher/native_app_navigation_util_unittest.mm:30:
void AddItem(const std::string& url_spec, ui::PageTransition transition) {
On 2017/03/13 21:05:41, Eugene But wrote:
> This could be useful for other tests. Could you please move this to
> web::TestNavigationManager

Moved to test_navigation_manager.mm

[Eugene But (OOO till 7-30), 2017-03-14 16:52:11]

lgtm

https://codereview.chromium.org/2742173002/diff/40001/ios/web/public/test/fak...
File ios/web/public/test/fakes/test_navigation_manager.h (right):

https://codereview.chromium.org/2742173002/diff/40001/ios/web/public/test/fak...
ios/web/public/test/fakes/test_navigation_manager.h:60: void AddItem(const
std::string& url_spec, ui::PageTransition transition);
Is there a reason for using url_spec instead of GURL?

https://codereview.chromium.org/2742173002/diff/40001/ios/web/public/test/fak...
File ios/web/public/test/fakes/test_navigation_manager.mm (right):

https://codereview.chromium.org/2742173002/diff/40001/ios/web/public/test/fak...
ios/web/public/test/fakes/test_navigation_manager.mm:105:
SetCurrentItemIndex(GetItemCount() - 1);
This is quite unexpected side effect. SessionController does this instead:

if (_currentNavigationIndex > index)
  _currentNavigationIndex--;

[pkl (ping after 24h if needed), 2017-03-14 21:16:19]

https://codereview.chromium.org/2742173002/diff/40001/ios/web/public/test/fak...
File ios/web/public/test/fakes/test_navigation_manager.h (right):

https://codereview.chromium.org/2742173002/diff/40001/ios/web/public/test/fak...
ios/web/public/test/fakes/test_navigation_manager.h:60: void AddItem(const
std::string& url_spec, ui::PageTransition transition);
On 2017/03/14 16:52:11, Eugene But wrote:
> Is there a reason for using url_spec instead of GURL?

const GURL& is a better choice. Fixed.

https://codereview.chromium.org/2742173002/diff/40001/ios/web/public/test/fak...
File ios/web/public/test/fakes/test_navigation_manager.mm (right):

https://codereview.chromium.org/2742173002/diff/40001/ios/web/public/test/fak...
ios/web/public/test/fakes/test_navigation_manager.mm:105:
SetCurrentItemIndex(GetItemCount() - 1);
On 2017/03/14 16:52:11, Eugene But wrote:
> This is quite unexpected side effect. SessionController does this instead:
> 
> if (_currentNavigationIndex > index)
>   _currentNavigationIndex--;
> 

Thanks for pointing this out. To mimic what's currently in SessionController,
I've added a DCHECK for index cannot be the current index.

[pkl (ping after 24h if needed), 2017-03-14 22:36:58]

The CQ bit was checked by pkl@chromium.org

[pkl (ping after 24h if needed), 2017-03-14 22:36:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
eugenebut@chromium.org
Link to the patchset: https://codereview.chromium.org/2742173002/#ps80001
(title: "fixed parameter naming")

[commit-bot: I haz the power, 2017-03-14 22:37:49]

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 22:48:32]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1489531018404590,
"parent_rev": "2221ed2f0b0419b80f184e580ee37b25812ec139", "commit_rev":
"3d9e8afb33950f06b047aab296e143e45ab576ce"}

[commit-bot: I haz the power, 2017-03-14 22:49:21]

Description was changed from

==========
Removed use of CRWSessionController from NativeAppNavigationUtilsTest.

Previous implementation uses WebStateImpl, NavigationManagerImpl, and
CRWSessionController which are all private. This new version uses a
TestNavigationManager that is all "faked".

BUG=619984
==========

to

==========
Removed use of CRWSessionController from NativeAppNavigationUtilsTest.

Previous implementation uses WebStateImpl, NavigationManagerImpl, and
CRWSessionController which are all private. This new version uses a
TestNavigationManager that is all "faked".

BUG=619984

Review-Url: https://codereview.chromium.org/2742173002
Cr-Commit-Position: refs/heads/master@{#456873}
Committed:
https://chromium.googlesource.com/chromium/src/+/3d9e8afb33950f06b047aab296e1...
==========

[commit-bot: I haz the power, 2017-03-14 22:49:23]

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