Set fromEntry to nil after discardNonCommittedItems.

Set fromEntry to nil after discardNonCommittedItems.

With the indirection CRWSessionEntry->NavigationItem,
discardNonCommittedItems can invalidate fromEntry.
Set if to nil if it is invalid.

This is a tentative fix by the sheriff to make the tree green.
Please review this CL and revert it later if there is a better solution.

BUG=691492
TBR=eugenebut@chromium.org, kkhorimoto@chromium.org,

Review-Url: https://codereview.chromium.org/2690033002
Cr-Commit-Position: refs/heads/master@{#449941}
Committed: https://chromium.googlesource.com/chromium/src/+/7f58024526f7dfa06703d876beac569242299357

[Olivier, 2017-02-13 10:24:18]

olivierrobin@chromium.org changed reviewers:
+ eugenebut@chromium.org, kkhorimoto@chromium.org, sdefresne@chromium.org

[Olivier, 2017-02-13 10:24:24]

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

[Olivier, 2017-02-13 10:24:32]

Description was changed from

==========
Refresh fromEntry after discardNonCommittedItems.

With the indirection CRWSessionEntry->NavigationItem,
discardNonCommittedItems can invalidate fromEntry.
Refetch if from the sessionController.

This is a tentative fix by the sheriff to make the tree green.
Please review this CL and revert it later if there is a better solution.

BUG=691492
==========

to

==========
Refresh fromEntry after discardNonCommittedItems.

With the indirection CRWSessionEntry->NavigationItem,
discardNonCommittedItems can invalidate fromEntry.
Refetch if from the sessionController.

This is a tentative fix by the sheriff to make the tree green.
Please review this CL and revert it later if there is a better solution.

BUG=691492
TBR=eugenebut@chromium.org, kkhorimoto@chromium.org,
==========

[commit-bot: I haz the power, 2017-02-13 10:24:37]

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-13 10:38:40]

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

[commit-bot: I haz the power, 2017-02-13 10:38:41]

Dry run: This issue passed the CQ dry run.

[sdefresne, 2017-02-13 10:47:36]

https://codereview.chromium.org/2690033002/diff/1/ios/web/web_state/ui/crw_we...
File ios/web/web_state/ui/crw_web_controller.mm (right):

https://codereview.chromium.org/2690033002/diff/1/ios/web/web_state/ui/crw_we...
ios/web/web_state/ui/crw_web_controller.mm:2167: [sessionController
discardNonCommittedItems];
I think we should instead retain the fromEntry, by changing the type to
base::scoped_nsobject<CRWSessionEntry>.

[Olivier, 2017-02-13 12:21:05]

https://codereview.chromium.org/2690033002/diff/1/ios/web/web_state/ui/crw_we...
File ios/web/web_state/ui/crw_web_controller.mm (right):

https://codereview.chromium.org/2690033002/diff/1/ios/web/web_state/ui/crw_we...
ios/web/web_state/ui/crw_web_controller.mm:2167: [sessionController
discardNonCommittedItems];
On 2017/02/13 10:47:36, sdefresne wrote:
> I think we should instead retain the fromEntry, by changing the type to
> base::scoped_nsobject<CRWSessionEntry>.

Thanks

I cannot retain the fromEntry as CRWSessionEntry does not own the
navigationItem.
Even if the entry is retained, the inside pointer will be invalid.

As discussed offline, I also tried to set the entry to nil.
But the first instruction of |webWillFinishHistoryNavigationFromEntry:| is
DCHECK(fromEntry), which obviously lead to a crash.

[Olivier, 2017-02-13 13:20:43]

Description was changed from

==========
Refresh fromEntry after discardNonCommittedItems.

With the indirection CRWSessionEntry->NavigationItem,
discardNonCommittedItems can invalidate fromEntry.
Refetch if from the sessionController.

This is a tentative fix by the sheriff to make the tree green.
Please review this CL and revert it later if there is a better solution.

BUG=691492
TBR=eugenebut@chromium.org, kkhorimoto@chromium.org,
==========

to

==========
Set fromEntry to nil after discardNonCommittedItems.

With the indirection CRWSessionEntry->NavigationItem,
discardNonCommittedItems can invalidate fromEntry.
Set if to nil if it is invalid.

This is a tentative fix by the sheriff to make the tree green.
Please review this CL and revert it later if there is a better solution.

BUG=691492
TBR=eugenebut@chromium.org, kkhorimoto@chromium.org,
==========

[Olivier, 2017-02-13 13:21:24]

https://codereview.chromium.org/2690033002/diff/1/ios/web/web_state/ui/crw_we...
File ios/web/web_state/ui/crw_web_controller.mm (right):

https://codereview.chromium.org/2690033002/diff/1/ios/web/web_state/ui/crw_we...
ios/web/web_state/ui/crw_web_controller.mm:2167: [sessionController
discardNonCommittedItems];
On 2017/02/13 12:21:05, Olivier Robin wrote:
> On 2017/02/13 10:47:36, sdefresne wrote:
> > I think we should instead retain the fromEntry, by changing the type to
> > base::scoped_nsobject<CRWSessionEntry>.
> 
> Thanks
> 
> I cannot retain the fromEntry as CRWSessionEntry does not own the
> navigationItem.
> Even if the entry is retained, the inside pointer will be invalid.
> 
> As discussed offline, I also tried to set the entry to nil.
> But the first instruction of |webWillFinishHistoryNavigationFromEntry:| is
> DCHECK(fromEntry), which obviously lead to a crash.

Done. PTAL.

[Olivier, 2017-02-13 13:23:04]

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

[commit-bot: I haz the power, 2017-02-13 13:23:26]

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

[sdefresne, 2017-02-13 13:30:36]

lgtm to fix the tree, Eugene/Kurt, please take a look and land the proper fix.

[Olivier, 2017-02-13 13:32:44]

The CQ bit was unchecked by olivierrobin@chromium.org

[Olivier, 2017-02-13 13:32:47]

The CQ bit was checked by olivierrobin@chromium.org

[commit-bot: I haz the power, 2017-02-13 13:33:04]

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-13 13:39:26]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486992767966010,
"parent_rev": "7a8b621535abb3aa5d9c0ff2196299f1d90449a5", "commit_rev":
"7f58024526f7dfa06703d876beac569242299357"}

[commit-bot: I haz the power, 2017-02-13 13:39:59]

Description was changed from

==========
Set fromEntry to nil after discardNonCommittedItems.

With the indirection CRWSessionEntry->NavigationItem,
discardNonCommittedItems can invalidate fromEntry.
Set if to nil if it is invalid.

This is a tentative fix by the sheriff to make the tree green.
Please review this CL and revert it later if there is a better solution.

BUG=691492
TBR=eugenebut@chromium.org, kkhorimoto@chromium.org,
==========

to

==========
Set fromEntry to nil after discardNonCommittedItems.

With the indirection CRWSessionEntry->NavigationItem,
discardNonCommittedItems can invalidate fromEntry.
Set if to nil if it is invalid.

This is a tentative fix by the sheriff to make the tree green.
Please review this CL and revert it later if there is a better solution.

BUG=691492
TBR=eugenebut@chromium.org, kkhorimoto@chromium.org,

Review-Url: https://codereview.chromium.org/2690033002
Cr-Commit-Position: refs/heads/master@{#449941}
Committed:
https://chromium.googlesource.com/chromium/src/+/7f58024526f7dfa06703d876beac...
==========

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

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