CTTE: FrameTree::top() should return a reference.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Sep 2013 20:09:25 +0000 (20:09 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Sep 2013 20:09:25 +0000 (20:09 +0000)
<https://webkit.org/b/121445>

Reviewed by Anders Carlsson.

There's always a top frame in the tree.

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

Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp
Source/WebCore/loader/cache/CachedResourceLoader.cpp
Source/WebCore/page/DOMWindow.cpp
Source/WebCore/page/FrameTree.cpp
Source/WebCore/page/FrameTree.h
Source/WebCore/page/FrameView.cpp

index c525936..f534a8d 100644 (file)
@@ -1,3 +1,12 @@
+2013-09-16  Andreas Kling  <akling@apple.com>
+
+        CTTE: FrameTree::top() should return a reference.
+        <https://webkit.org/b/121445>
+
+        Reviewed by Anders Carlsson.
+
+        There's always a top frame in the tree.
+
 2013-09-16  Zan Dobersek  <zdobersek@igalia.com>
 
         Unreviewed, fixing GObject bindings tests after r155850 by adding the *.symbols files that are now also generated.
index 6847a21..ae5b73f 100644 (file)
@@ -2745,7 +2745,7 @@ bool Document::canNavigate(Frame* targetFrame)
         return true;
 
     // Frame-busting is generally allowed, but blocked for sandboxed frames lacking the 'allow-top-navigation' flag.
-    if (!isSandboxed(SandboxTopNavigation) && targetFrame == m_frame->tree().top())
+    if (!isSandboxed(SandboxTopNavigation) && targetFrame == &m_frame->tree().top())
         return true;
 
     if (isSandboxed(SandboxNavigation)) {
@@ -2753,7 +2753,7 @@ bool Document::canNavigate(Frame* targetFrame)
             return true;
 
         const char* reason = "The frame attempting navigation is sandboxed, and is therefore disallowed from navigating its ancestors.";
-        if (isSandboxed(SandboxTopNavigation) && targetFrame == m_frame->tree().top())
+        if (isSandboxed(SandboxTopNavigation) && targetFrame == &m_frame->tree().top())
             reason = "The frame attempting navigation of the top-level window is sandboxed, but the 'allow-top-navigation' flag is not set.";
 
         printNavigationErrorMessage(targetFrame, url(), reason);
index de92970..f28b0d5 100644 (file)
@@ -505,9 +505,9 @@ void DocumentLoader::willSendRequest(ResourceRequest& newRequest, const Resource
     if (newRequest.cachePolicy() == UseProtocolCachePolicy && isPostOrRedirectAfterPost(newRequest, redirectResponse))
         newRequest.setCachePolicy(ReloadIgnoringCacheData);
 
-    Frame* top = m_frame->tree().top();
-    if (top != m_frame) {
-        if (!frameLoader()->mixedContentChecker().canDisplayInsecureContent(top->document()->securityOrigin(), newRequest.url())) {
+    Frame& topFrame = m_frame->tree().top();
+    if (&topFrame != m_frame) {
+        if (!frameLoader()->mixedContentChecker().canDisplayInsecureContent(topFrame.document()->securityOrigin(), newRequest.url())) {
             cancelMainResourceLoad(frameLoader()->cancelledError(newRequest));
             return;
         }
index 4d7d899..4b78e4a 100644 (file)
@@ -1215,8 +1215,8 @@ void FrameLoader::loadURL(const KURL& newURL, const String& referrer, const Stri
         addHTTPOriginIfNeeded(request, referrerOrigin->toString());
     }
 #if ENABLE(CACHE_PARTITIONING)
-    if (m_frame.tree().top() != &m_frame)
-        request.setCachePartition(m_frame.tree().top()->document()->securityOrigin()->cachePartition());
+    if (&m_frame.tree().top() != &m_frame)
+        request.setCachePartition(m_frame.tree().top().document()->securityOrigin()->cachePartition());
 #endif
     addExtraFieldsToRequest(request, newLoadType, true);
     if (newLoadType == FrameLoadTypeReload || newLoadType == FrameLoadTypeReloadFromOrigin)
@@ -2981,8 +2981,8 @@ bool FrameLoader::shouldInterruptLoadForXFrameOptions(const String& content, con
 {
     FeatureObserver::observe(m_frame.document(), FeatureObserver::XFrameOptions);
 
-    Frame* topFrame = m_frame.tree().top();
-    if (&m_frame == topFrame)
+    Frame& topFrame = m_frame.tree().top();
+    if (&m_frame == &topFrame)
         return false;
 
     XFrameOptionsDisposition disposition = parseXFrameOptionsHeader(content);
@@ -2991,7 +2991,7 @@ bool FrameLoader::shouldInterruptLoadForXFrameOptions(const String& content, con
     case XFrameOptionsSameOrigin: {
         FeatureObserver::observe(m_frame.document(), FeatureObserver::XFrameOptionsSameOrigin);
         RefPtr<SecurityOrigin> origin = SecurityOrigin::create(url);
-        if (!origin->isSameSchemeHostPort(topFrame->document()->securityOrigin()))
+        if (!origin->isSameSchemeHostPort(topFrame.document()->securityOrigin()))
             return true;
         for (Frame* frame = m_frame.tree().parent(); frame; frame = frame->tree().parent()) {
             if (!origin->isSameSchemeHostPort(frame->document()->securityOrigin())) {
index b78582e..d6bf987 100644 (file)
@@ -141,7 +141,7 @@ void ApplicationCacheGroup::selectCache(Frame* frame, const KURL& passedManifest
     if (!frame->settings().offlineWebApplicationCacheEnabled())
         return;
 
-    if (!frame->document()->securityOrigin()->canAccessApplicationCache(frame->tree().top()->document()->securityOrigin()))
+    if (!frame->document()->securityOrigin()->canAccessApplicationCache(frame->tree().top().document()->securityOrigin()))
         return;
     
     DocumentLoader* documentLoader = frame->loader().documentLoader();
@@ -217,7 +217,7 @@ void ApplicationCacheGroup::selectCacheWithoutManifestURL(Frame* frame)
     if (!frame->settings().offlineWebApplicationCacheEnabled())
         return;
 
-    if (!frame->document()->securityOrigin()->canAccessApplicationCache(frame->tree().top()->document()->securityOrigin()))
+    if (!frame->document()->securityOrigin()->canAccessApplicationCache(frame->tree().top().document()->securityOrigin()))
         return;
 
     DocumentLoader* documentLoader = frame->loader().documentLoader();
index 8ca1508..2fe976a 100644 (file)
@@ -286,8 +286,8 @@ bool CachedResourceLoader::checkInsecureContent(CachedResource::Type type, const
     case CachedResource::FontResource: {
         // These resources can corrupt only the frame's pixels.
         if (Frame* f = frame()) {
-            Frame* top = f->tree().top();
-            if (!top->loader().mixedContentChecker().canDisplayInsecureContent(top->document()->securityOrigin(), url))
+            Frame& topFrame = f->tree().top();
+            if (!topFrame.loader().mixedContentChecker().canDisplayInsecureContent(topFrame.document()->securityOrigin(), url))
                 return false;
         }
         break;
index b2374fe..c4bf6df 100644 (file)
@@ -1331,7 +1331,7 @@ DOMWindow* DOMWindow::top() const
     if (!page)
         return 0;
 
-    return m_frame->tree().top()->document()->domWindow();
+    return m_frame->tree().top().document()->domWindow();
 }
 
 Document* DOMWindow::document() const
@@ -1949,7 +1949,7 @@ PassRefPtr<DOMWindow> DOMWindow::open(const String& urlString, const AtomicStrin
     // In those cases, we schedule a location change right now and return early.
     Frame* targetFrame = 0;
     if (frameName == "_top")
-        targetFrame = m_frame->tree().top();
+        targetFrame = &m_frame->tree().top();
     else if (frameName == "_parent") {
         if (Frame* parent = m_frame->tree().parent())
             targetFrame = parent;
index 82dfe1f..2083e6f 100644 (file)
@@ -272,7 +272,7 @@ Frame* FrameTree::find(const AtomicString& name) const
         return m_thisFrame;
     
     if (name == "_top")
-        return top();
+        return &top();
     
     if (name == "_parent")
         return parent() ? parent() : m_thisFrame;
@@ -398,12 +398,12 @@ Frame* FrameTree::deepLastChild() const
     return result;
 }
 
-Frame* FrameTree::top() const
+Frame& FrameTree::top() const
 {
     Frame* frame = m_thisFrame;
     for (Frame* parent = m_thisFrame; parent; parent = parent->tree().parent())
         frame = parent;
-    return frame;
+    return *frame;
 }
 
 } // namespace WebCore
@@ -416,27 +416,27 @@ static void printIndent(int indent)
         printf("    ");
 }
 
-static void printFrames(const WebCore::Frame* frame, const WebCore::Frame* targetFrame, int indent)
+static void printFrames(const WebCore::Frame& frame, const WebCore::Frame* targetFrame, int indent)
 {
-    if (frame == targetFrame) {
+    if (&frame == targetFrame) {
         printf("--> ");
         printIndent(indent - 1);
     } else
         printIndent(indent);
 
-    WebCore::FrameView* view = frame->view();
-    printf("Frame %p %dx%d\n", frame, view ? view->width() : 0, view ? view->height() : 0);
+    WebCore::FrameView* view = frame.view();
+    printf("Frame %p %dx%d\n", &frame, view ? view->width() : 0, view ? view->height() : 0);
     printIndent(indent);
-    printf("  ownerElement=%p\n", frame->ownerElement());
+    printf("  ownerElement=%p\n", frame.ownerElement());
     printIndent(indent);
     printf("  frameView=%p\n", view);
     printIndent(indent);
-    printf("  document=%p\n", frame->document());
+    printf("  document=%p\n", frame.document());
     printIndent(indent);
-    printf("  uri=%s\n\n", frame->document()->documentURI().utf8().data());
+    printf("  uri=%s\n\n", frame.document()->documentURI().utf8().data());
 
-    for (WebCore::Frame* child = frame->tree().firstChild(); child; child = child->tree().nextSibling())
-        printFrames(child, targetFrame, indent + 1);
+    for (WebCore::Frame* child = frame.tree().firstChild(); child; child = child->tree().nextSibling())
+        printFrames(*child, targetFrame, indent + 1);
 }
 
 void showFrameTree(const WebCore::Frame* frame)
index 5ee7171..80c4228 100644 (file)
@@ -72,7 +72,7 @@ namespace WebCore {
 
         AtomicString uniqueChildName(const AtomicString& requestedName) const;
 
-        Frame* top() const;
+        Frame& top() const;
 
         Frame* scopedChild(unsigned index) const;
         Frame* scopedChild(const AtomicString& name) const;
index f817af1..80fdd64 100644 (file)
@@ -2276,7 +2276,7 @@ void FrameView::doDeferredRepaints()
 bool FrameView::shouldUseLoadTimeDeferredRepaintDelay() const
 {
     // Don't defer after the initial load of the page has been completed.
-    if (frame().tree().top()->loader().isComplete())
+    if (frame().tree().top().loader().isComplete())
         return false;
     Document* document = frame().document();
     if (!document)
@@ -4022,7 +4022,7 @@ void FrameView::setTracksRepaints(bool trackRepaints)
     }
 
 #if USE(ACCELERATED_COMPOSITING)
-    for (Frame* frame = m_frame->tree().top(); frame; frame = frame->tree().traverseNext()) {
+    for (Frame* frame = &m_frame->tree().top(); frame; frame = frame->tree().traverseNext()) {
         if (RenderView* renderView = frame->contentRenderer())
             renderView->compositor().setTracksRepaints(trackRepaints);
     }