Do not fire didStartLoading and didStopLoading events twice.

Do not fire didStartLoading and didStopLoading events twice.

This issue was found by extra DCHECK on the Chromium side, ensuring these events
are never fired more than once.

BUG=464675, 429399
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=191464

[Fabrice (no longer in Chrome), 2015-03-06 14:44:00]

fdegans@chromium.org changed reviewers:
+ clamy@chromium.org, japhet@chromium.org

[Fabrice (no longer in Chrome), 2015-03-06 14:44:01]

PTAL
The issue is that the current test makes
FrameLoader::updateForSameDocumentNavigation call did(Start|Stop)Loading a
second time and that triggered a DCHECK we added to ensure that does not happen.

[clamy, 2015-03-06 15:11:03]

Thanks! Lgtm

[Nate Chapin, 2015-03-06 18:22:20]

https://codereview.chromium.org/984983002/diff/1/Source/core/dom/Document.h
File Source/core/dom/Document.h (right):

https://codereview.chromium.org/984983002/diff/1/Source/core/dom/Document.h#n...
Source/core/dom/Document.h:914: bool shouldFireLoadingEvents() const { return
m_loadEventProgress == LoadEventCompleted; }
This name seems overly specific to its usage. isNotLoadingOrUnloading?
completeButNotUnloading? inStableLoadingState?

[Fabrice (no longer in Chrome), 2015-03-06 18:39:14]

https://codereview.chromium.org/984983002/diff/1/Source/core/dom/Document.h
File Source/core/dom/Document.h (right):

https://codereview.chromium.org/984983002/diff/1/Source/core/dom/Document.h#n...
Source/core/dom/Document.h:914: bool shouldFireLoadingEvents() const { return
m_loadEventProgress == LoadEventCompleted; }
On 2015/03/06 18:22:20, Nate Chapin wrote:
> This name seems overly specific to its usage. isNotLoadingOrUnloading?
> completeButNotUnloading? inStableLoadingState?

I kinda like it because it's only really supposed to be used for that.
Anyway, I changed it to isLoadEventCompleted.

[Nate Chapin, 2015-03-06 18:40:00]

On 2015/03/06 18:39:14, Fabrice wrote:
> https://codereview.chromium.org/984983002/diff/1/Source/core/dom/Document.h
> File Source/core/dom/Document.h (right):
> 
>
https://codereview.chromium.org/984983002/diff/1/Source/core/dom/Document.h#n...
> Source/core/dom/Document.h:914: bool shouldFireLoadingEvents() const { return
> m_loadEventProgress == LoadEventCompleted; }
> On 2015/03/06 18:22:20, Nate Chapin wrote:
> > This name seems overly specific to its usage. isNotLoadingOrUnloading?
> > completeButNotUnloading? inStableLoadingState?
> 
> I kinda like it because it's only really supposed to be used for that.
> Anyway, I changed it to isLoadEventCompleted.

That's a fair point, but I get worried that people will reuse it without
changing the name :) LGTM

[Fabrice (no longer in Chrome), 2015-03-06 18:43:31]

The CQ bit was checked by fdegans@chromium.org

[Fabrice (no longer in Chrome), 2015-03-06 18:43:32]

The patchset sent to the CQ was uploaded after l-g-t-m from clamy@chromium.org
Link to the patchset: https://codereview.chromium.org/984983002/#ps20001 (title:
"Change new method name")

[commit-bot: I haz the power, 2015-03-06 18:43:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/984983002/20001

[Fabrice (no longer in Chrome), 2015-03-06 22:03:07]

The CQ bit was unchecked by fdegans@chromium.org

[Fabrice (no longer in Chrome), 2015-03-06 22:03:30]

The CQ bit was checked by fdegans@chromium.org

[commit-bot: I haz the power, 2015-03-06 22:03:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/984983002/20001

[Fabrice (no longer in Chrome), 2015-03-06 22:34:35]

The CQ bit was unchecked by fdegans@chromium.org

[Fabrice (no longer in Chrome), 2015-03-06 22:46:00]

The CQ bit was checked by fdegans@chromium.org

[commit-bot: I haz the power, 2015-03-06 22:46:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/984983002/20001

[commit-bot: I haz the power, 2015-03-07 00:29:59]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=191464

[f(malita), 2015-03-07 21:52:58]

On 2015/03/07 00:29:59, I haz the power (commit-bot) wrote:
> Committed patchset #2 (id:20001) as
> https://src.chromium.org/viewvc/blink?view=rev&revision=191464

This is causing WebContentsImplBrowserTest.ClearNonVisiblePendingOnFail to time
out:

http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...
https://codereview.chromium.org/987953002/

Not sure whether it's a problem with the CL itself or with the test, but since
it's been blocking Blink rolls for a while I'll go ahead and revert.

[f(malita), 2015-03-07 21:55:34]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/989923002/ by fmalita@chromium.org.

The reason for reverting is:
WebContentsImplBrowserTest.ClearNonVisiblePendingOnFail timeouts, blocking
rolls:

http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_....