This is because the NTP is no longer considered as NativeView, so -discardNonCommittedItemsIfLastCommittedWasNotNativeView in CRWWebController is actually discarding the non committed item, but the NTP isn't able to reload itself because there are not WebState's observer callback to warn them that the non-committed items have been removed (so the last call is DidFinishNavigation() on the non-committed item).

Any idea?

It seems like we either need to trigger NTPTabHelper based on the isLoading WebStateObserver callback, or we need to trigger some other WebStateObserver callback when we discard the noncomitteditem.

I always disliked discardNonCommittedItemsIfLastCommittedWasNotNativeView because it was a workaround for UI in the layer which has nothing to do with UI.

Can we load NPT if DidFinishNavigation was called with empty navigation manager?

DidFinishNavigation is not called after discarding the last non committed item.
We don't have that many callbacks (it seems that we have "stopLoading", but I am not sure this is intentional).

Sorry I misunderstood the problem. I think DidFinishNavigation is called for "download" navigations, which is verified by WebStateObserverTest.DownloadNavigation test. Inside DidFinishNavigation, can we check that web_state->GetLastCommittedURL() is NTP? And if last committed item is NTP, then can we show NTP view?

This will not yield excacly the same UI behavior as we had with NativeContent NTP, but I'm not sure if there will be a better fix for the branch.

I have a demo patch: https://chromium-review.googlesource.com/c/chromium/src/+/1396036/.
It solves the issue, but it feels a bit hacky. Are we sure that we can't have DidFinishNavigation with NTP as last committed URL otherwise?

I don't think that your fix is worse than discardNonCommittedItemsIfLastCommittedWasNotNativeView

|IsNTPURL(web_state->GetLastCommittedURL())| is a superset of |IsNTPURL(navigation_context->GetUrl())|, so I think the change is fine.

The only downside I see would be the ugly and possibly quick animation when NTP is shown.

Justin: I am leaving for the week, can you take care of landing/cherry-picking the fix as we are getting late in the branch?
Current proposal: https://chromium-review.googlesource.com/c/chromium/src/+/1396036/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ebe14882b66eaa8c1a6fe0476684489cae490b53

commit ebe14882b66eaa8c1a6fe0476684489cae490b53
Author: Gauthier Ambard <gambard@chromium.org>
Date: Mon Jan 07 17:04:05 2019

[iOS] Check the last committed URL for showing the NTP

This CL checks the URL of the navigation context and the URL of the last
committed URL of the WebState to decide if the NTP should be shown or
not.
It allows us to fix an issue where the navigation was ended while having
a pending item. The URL of the navigation context is then set to the URL
of the pending it, preventing the NTP from being shown.

Bug:  917358 
Change-Id: Ie01c5dea2003e0912dd9036db35d72d63aab19ad
Reviewed-on: https://chromium-review.googlesource.com/c/1396036
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620361}
[modify] https://crrev.com/ebe14882b66eaa8c1a6fe0476684489cae490b53/ios/chrome/browser/ntp/new_tab_page_tab_helper.mm
[modify] https://crrev.com/ebe14882b66eaa8c1a6fe0476684489cae490b53/ios/chrome/browser/ntp/new_tab_page_tab_helper_unittest.mm

[Auto-generated comment by a script] We noticed that this issue is targeted for M-72; it appears the fix may have landed after branch point, meaning a merge might be required. The owner of this bug should confirm if a merge is required here. If so, add Merge-Request-72 label and indicate which commits/CLs are to be merged. Otherwise, remove Merge-TBD label. Thanks.

rakurati@ can you please verify on tomorrow's build

This bug requires manual review: Less than 18 days to go before AppStore submit on M72
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

rakurati: see c13.

Tested in 
Build Version: 73.0.3667.0 Canary
Device Details: iPhone XS (iOS 12.1.2), iPhone 8plus(iOS 11.4.1) and iPad 2018(iOS 12.1.1)

In NTP, on loading download link URL from clipboard the URL is not displayed anymore in omnibox instead NTP refreshs and a download manager info bar is displayed this behavior looks different from the M71 stable behavior. Please refer to the attached video. 

Link to video:
M73 behavior:
https://drive.google.com/file/d/1P5YMRrDd4et1fHZMDaWXHux2MVNWhWuy/view?usp=sharing
M71 Stable behavior:
https://drive.google.com/file/d/16UR1hLzJreJ_9GX3XnpgtxZxSFfDwn5J/view?usp=sharing

Note: Reopening the issue as the Canary behavior is different from stable.

I like the functionality of M73 better than M71 -- it is more useful, and is the same as desktop.

1) M71 just leaves you with a blank page, which isn't very useful.
2) Desktop functions similarly to M73 (open new tab, paste in link from OP, still see NTP).

Flipping back to fixed.   Do you agree this should be verified?

The behaviour from M73 is looking better than M71. Verified on M73.0.3667.0 canary.
Verified on iOS11, 12 with iPad mini and iPhoneX.

Approved, please merge asap.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/895d6a3c6de00ade6d23cc1e23975454fa7b0d18

commit 895d6a3c6de00ade6d23cc1e23975454fa7b0d18
Author: Gauthier Ambard <gambard@chromium.org>
Date: Fri Jan 11 15:58:01 2019

[iOS] Check the last committed URL for showing the NTP

This CL checks the URL of the navigation context and the URL of the last
committed URL of the WebState to decide if the NTP should be shown or
not.
It allows us to fix an issue where the navigation was ended while having
a pending item. The URL of the navigation context is then set to the URL
of the pending it, preventing the NTP from being shown.

Bug:  917358 
Change-Id: Ie01c5dea2003e0912dd9036db35d72d63aab19ad
Reviewed-on: https://chromium-review.googlesource.com/c/1396036
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#620361}(cherry picked from commit ebe14882b66eaa8c1a6fe0476684489cae490b53)
Reviewed-on: https://chromium-review.googlesource.com/c/1407032
Cr-Commit-Position: refs/branch-heads/3626@{#639}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/895d6a3c6de00ade6d23cc1e23975454fa7b0d18/ios/chrome/browser/ntp/new_tab_page_tab_helper.mm
[modify] https://crrev.com/895d6a3c6de00ade6d23cc1e23975454fa7b0d18/ios/chrome/browser/ntp/new_tab_page_tab_helper_unittest.mm

The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/895d6a3c6de00ade6d23cc1e23975454fa7b0d18

Commit: 895d6a3c6de00ade6d23cc1e23975454fa7b0d18
Author: gambard@chromium.org
Commiter: justincohen@chromium.org
Date: 2019-01-11 15:58:01 +0000 UTC

[iOS] Check the last committed URL for showing the NTP

This CL checks the URL of the navigation context and the URL of the last
committed URL of the WebState to decide if the NTP should be shown or
not.
It allows us to fix an issue where the navigation was ended while having
a pending item. The URL of the navigation context is then set to the URL
of the pending it, preventing the NTP from being shown.

Bug:  917358 
Change-Id: Ie01c5dea2003e0912dd9036db35d72d63aab19ad
Reviewed-on: https://chromium-review.googlesource.com/c/1396036
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#620361}(cherry picked from commit ebe14882b66eaa8c1a6fe0476684489cae490b53)
Reviewed-on: https://chromium-review.googlesource.com/c/1407032
Cr-Commit-Position: refs/branch-heads/3626@{#639}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Tested in 
Build Version: 72.0.3626.59 Beta
Device Details: iPhone XR (iOS 12.1.2), iPhone 8plus(iOS 11.4.1) and iPad Pro(iOS 12.1.1)

In NTP, on loading download link URL from clipboard the URL is not displayed anymore in omnibox instead NTP refreshs and a download manager info bar is displayed, looks good.