Simply FrameLoaderStateMachine and FrameLoader::transitionToCommitted().

Simply FrameLoaderStateMachine and FrameLoader::transitionToCommitted().

This removes 2 cases from FrameLoaderStateMachine. FirstLayoutDone is
closely approximated by FrameView, and DisplayingInitialEmptyDocumentPostCommit
appears to be a workaround for declaring the first real load committed a bit
too early.

BUG=none
TEST=none

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=149766

[Nate Chapin, 2013-05-01 21:38:34]

https://codereview.chromium.org/14787005/diff/3001/Source/core/loader/FrameLo...
File Source/core/loader/FrameLoader.cpp (left):

https://codereview.chromium.org/14787005/diff/3001/Source/core/loader/FrameLo...
Source/core/loader/FrameLoader.cpp:1667:
history()->updateForStandardLoad(HistoryController::UpdateAllExceptBackForwardList);
This case appears to be a Safari-specific hack:
http://trac.webkit.org/changeset/61230

https://codereview.chromium.org/14787005/diff/3001/Source/core/loader/FrameLo...
Source/core/loader/FrameLoader.cpp:1690:
m_frame->view()->setScrollbarsSuppressed(true);
This appears to only be needed for SVGImage, so it was moved there.

https://codereview.chromium.org/14787005/diff/3001/Source/core/loader/FrameLo...
Source/core/loader/FrameLoader.cpp:1705:
m_documentLoader->writer()->setMIMEType(dl->responseMIMEType());
Moved to DocumentLoader.

https://codereview.chromium.org/14787005/diff/3001/Source/core/loader/FrameLo...
Source/core/loader/FrameLoader.cpp:1708: ASSERT(m_frame->view());
This is guaranteed by transitionToCommittedForNewPage.

https://codereview.chromium.org/14787005/diff/3001/Source/core/loader/FrameLo...
Source/core/loader/FrameLoader.cpp:1714:
m_stateMachine.advanceTo(FrameLoaderStateMachine::DisplayingInitialEmptyDocumentPostCommit);
This has been changed to advance to CommittedFirstRealLoad and has been moved
slightly later, in DocumentLoader::commitData()'s handling of first data.

https://codereview.chromium.org/14787005/diff/3001/Source/core/loader/History...
File Source/core/loader/HistoryController.cpp (right):

https://codereview.chromium.org/14787005/diff/3001/Source/core/loader/History...
Source/core/loader/HistoryController.cpp:376: ||
(isReloadTypeWithProvisionalItem(type) &&
!frameLoader->documentLoader()->unreachableURL().isEmpty())) {
This used to be called just before the provisional document loader was promoted
to the committed document loader. With this patch, it is called just after.

[abarth-chromium, 2013-05-06 17:09:42]

LGTM!

Very nice.

https://codereview.chromium.org/14787005/diff/3001/Source/core/loader/FrameLo...
File Source/core/loader/FrameLoader.cpp (left):

https://codereview.chromium.org/14787005/diff/3001/Source/core/loader/FrameLo...
Source/core/loader/FrameLoader.cpp:1705:
m_documentLoader->writer()->setMIMEType(dl->responseMIMEType());
On 2013/05/01 21:38:34, Nate Chapin wrote:
> Moved to DocumentLoader.

Nice.

https://codereview.chromium.org/14787005/diff/3001/Source/core/loader/History...
File Source/core/loader/HistoryController.cpp (right):

https://codereview.chromium.org/14787005/diff/3001/Source/core/loader/History...
Source/core/loader/HistoryController.cpp:376: ||
(isReloadTypeWithProvisionalItem(type) &&
!frameLoader->documentLoader()->unreachableURL().isEmpty())) {
On 2013/05/01 21:38:34, Nate Chapin wrote:
> This used to be called just before the provisional document loader was
promoted
> to the committed document loader. With this patch, it is called just after.

Makes sense.

[commit-bot: I haz the power, 2013-05-06 17:12:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/japhet@chromium.org/14787005/3001

[commit-bot: I haz the power, 2013-05-06 17:12:28]

Failed to apply patch for Source/core/loader/FrameLoader.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/loader/FrameLoader.cpp
  Hunk #1 succeeded at 501 (offset -1 lines).
  Hunk #2 succeeded at 564 (offset -1 lines).
  Hunk #3 succeeded at 607 (offset -1 lines).
  Hunk #4 FAILED at 914.
  Hunk #6 succeeded at 1557 (offset -61 lines).
  Hunk #7 succeeded at 1583 with fuzz 1 (offset -61 lines).
  Hunk #8 succeeded at 1865 (offset -73 lines).
  Hunk #9 succeeded at 2787 (offset -93 lines).
  1 out of 9 hunks FAILED -- saving rejects to file
Source/core/loader/FrameLoader.cpp.rej

Patch:       Source/core/loader/FrameLoader.cpp
Index: Source/core/loader/FrameLoader.cpp
diff --git a/Source/core/loader/FrameLoader.cpp
b/Source/core/loader/FrameLoader.cpp
index
0dbd10893807066b92253f7be86ba6677988bf73..7a4fe20f3a8a9565c41370ed4fa2effb62ceecd8
100644
--- a/Source/core/loader/FrameLoader.cpp
+++ b/Source/core/loader/FrameLoader.cpp
@@ -502,7 +502,7 @@ void FrameLoader::didExplicitOpen()
 
     // Calling document.open counts as committing the first real document load.
     if (!m_stateMachine.committedFirstRealDocumentLoad())
-       
m_stateMachine.advanceTo(FrameLoaderStateMachine::DisplayingInitialEmptyDocumentPostCommit);
+       
m_stateMachine.advanceTo(FrameLoaderStateMachine::CommittedFirstRealLoad);
     
     // Prevent window.open(url) -- eg window.open("about:blank") -- from
blowing away results
     // from a subsequent window.document.open / window.document.write call. 
@@ -565,9 +565,6 @@ void FrameLoader::clear(bool clearWindowProperties, bool
clearScriptObjects, boo
     m_checkTimer.stop();
     m_shouldCallCheckCompleted = false;
     m_shouldCallCheckLoadComplete = false;
-
-    if (m_stateMachine.isDisplayingInitialEmptyDocument() &&
m_stateMachine.committedFirstRealDocumentLoad())
-       
m_stateMachine.advanceTo(FrameLoaderStateMachine::CommittedFirstRealLoad);
 }
 
 void FrameLoader::receivedFirstData()
@@ -611,10 +608,8 @@ void FrameLoader::didBeginDocument(bool dispatch)
     m_didCallImplicitClose = false;
     m_frame->document()->setReadyState(Document::Loading);
 
-    if (m_pendingStateObject) {
-        m_frame->document()->statePopped(m_pendingStateObject.get());
-        m_pendingStateObject.clear();
-    }
+    if (history()->currentItem())
+       
m_frame->document()->statePopped(history()->currentItem()->stateObject());
 
     if (dispatch)
         dispatchDidClearWindowObjectsInAllWorlds();
@@ -919,8 +914,6 @@ void FrameLoader::handleFallbackContent()
 
 void FrameLoader::provisionalLoadStarted()
 {
-    if (m_stateMachine.firstLayoutDone())
-       
m_stateMachine.advanceTo(FrameLoaderStateMachine::CommittedFirstRealLoad);
     m_frame->navigationScheduler()->cancel(true);
 }
 
@@ -1047,7 +1040,6 @@ void FrameLoader::prepareForHistoryNavigation()
         frame()->page()->backForward()->setCurrentItem(currentItem.get());
 
         ASSERT(stateMachine()->isDisplayingInitialEmptyDocument());
-       
stateMachine()->advanceTo(FrameLoaderStateMachine::DisplayingInitialEmptyDocumentPostCommit);
        
stateMachine()->advanceTo(FrameLoaderStateMachine::CommittedFirstRealLoad);
     }
 }
@@ -1624,8 +1616,6 @@ void FrameLoader::transitionToCommitted()
             scrollAnimator->cancelAnimations();
     }
 
-    history()->updateForCommit();
-
     // The call to closeURL() invokes the unload event handler, which can
execute arbitrary
     // JavaScript. If the script initiates a new load, we need to abandon the
current load,
     // or the two will stomp each other.
@@ -1652,66 +1642,8 @@ void FrameLoader::transitionToCommitted()
     if (isLoadingMainFrame())
         m_frame->page()->chrome()->client()->needTouchEvents(false);
 
-    // Handle adding the URL to the back/forward list.
-    DocumentLoader* dl = m_documentLoader.get();
-
-    switch (m_loadType) {
-        case FrameLoadTypeForward:
-        case FrameLoadTypeBack:
-        case FrameLoadTypeIndexedBackForward:
-            if (m_frame->page()) {
-                // If the first load within a frame is a navigation within a
back/forward list that was attached
-                // without any of the items being loaded then we need to update
the history in a similar manner as
-                // for a standard load with the exception of updating the
back/forward list (<rdar://problem/8091103>).
-                if (!m_stateMachine.committedFirstRealDocumentLoad() &&
isLoadingMainFrame())
-                   
history()->updateForStandardLoad(HistoryController::UpdateAllExceptBackForwardList);
-
-                history()->updateForBackForwardNavigation();
-
-                if (history()->currentItem())
-                    m_pendingStateObject =
history()->currentItem()->stateObject();
-
-                // Create a document view for this document.
-                m_client->transitionToCommittedForNewPage();
-            }
-            break;
-
-        case FrameLoadTypeReload:
-        case FrameLoadTypeReloadFromOrigin:
-        case FrameLoadTypeSame:
-        case FrameLoadTypeReplace:
-            history()->updateForReload();
-            m_client->transitionToCommittedForNewPage();
-            break;
-
-        case FrameLoadTypeStandard:
-            history()->updateForStandardLoad();
-            if (m_frame->view())
-                m_frame->view()->setScrollbarsSuppressed(true);
-            m_client->transitionToCommittedForNewPage();
-            break;
-
-        case FrameLoadTypeRedirectWithLockedBackForwardList:
-            history()->updateForRedirectWithLockedBackForwardList();
-            m_client->transitionToCommittedForNewPage();
-            break;
-
-        // FIXME Remove this check when dummy ds is removed (whatever "dummy
ds" is).
-        // An exception should be thrown if we're in the
FrameLoadTypeUninitialized state.
-        default:
-            ASSERT_NOT_REACHED();
-    }
-
-    m_documentLoader->writer()->setMIMEType(dl->responseMIMEType());
-
-    // Tell the client we've committed this URL.
-    ASSERT(m_frame->view());
-
-    if (m_stateMachine.creatingInitialEmptyDocument())
-        return;
-
-    if (!m_stateMachine.committedFirstRealDocumentLoad())
-       
m_stateMachine.advanceTo(FrameLoaderStateMachine::DisplayingInitialEmptyDocumentPostCommit);
+    history()->updateForCommit();
+    m_client->transitionToCommittedForNewPage();
 }
 
 void FrameLoader::clientRedirectCancelledOrFinished(bool
cancelWithLoadInProgress)
@@ -2004,20 +1936,12 @@ void FrameLoader::didFirstLayout()
 {
     if (m_frame->page() && isBackForwardLoadType(m_loadType))
         history()->restoreScrollPositionAndViewState();
-
-    if (m_stateMachine.committedFirstRealDocumentLoad() &&
!m_stateMachine.isDisplayingInitialEmptyDocument() &&
!m_stateMachine.firstLayoutDone())
-        m_stateMachine.advanceTo(FrameLoaderStateMachine::FirstLayoutDone);
 }
 
 void FrameLoader::frameLoadCompleted()
 {
     // Note: Can be called multiple times.
     history()->updateForFrameLoadCompleted();
-
-    // After a canceled provisional load, firstLayoutDone is false.
-    // Reset it to true if we're displaying a page.
-    if (m_documentLoader && m_stateMachine.committedFirstRealDocumentLoad() &&
!m_stateMachine.isDisplayingInitialEmptyDocument() &&
!m_stateMachine.firstLayoutDone())
-        m_stateMachine.advanceTo(FrameLoaderStateMachine::FirstLayoutDone);
 }
 
 void FrameLoader::detachChildren()
@@ -2954,7 +2878,6 @@ void FrameLoader::reportMemoryUsage(MemoryObjectInfo*
memoryObjectInfo) const
     info.addMember(m_documentLoader, "documentLoader");
     info.addMember(m_provisionalDocumentLoader, "provisionalDocumentLoader");
     info.addMember(m_policyDocumentLoader, "policyDocumentLoader");
-    info.addMember(m_pendingStateObject, "pendingStateObject");
     info.addMember(m_submittedFormURL, "submittedFormURL");
     info.addMember(m_checkTimer, "checkTimer");
     info.addMember(m_opener, "opener");

[commit-bot: I haz the power, 2013-05-06 17:16:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/japhet@chromium.org/14787005/14001

[commit-bot: I haz the power, 2013-05-06 18:12:18]

Retried try job too often on linux_layout_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layo...

[commit-bot: I haz the power, 2013-05-06 18:26:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/japhet@chromium.org/14787005/14001

[commit-bot: I haz the power, 2013-05-06 18:38:28]

Change committed as 149766