Discussions in https://codereview.chromium.org/2398733004/#msg14 suggests this is wrong.

There is RenderFrameObserver::FrameDetached(), but DidFinishLoad fires before FrameDetached(), so observers can't rely on it to detect atm.

As mentioned on the code review, I'm not entirely sure this is wrong:
- didFinishDocumentLoad is roughly equivalent to the DOMContentLoaded event
- didFinishLoad is roughly equivalent to the load event.

In the normal scenario, if an iframe stopped loading and that caused the page to finish loading, that should cause the load event to fire, I think?

That being said, we have a lot of legacy debt here. We have far too many loading callbacks (issue 555773, issue 600000), and if we can rationalize/document the callbacks, that would greatly improve the state of affairs.

As vabr@ commented in the CL, the comment says:
// The frame's document and all of its subresources succeeded to load.
https://cs.chromium.org/chromium/src/third_party/WebKit/public/web/WebFrameClient.h?q=didFinishLoad+file:%5C.h$&sq=package:chromium&l=399&dr=CSs

and cancel/close doesn't sound like "succeeded to load".

If the intention of didFinishLoad includes partial loads, cancel/close, maybe the work is just to clarify that and ensure all observers are happy with it.

Yes, the comments are sometimes quite stale and don't match what their actual purpose is =(
For a particularly egregious example, see https://chromium.googlesource.com/chromium/src/+/9f8e2f611e44753f7d8de5a2a6716bd030bc7283%5E%21/#F30

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

commit a398abc5085c5dc2d8db3076543072d1f196d668
Author: kojii <kojii@chromium.org>
Date: Wed Oct 12 08:37:58 2016

Do not SendPasswordForms when LocalFrame is being detached

This patch stops PasswordAutofillAgent::SendPasswordForms() to
enumerates forms in the document when LocalFrame is being detached.

This patch does not address the root cause of issue 561873 nor
issue 595078, but it is expected to reduce the crashes by large, and
it is reasonable not to fill passwords when Page is being destroyed.

BUG=561873, 595078, 654654

Review-Url: https://codereview.chromium.org/2398733004
Cr-Commit-Position: refs/heads/master@{#424692}

[modify] https://crrev.com/a398abc5085c5dc2d8db3076543072d1f196d668/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/a398abc5085c5dc2d8db3076543072d1f196d668/third_party/WebKit/Source/core/frame/LocalFrame.cpp
[modify] https://crrev.com/a398abc5085c5dc2d8db3076543072d1f196d668/third_party/WebKit/Source/core/frame/LocalFrame.h
[modify] https://crrev.com/a398abc5085c5dc2d8db3076543072d1f196d668/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/a398abc5085c5dc2d8db3076543072d1f196d668/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/a398abc5085c5dc2d8db3076543072d1f196d668/third_party/WebKit/public/web/WebLocalFrame.h

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

commit 79173dcd6d83bfacb4c5f389067822cfb69223f4
Author: Koji Ishii <kojii@chromium.org>
Date: Fri Oct 28 06:37:46 2016

Merge 2883: Do not SendPasswordForms when LocalFrame is being detached

Manually merged after resolving conflict with crbug.com/646610

This patch stops PasswordAutofillAgent::SendPasswordForms() to
enumerates forms in the document when LocalFrame is being detached.

This patch does not address the root cause of issue 561873 nor
issue 595078, but it is expected to reduce the crashes by large, and
it is reasonable not to fill passwords when Page is being destroyed.

TBR=dcheng@chromium.org, vabr@chromium.org
BUG=561873, 595078, 654654

Review-Url: https://codereview.chromium.org/2398733004
Cr-Commit-Position: refs/heads/master@{#424692}
(cherry picked from commit a398abc5085c5dc2d8db3076543072d1f196d668)

Review URL: https://codereview.chromium.org/2455053005 .

Cr-Commit-Position: refs/branch-heads/2883@{#356}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/79173dcd6d83bfacb4c5f389067822cfb69223f4/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/79173dcd6d83bfacb4c5f389067822cfb69223f4/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/79173dcd6d83bfacb4c5f389067822cfb69223f4/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/79173dcd6d83bfacb4c5f389067822cfb69223f4/third_party/WebKit/public/web/WebLocalFrame.h

Looks like this is just an issue of improving documentation on callbacks? Should we merge into either of the other issues dcheng mentioned in #3?

Actually I have a CL in progress... but the test passes with and without my CL =P

The best kind of test! Let me mark you as owner of this issue then :)

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

commit ff60cfffbf14d2419494ffc5fcf544f84dd05aa9
Author: dcheng <dcheng@chromium.org>
Date: Wed Nov 23 13:10:37 2016

Don't send loading completion callbacks for detaching frames.

BUG=561873,595078,654654

Review-Url: https://codereview.chromium.org/2428803002
Cr-Commit-Position: refs/heads/master@{#434155}

[modify] https://crrev.com/ff60cfffbf14d2419494ffc5fcf544f84dd05aa9/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/ff60cfffbf14d2419494ffc5fcf544f84dd05aa9/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/ff60cfffbf14d2419494ffc5fcf544f84dd05aa9/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/ff60cfffbf14d2419494ffc5fcf544f84dd05aa9/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/ff60cfffbf14d2419494ffc5fcf544f84dd05aa9/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
[add] https://crrev.com/ff60cfffbf14d2419494ffc5fcf544f84dd05aa9/third_party/WebKit/Source/web/tests/data/frame_with_frame.html
[add] https://crrev.com/ff60cfffbf14d2419494ffc5fcf544f84dd05aa9/third_party/WebKit/Source/web/tests/data/parent_detaching_frame.html
[modify] https://crrev.com/ff60cfffbf14d2419494ffc5fcf544f84dd05aa9/third_party/WebKit/public/web/WebLocalFrame.h