Removed CRWSessionEntry from CRWWebController.

Removed CRWSessionEntry from CRWWebController.

BUG=454984, 684098
Review-Url: https://codereview.chromium.org/2705673003
Cr-Commit-Position: refs/heads/master@{#455615}
Committed: https://chromium.googlesource.com/chromium/src/+/725f29f1dd104cbc643cd14af8eebf768520b27c

[kkhorimoto, 2017-02-18 00:52:01]

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

[commit-bot: I haz the power, 2017-02-18 00:54:01]

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-02-18 02:56:01]

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

[commit-bot: I haz the power, 2017-02-18 02:56:02]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[kkhorimoto, 2017-02-21 23:14:46]

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

[Eugene But (OOO till 7-30), 2017-02-21 23:29:24]

lgtm

https://codereview.chromium.org/2705673003/diff/20001/ios/web/web_state/ui/cr...
File ios/web/web_state/ui/crw_web_controller.mm (right):

https://codereview.chromium.org/2705673003/diff/20001/ios/web/web_state/ui/cr...
ios/web/web_state/ui/crw_web_controller.mm:2851:
static_cast<web::NavigationItemImpl*>(self.currentNavItem);
Do you need this cast? |currentNavItem| returns web::NavigationItemImpl*

[kkhorimoto, 2017-03-02 22:01:11]

https://codereview.chromium.org/2705673003/diff/20001/ios/web/web_state/ui/cr...
File ios/web/web_state/ui/crw_web_controller.mm (right):

https://codereview.chromium.org/2705673003/diff/20001/ios/web/web_state/ui/cr...
ios/web/web_state/ui/crw_web_controller.mm:2851:
static_cast<web::NavigationItemImpl*>(self.currentNavItem);
On 2017/02/21 23:29:24, Eugene But wrote:
> Do you need this cast? |currentNavItem| returns web::NavigationItemImpl*

Removed

[kkhorimoto, 2017-03-02 22:10:39]

Description was changed from

==========
Removed CRWSessionEntry from CRWWebController.

BUG=454984
==========

to

==========
Removed CRWSessionEntry from CRWWebController.

BUG=454984, 684098
==========

[kkhorimoto, 2017-03-02 22:13:41]

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

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

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:31:38]

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

[commit-bot: I haz the power, 2017-03-02 22:31:39]

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

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

https://codereview.chromium.org/2705673003/diff/80001/ios/web/public/web_stat...
File ios/web/public/web_state/ui/crw_web_delegate.h (left):

https://codereview.chromium.org/2705673003/diff/80001/ios/web/public/web_stat...
ios/web/public/web_state/ui/crw_web_delegate.h:79: -
(void)webWillFinishHistoryNavigationFromEntry:(CRWSessionEntry*)fromEntry;
I think we still need this code for old back forward navigation.

[kkhorimoto, 2017-03-03 00:27:55]

https://codereview.chromium.org/2705673003/diff/80001/ios/web/public/web_stat...
File ios/web/public/web_state/ui/crw_web_delegate.h (left):

https://codereview.chromium.org/2705673003/diff/80001/ios/web/public/web_stat...
ios/web/public/web_state/ui/crw_web_delegate.h:79: -
(void)webWillFinishHistoryNavigationFromEntry:(CRWSessionEntry*)fromEntry;
On 2017/03/02 22:49:41, Eugene But wrote:
> I think we still need this code for old back forward navigation.

Tab's implementation of this function only calls |notifyTabChanged:| on the
TabModel, but this will already be called due to the OnNavigationItemCommitted()
callback.

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

https://codereview.chromium.org/2705673003/diff/80001/ios/web/public/web_stat...
File ios/web/public/web_state/ui/crw_web_delegate.h (left):

https://codereview.chromium.org/2705673003/diff/80001/ios/web/public/web_stat...
ios/web/public/web_state/ui/crw_web_delegate.h:79: -
(void)webWillFinishHistoryNavigationFromEntry:(CRWSessionEntry*)fromEntry;
On 2017/03/03 00:27:55, kkhorimoto_ wrote:
> On 2017/03/02 22:49:41, Eugene But wrote:
> > I think we still need this code for old back forward navigation.
> 
> Tab's implementation of this function only calls |notifyTabChanged:| on the
> TabModel, but this will already be called due to the
OnNavigationItemCommitted()
> callback.
|notifyTabChanged:| is definitely called for same-document navigation (probably
multiple times). But is it called for different-document navigations? Is there a
reason why you want to remove this callback now? It could be easily removed
after cleaning up "PendingIndexNavigationDisabled" experimental setting.

[kkhorimoto, 2017-03-08 21:31:33]

https://codereview.chromium.org/2705673003/diff/80001/ios/web/public/web_stat...
File ios/web/public/web_state/ui/crw_web_delegate.h (left):

https://codereview.chromium.org/2705673003/diff/80001/ios/web/public/web_stat...
ios/web/public/web_state/ui/crw_web_delegate.h:79: -
(void)webWillFinishHistoryNavigationFromEntry:(CRWSessionEntry*)fromEntry;
On 2017/03/03 01:53:36, Eugene But wrote:
> On 2017/03/03 00:27:55, kkhorimoto_ wrote:
> > On 2017/03/02 22:49:41, Eugene But wrote:
> > > I think we still need this code for old back forward navigation.
> > 
> > Tab's implementation of this function only calls |notifyTabChanged:| on the
> > TabModel, but this will already be called due to the
> OnNavigationItemCommitted()
> > callback.
> |notifyTabChanged:| is definitely called for same-document navigation
(probably
> multiple times). But is it called for different-document navigations? Is there
a
> reason why you want to remove this callback now? It could be easily removed
> after cleaning up "PendingIndexNavigationDisabled" experimental setting.

My main motivation for removing this was that A) it didn't serve any purpose
since |notifyTabChanged:| is already called, and B) it exposes a deprecated
private class in a public delegate protocol.  I've verified that
|notifyTabChanged:| is called numerous times for same- and different-document
navigations both when the experiment is disabled and enabled.  I considered just
leaving this as |-webWillFinishHistoryNavigation|, since |fromEntry| isn't
actually used in the implementation, but since it seems that this is not adding
anything, I just removed it for simplicity.

[kkhorimoto, 2017-03-08 22:18:11]

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

[kkhorimoto, 2017-03-08 22:18:38]

https://codereview.chromium.org/2705673003/diff/80001/ios/web/public/web_stat...
File ios/web/public/web_state/ui/crw_web_delegate.h (left):

https://codereview.chromium.org/2705673003/diff/80001/ios/web/public/web_stat...
ios/web/public/web_state/ui/crw_web_delegate.h:79: -
(void)webWillFinishHistoryNavigationFromEntry:(CRWSessionEntry*)fromEntry;
On 2017/03/08 21:31:33, kkhorimoto_ wrote:
> On 2017/03/03 01:53:36, Eugene But wrote:
> > On 2017/03/03 00:27:55, kkhorimoto_ wrote:
> > > On 2017/03/02 22:49:41, Eugene But wrote:
> > > > I think we still need this code for old back forward navigation.
> > > 
> > > Tab's implementation of this function only calls |notifyTabChanged:| on
the
> > > TabModel, but this will already be called due to the
> > OnNavigationItemCommitted()
> > > callback.
> > |notifyTabChanged:| is definitely called for same-document navigation
> (probably
> > multiple times). But is it called for different-document navigations? Is
there
> a
> > reason why you want to remove this callback now? It could be easily removed
> > after cleaning up "PendingIndexNavigationDisabled" experimental setting.
> 
> My main motivation for removing this was that A) it didn't serve any purpose
> since |notifyTabChanged:| is already called, and B) it exposes a deprecated
> private class in a public delegate protocol.  I've verified that
> |notifyTabChanged:| is called numerous times for same- and different-document
> navigations both when the experiment is disabled and enabled.  I considered
just
> leaving this as |-webWillFinishHistoryNavigation|, since |fromEntry| isn't
> actually used in the implementation, but since it seems that this is not
adding
> anything, I just removed it for simplicity.

I've added this back in so that chrome is notified at the proper time for legacy
history navigations.

[commit-bot: I haz the power, 2017-03-08 22:18:47]

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

[Eugene But (OOO till 7-30), 2017-03-08 22:26:51]

lgtm

[commit-bot: I haz the power, 2017-03-08 23:20:03]

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

[commit-bot: I haz the power, 2017-03-08 23:20:04]

Dry run: This issue passed the CQ dry run.

[kkhorimoto, 2017-03-09 00:30:50]

The CQ bit was checked by kkhorimoto@chromium.org

[commit-bot: I haz the power, 2017-03-09 00:31:13]

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-09 00:39:01]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1489019450367620,
"parent_rev": "021fb549d0d767d33b795d8d7e6321bc515efcce", "commit_rev":
"725f29f1dd104cbc643cd14af8eebf768520b27c"}

[commit-bot: I haz the power, 2017-03-09 00:40:02]

Description was changed from

==========
Removed CRWSessionEntry from CRWWebController.

BUG=454984, 684098
==========

to

==========
Removed CRWSessionEntry from CRWWebController.

BUG=454984, 684098

Review-Url: https://codereview.chromium.org/2705673003
Cr-Commit-Position: refs/heads/master@{#455615}
Committed:
https://chromium.googlesource.com/chromium/src/+/725f29f1dd104cbc643cd14af8ee...
==========

[commit-bot: I haz the power, 2017-03-09 00:40:03]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/725f29f1dd104cbc643cd14af8ee...