There's more related discussion in  issue 817881 , which I think will be unblocked by this.

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

commit 5325a952741fffc94b07ef324266cdadea3fa999
Author: Karan Bhatia <karandeepb@chromium.org>
Date: Thu Mar 22 20:17:51 2018

Allow NavigationHandle::GetRenderFrameHost to always be called.

While the NavigationHandle in WebContentsObserver::ReadyToCommitNavigation is
always accessible, the same is not true for uncommitted navigations in
WebContentsObserver::DidFinishNavigation. This CL removes this inconsistency in
the WebContentsObserver interface by allowing
NavigationHandle::GetRenderFrameHost to always be called.

BUG= 824576 ,  811460 

Change-Id: I4f37a5fd4372eb505b065368312ebfc0c5cd730e
Reviewed-on: https://chromium-review.googlesource.com/972667
Reviewed-by: Camille Lamy <clamy@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545208}
[modify] https://crrev.com/5325a952741fffc94b07ef324266cdadea3fa999/content/browser/frame_host/navigation_handle_impl.cc
[modify] https://crrev.com/5325a952741fffc94b07ef324266cdadea3fa999/content/public/browser/navigation_handle.h
[modify] https://crrev.com/5325a952741fffc94b07ef324266cdadea3fa999/content/test/web_contents_observer_sanity_checker.cc

I'm actually strongly opposed to this! The RFH is only available at the time we know it and returning null will only cause our code to be littered with null checks. Based on the discussions off of the linked CLs, I can see us relaxing the time to when it can be called to include "ReadyToCommitNavigation", but not completely removing this enforcement. 

To document the discussion I had with nasko:

- RFH is already always non-null in ReadyToCommitNavigation.
- The motivation was for clients to reset state for the RFH in DidFinishNavigation for the case when the navigation didn't commit.
- As per nasko@: "I'm almost certain that if you get ReadyToCommitNavigation you will get a DidFinishNavigation that has "HasCommitted" returning true."

I will try to assert this in a WebContentsObserverSanityChecker test. Other navigation folks: feel free to chime in if you have context regarding whether this is true/false.

- In the meanwhile, I'll revert the CL till we can ensure that this is the correct thing to do from an API standpoint.

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

commit 5070af30d42677c1e2b06052d189bdf493bd4b4c
Author: Karan Bhatia <karandeepb@chromium.org>
Date: Thu Mar 22 21:27:07 2018

Revert "Allow NavigationHandle::GetRenderFrameHost to always be called."

This reverts commit 5325a952741fffc94b07ef324266cdadea3fa999.

Reason for revert: There isn't consensus currently whether this is the correct thing to do from an API standpoint. Reverting temporarily till we can ensure the same.

Original change's description:
> Allow NavigationHandle::GetRenderFrameHost to always be called.
> 
> While the NavigationHandle in WebContentsObserver::ReadyToCommitNavigation is
> always accessible, the same is not true for uncommitted navigations in
> WebContentsObserver::DidFinishNavigation. This CL removes this inconsistency in
> the WebContentsObserver interface by allowing
> NavigationHandle::GetRenderFrameHost to always be called.
> 
> BUG= 824576 ,  811460 
> 
> Change-Id: I4f37a5fd4372eb505b065368312ebfc0c5cd730e
> Reviewed-on: https://chromium-review.googlesource.com/972667
> Reviewed-by: Camille Lamy <clamy@chromium.org>
> Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#545208}

TBR=clamy@chromium.org,karandeepb@chromium.org

Change-Id: I5a06c842a0a87cbc1b8a87c18db1ec313b83ac2f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  824576 ,  811460 
Reviewed-on: https://chromium-review.googlesource.com/976604
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545255}
[modify] https://crrev.com/5070af30d42677c1e2b06052d189bdf493bd4b4c/content/browser/frame_host/navigation_handle_impl.cc
[modify] https://crrev.com/5070af30d42677c1e2b06052d189bdf493bd4b4c/content/public/browser/navigation_handle.h
[modify] https://crrev.com/5070af30d42677c1e2b06052d189bdf493bd4b4c/content/test/web_contents_observer_sanity_checker.cc

@nasko: "I'm almost certain that if you get ReadyToCommitNavigation you will get a DidFinishNavigation that has "HasCommitted" returning true." No I don't think this is the case, which is why I allowed the check to be relaxed in the first place. For HasCommitted to be true, you need to reach the state DID_COMMIT or DID_COMMIT_ERROR_PAGE. This only happens when you actually get the DidCommitProvisionalLoad message. If for whatever reason your navigation is destroyed before that, you will still be in READY_TO_COMMIT, which means that has committed will (rightly) return false. We know we currently have race conditions where this happens, so there will be cases where the cleanup cannot be done properly.

Marking this as a WontFix for now since it was decided that this is not be done. 

Navigation owners can decide whether the existing NavigationHandle/WebContentsObserver API needs to be improved for the case this bug talks about.