clamy@, creis@, kkhorimoto@, danyao@, what do you think about the proposal to unify all DidFinishLoading callbacks?

I agree that these should be unified.  The current state of the code in iOS after crrev.com/c/754513 requires users of the WebStateObserver API to carefully consider all commenting before deciding which callback to use; moving this all into a single callback would make it easier on developers, and will also potentially eliminate some code duplication, as much of our handling of successful vs. unsuccessful loads is the same.

DidStopLoading should not be merged with the other callbacks. DidStopLoading relates to the general loading state of the tab, and is not correlated with the document load phase the other callbacks relate to. In particular, if your navigation is aborted you will get DidStopLoading, but you won't get DidFailLoad because you were not in the document load phase of the page load. The fact that DidStopLoading does not provide a RenderFrameHost in content// also highlights the fact that this is a per-WebContents callback, as opposed to DidFinishLoad and DidFailLoad which are per-frame methods.

When it comes to DidFinishLoad and DidFailLoad I'd be ok with merging the two of them in a single method (if it's not too complicated with observers), similar with what we did with DidFinishNavigation. I'd suggest naming it DidFinishDocumentLoad, so that it is clear what happened.

This way you get three callbacks:
- DidFinishNavigation
- DidFinishDocumentLoad
- DidStopLoading (and this one can happen at any point in time during the page load).

Thanks Camille! Now I understand the difference between DidStopLoading/ DidFinishLoad/DidFailLoad. On iOS there is no way to provide navigation callbacks for iframes, that's why existence of DidStopLoading/PageLoaded pair is confusing. 

I think it will be better for iOS to keep DidStopLoading as a separate callback to make the development of navigation service easier.

I will update WebStateObserver comments to clarify the conceptual difference between DidStopLoading and PageLoaded. 

Final question for Camille or Charlie. Does DidStartLoading/DidStopLoading pair get called for iframes loads as well? Or this is main frame only callback?

They are called whenever a frame on the page is loading. For example, they would be called if you navigate an iframe.

Thanks! Danyao, do you know if WKWebView.loading KVO callback is called for iframes? If yes, then we could switch DidStartLoading/DidFinishLoading to use WKWebView.loading to fully match //content.

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

commit f5d9f686a334cde4c11e652f15a17fdc2c59d52a
Author: Eugene But <eugenebut@google.com>
Date: Tue Nov 07 18:32:02 2017

Clarified DidStopLoading/PageLoaded difference in the comments.

Bug:  781871 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I115ca9b555055b37ea00361c26fab0e3376a4c0c
Reviewed-on: https://chromium-review.googlesource.com/757277
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514516}
[modify] https://crrev.com/f5d9f686a334cde4c11e652f15a17fdc2c59d52a/ios/web/public/web_state/web_state_observer.h

No, WKWebView.loading KVO is only called for main frame.

:( Thank you for confirming that.