2011-01-26 Nate Chapin <japhet@chromium.org>
authorjaphet@chromium.org <japhet@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Jan 2011 19:14:26 +0000 (19:14 +0000)
committerjaphet@chromium.org <japhet@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Jan 2011 19:14:26 +0000 (19:14 +0000)
        Reviewed by Adam Barth.

        Remove m_URL from FrameLoader and depend on Document::url()
        instead. FrameLoader::url() will be removed in a followup patch.
        https://bugs.webkit.org/show_bug.cgi?id=41165

        Refactor only, no new tests.

        * WebCore.exp.in:
        * dom/Document.cpp:
        (WebCore::Document::Document):
        (WebCore::Document::updateURLForPushOrReplaceState):
        * loader/DocumentWriter.cpp:
        (WebCore::DocumentWriter::begin):
        * loader/FrameLoader.cpp:
        (WebCore::FrameLoader::iconURL):
        (WebCore::FrameLoader::didOpenURL):
        (WebCore::FrameLoader::didExplicitOpen):
        (WebCore::FrameLoader::receivedFirstData):
        (WebCore::FrameLoader::url):
        (WebCore::FrameLoader::setOutgoingReferrer):
        (WebCore::FrameLoader::startIconLoader):
        (WebCore::FrameLoader::commitIconURLToIconDatabase):
        (WebCore::FrameLoader::finishedParsing):
        (WebCore::FrameLoader::checkIfDisplayInsecureContent):
        (WebCore::FrameLoader::checkIfRunInsecureContent):
        (WebCore::FrameLoader::updateFirstPartyForCookies):
        (WebCore::FrameLoader::loadInSameDocument):
        (WebCore::FrameLoader::commitProvisionalLoad):
        (WebCore::FrameLoader::open):
        (WebCore::FrameLoader::shouldScrollToAnchor):
        * loader/FrameLoader.h: Rename setURL() to setOutgoingReferrer().

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@76702 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/dom/Document.cpp
Source/WebCore/loader/DocumentWriter.cpp
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/FrameLoader.h

index 93e0491..e52126a 100644 (file)
@@ -1,3 +1,38 @@
+2011-01-26  Nate Chapin  <japhet@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        Remove m_URL from FrameLoader and depend on Document::url()
+        instead. FrameLoader::url() will be removed in a followup patch.
+        https://bugs.webkit.org/show_bug.cgi?id=41165
+
+        Refactor only, no new tests.
+
+        * WebCore.exp.in:
+        * dom/Document.cpp:
+        (WebCore::Document::Document):
+        (WebCore::Document::updateURLForPushOrReplaceState):
+        * loader/DocumentWriter.cpp:
+        (WebCore::DocumentWriter::begin):
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::iconURL):
+        (WebCore::FrameLoader::didOpenURL):
+        (WebCore::FrameLoader::didExplicitOpen):
+        (WebCore::FrameLoader::receivedFirstData):
+        (WebCore::FrameLoader::url):
+        (WebCore::FrameLoader::setOutgoingReferrer):
+        (WebCore::FrameLoader::startIconLoader):
+        (WebCore::FrameLoader::commitIconURLToIconDatabase):
+        (WebCore::FrameLoader::finishedParsing):
+        (WebCore::FrameLoader::checkIfDisplayInsecureContent):
+        (WebCore::FrameLoader::checkIfRunInsecureContent):
+        (WebCore::FrameLoader::updateFirstPartyForCookies):
+        (WebCore::FrameLoader::loadInSameDocument):
+        (WebCore::FrameLoader::commitProvisionalLoad):
+        (WebCore::FrameLoader::open):
+        (WebCore::FrameLoader::shouldScrollToAnchor):
+        * loader/FrameLoader.h: Rename setURL() to setOutgoingReferrer().
+
 2011-01-25  Brian Weinstein  <bweinstein@apple.com>
 
         Reviewed by Antti Koivisto.
index 9b2117d..acc6cb9 100644 (file)
@@ -971,6 +971,7 @@ __ZNK7WebCore11FrameLoader16outgoingReferrerEv
 __ZNK7WebCore11FrameLoader17networkingContextEv
 __ZNK7WebCore11FrameLoader20activeDocumentLoaderEv
 __ZNK7WebCore11FrameLoader27numPendingOrLoadingRequestsEb
+__ZNK7WebCore11FrameLoader3urlEv
 __ZNK7WebCore11FrameLoader8loadTypeEv
 __ZNK7WebCore11HistoryItem10visitCountEv
 __ZNK7WebCore11HistoryItem11hasChildrenEv
index 7bbd0e0..ea17763 100644 (file)
@@ -444,7 +444,12 @@ Document::Document(Frame* frame, const KURL& url, bool isXHTML, bool isHTML, con
 
     m_frame = frame;
 
-    if (frame || !url.isEmpty())
+    // We depend on the url getting immediately set in subframes, but we
+    // also depend on the url NOT getting immediately set in opened windows.
+    // See fast/dom/early-frame-url.html
+    // and fast/dom/location-new-window-no-crash.html, respectively.
+    // FIXME: Can/should we unify this behavior?
+    if ((frame && frame->ownerElement()) || !url.isEmpty())
         setURL(url);
 
     // Setting of m_baseURL needs to happen after the setURL call, since that
@@ -4567,9 +4572,8 @@ void Document::updateURLForPushOrReplaceState(const KURL& url)
     if (!f)
         return;
 
-    // FIXME: Eliminate this redundancy.
     setURL(url);
-    f->loader()->setURL(url);
+    f->loader()->setOutgoingReferrer(url);
     f->loader()->documentLoader()->replaceRequestURLForSameDocumentNavigation(url);
 }
 
index 5b03cd7..d3dcac6 100644 (file)
@@ -126,7 +126,7 @@ void DocumentWriter::begin(const KURL& url, bool dispatch, SecurityOrigin* origi
     if (resetScripting)
         m_frame->script()->updatePlatformScriptObjects();
 
-    m_frame->loader()->setURL(url);
+    m_frame->loader()->setOutgoingReferrer(url);
     m_frame->setDocument(document);
 
     if (m_decoder)
index 1129bac..8c2979a 100644 (file)
@@ -476,15 +476,16 @@ KURL FrameLoader::iconURL()
         return KURL(ParsedURLString, m_frame->document()->iconURL());
 
     // Don't return a favicon iconURL unless we're http or https
-    if (!m_URL.protocolInHTTPFamily())
+    KURL documentURL = m_frame->document()->url();
+    if (!documentURL.protocolInHTTPFamily())
         return KURL();
 
     KURL url;
-    bool couldSetProtocol = url.setProtocol(m_URL.protocol());
+    bool couldSetProtocol = url.setProtocol(documentURL.protocol());
     ASSERT_UNUSED(couldSetProtocol, couldSetProtocol);
-    url.setHost(m_URL.host());
-    if (m_URL.hasPort())
-        url.setPort(m_URL.port());
+    url.setHost(documentURL.host());
+    if (documentURL.hasPort())
+        url.setPort(documentURL.port());
     url.setPath("/favicon.ico");
     return url;
 }
@@ -513,10 +514,9 @@ bool FrameLoader::didOpenURL(const KURL& url)
             window->setDefaultStatus(String());
         }
     }
-    m_URL = url;
-    if (m_URL.protocolInHTTPFamily() && !m_URL.host().isEmpty() && m_URL.path().isEmpty())
-        m_URL.setPath("/");
-    m_workingURL = m_URL;
+    m_workingURL = url;
+    if (m_workingURL.protocolInHTTPFamily() && !m_workingURL.host().isEmpty() && m_workingURL.path().isEmpty())
+        m_workingURL.setPath("/");
 
     started();
 
@@ -536,9 +536,7 @@ void FrameLoader::didExplicitOpen()
     // from a subsequent window.document.open / window.document.write call. 
     // Canceling redirection here works for all cases because document.open 
     // implicitly precedes document.write.
-    m_frame->navigationScheduler()->cancel(); 
-    if (m_frame->document()->url() != blankURL())
-        m_URL = m_frame->document()->url();
+    m_frame->navigationScheduler()->cancel();
 }
 
 
@@ -630,21 +628,26 @@ void FrameLoader::receivedFirstData()
         return;
 
     if (url.isEmpty())
-        url = m_URL.string();
+        url = m_frame->document()->url().string();
     else
         url = m_frame->document()->completeURL(url).string();
 
     m_frame->navigationScheduler()->scheduleRedirect(delay, url);
 }
 
-void FrameLoader::setURL(const KURL& url)
+const KURL& FrameLoader::url() const
+{
+    ASSERT(m_frame->document());
+    return m_frame->document()->url();
+}
+
+void FrameLoader::setOutgoingReferrer(const KURL& url)
 {
-    KURL ref(url);
-    ref.setUser(String());
-    ref.setPass(String());
-    ref.removeFragmentIdentifier();
-    m_outgoingReferrer = ref.string();
-    m_URL = url;
+    KURL outgoingReferrer(url);
+    outgoingReferrer.setUser(String());
+    outgoingReferrer.setPass(String());
+    outgoingReferrer.removeFragmentIdentifier();
+    m_outgoingReferrer = outgoingReferrer.string();
 }
 
 void FrameLoader::didBeginDocument(bool dispatch)
@@ -720,7 +723,7 @@ void FrameLoader::startIconLoader()
             if (!iconDatabase()->iconDataKnownForIconURL(urlString)) {
                 LOG(IconDatabase, "Told not to load icon %s but icon data is not yet available - registering for notification and requesting load from disk", urlString.ascii().data());
                 m_client->registerForIconNotification();
-                iconDatabase()->iconForPageURL(m_URL.string(), IntSize(0, 0));
+                iconDatabase()->iconForPageURL(m_frame->document()->url().string(), IntSize(0, 0));
                 iconDatabase()->iconForPageURL(originalRequestURL().string(), IntSize(0, 0));
             } else
                 m_client->dispatchDidReceiveIcon();
@@ -757,8 +760,8 @@ void FrameLoader::startIconLoader()
 void FrameLoader::commitIconURLToIconDatabase(const KURL& icon)
 {
     ASSERT(iconDatabase());
-    LOG(IconDatabase, "Committing iconURL %s to database for pageURLs %s and %s", icon.string().ascii().data(), m_URL.string().ascii().data(), originalRequestURL().string().ascii().data());
-    iconDatabase()->setIconURLForPageURL(icon.string(), m_URL.string());
+    LOG(IconDatabase, "Committing iconURL %s to database for pageURLs %s and %s", icon.string().ascii().data(), m_frame->document()->url().string().ascii().data(), originalRequestURL().string().ascii().data());
+    iconDatabase()->setIconURLForPageURL(icon.string(), m_frame->document()->url().string());
     iconDatabase()->setIconURLForPageURL(icon.string(), originalRequestURL().string());
 }
 
@@ -784,7 +787,7 @@ void FrameLoader::finishedParsing()
     // Check if the scrollbars are really needed for the content.
     // If not, remove them, relayout, and repaint.
     m_frame->view()->restoreScrollbar();
-    m_frame->view()->scrollToFragment(m_URL);
+    m_frame->view()->scrollToFragment(m_frame->document()->url());
 }
 
 void FrameLoader::loadDone()
@@ -1023,7 +1026,7 @@ void FrameLoader::checkIfDisplayInsecureContent(SecurityOrigin* context, const K
     if (!isMixedContent(context, url))
         return;
 
-    String message = makeString("The page at ", m_URL.string(), " displayed insecure content from ", url.string(), ".\n");
+    String message = makeString("The page at ", m_frame->document()->url().string(), " displayed insecure content from ", url.string(), ".\n");
     m_frame->domWindow()->console()->addMessage(HTMLMessageSource, LogMessageType, WarningMessageLevel, message, 1, String());
 
     m_client->didDisplayInsecureContent();
@@ -1034,7 +1037,7 @@ void FrameLoader::checkIfRunInsecureContent(SecurityOrigin* context, const KURL&
     if (!isMixedContent(context, url))
         return;
 
-    String message = makeString("The page at ", m_URL.string(), " ran insecure content from ", url.string(), ".\n");
+    String message = makeString("The page at ", m_frame->document()->url().string(), " ran insecure content from ", url.string(), ".\n");
     m_frame->domWindow()->console()->addMessage(HTMLMessageSource, LogMessageType, WarningMessageLevel, message, 1, String());
 
     m_client->didRunInsecureContent(context);
@@ -1111,7 +1114,7 @@ void FrameLoader::updateFirstPartyForCookies()
     if (m_frame->tree()->parent())
         setFirstPartyForCookies(m_frame->tree()->parent()->document()->firstPartyForCookies());
     else
-        setFirstPartyForCookies(m_URL);
+        setFirstPartyForCookies(m_frame->document()->url());
 }
 
 void FrameLoader::setFirstPartyForCookies(const KURL& url)
@@ -1129,6 +1132,7 @@ void FrameLoader::loadInSameDocument(const KURL& url, SerializedScriptValue* sta
     ASSERT(!stateObject || (stateObject && !isNewNavigation));
 
     // Update the data source's request with the new URL to fake the URL change
+    KURL oldURL = m_frame->document()->url();
     m_frame->document()->setURL(url);
     documentLoader()->replaceRequestURLForSameDocumentNavigation(url);
     if (isNewNavigation && !shouldTreatURLAsSameAsCurrent(url) && !stateObject) {
@@ -1146,11 +1150,8 @@ void FrameLoader::loadInSameDocument(const KURL& url, SerializedScriptValue* sta
         history()->updateBackForwardListForFragmentScroll();
     }
     
-    String oldURL;
-    bool hashChange = equalIgnoringFragmentIdentifier(url, m_URL) && url.fragmentIdentifier() != m_URL.fragmentIdentifier();
-    oldURL = m_URL;
+    bool hashChange = equalIgnoringFragmentIdentifier(url, oldURL) && url.fragmentIdentifier() != oldURL.fragmentIdentifier();
     
-    m_URL = url;
     history()->updateForSameDocumentNavigation();
 
     // If we were in the autoscroll/panScroll mode we want to stop it before following the link to the anchor
@@ -1164,7 +1165,7 @@ void FrameLoader::loadInSameDocument(const KURL& url, SerializedScriptValue* sta
     // We need to scroll to the fragment whether or not a hash change occurred, since
     // the user might have scrolled since the previous navigation.
     if (FrameView* view = m_frame->view())
-        view->scrollToFragment(m_URL);
+        view->scrollToFragment(url);
     
     m_isComplete = false;
     checkCompleted();
@@ -1849,7 +1850,8 @@ void FrameLoader::commitProvisionalLoad()
     RefPtr<CachedPage> cachedPage = m_loadingFromCachedPage ? pageCache()->get(history()->provisionalItem()) : 0;
     RefPtr<DocumentLoader> pdl = m_provisionalDocumentLoader;
 
-    LOG(PageCache, "WebCoreLoading %s: About to commit provisional load from previous URL '%s' to new URL '%s'", m_frame->tree()->uniqueName().string().utf8().data(), m_URL.string().utf8().data(), 
+    LOG(PageCache, "WebCoreLoading %s: About to commit provisional load from previous URL '%s' to new URL '%s'", m_frame->tree()->uniqueName().string().utf8().data(),
+        m_frame->document() ? m_frame->document()->url().string().utf8().data() : "", 
         pdl ? pdl->url().string().utf8().data() : "<no provisional DocumentLoader>");
 
     // Check to see if we need to cache the page we are navigating away from into the back/forward cache.
@@ -1903,7 +1905,8 @@ void FrameLoader::commitProvisionalLoad()
         didOpenURL(url);
     }
 
-    LOG(Loading, "WebCoreLoading %s: Finished committing provisional load to URL %s", m_frame->tree()->uniqueName().string().utf8().data(), m_URL.string().utf8().data());
+    LOG(Loading, "WebCoreLoading %s: Finished committing provisional load to URL %s", m_frame->tree()->uniqueName().string().utf8().data(),
+        m_frame->document() ? m_frame->document()->url().string().utf8().data() : "");
 
     if (m_loadType == FrameLoadTypeStandard && m_documentLoader->isClientRedirect())
         history()->updateForClientRedirect();
@@ -2136,7 +2139,6 @@ void FrameLoader::open(CachedFrameBase& cachedFrame)
     if (url.protocolInHTTPFamily() && !url.host().isEmpty() && url.path().isEmpty())
         url.setPath("/");
     
-    m_URL = url;
     m_workingURL = url;
 
     started();
@@ -2862,7 +2864,7 @@ bool FrameLoader::shouldScrollToAnchor(bool isFormSubmission, const String& http
         && loadType != FrameLoadTypeReload
         && loadType != FrameLoadTypeReloadFromOrigin
         && loadType != FrameLoadTypeSame
-        && !shouldReload(this->url(), url)
+        && !shouldReload(m_frame->document()->url(), url)
         // We don't want to just scroll if a link from within a
         // frameset is trying to reload the frameset into _top.
         && !m_frame->document()->isFrameSet();
index 427cdee..0fe8776 100644 (file)
@@ -267,10 +267,8 @@ public:
 
     void frameDetached();
 
-    const KURL& url() const { return m_URL; }
-
-    // setURL is a low-level setter and does not trigger loading.
-    void setURL(const KURL&);
+    const KURL& url() const;
+    void setOutgoingReferrer(const KURL&);
 
     void loadDone();
     void finishedParsing();
@@ -468,7 +466,6 @@ private:
 
     RefPtr<SerializedScriptValue> m_pendingStateObject;
 
-    KURL m_URL;
     KURL m_workingURL;
 
     OwnPtr<IconLoader> m_iconLoader;