CTTE: FrameLoader::notifier() should return a reference.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 6 Oct 2013 21:07:29 +0000 (21:07 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 6 Oct 2013 21:07:29 +0000 (21:07 +0000)
<https://webkit.org/b/122424>

Reviewed by Anders Carlsson.

It was just returning the address of an inline member, so we should
use a reference instead. Also made the backpointer to Frame in
ResourceLoadNotifier a reference.

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

Source/WebCore/ChangeLog
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/FrameLoader.h
Source/WebCore/loader/ResourceLoadNotifier.cpp
Source/WebCore/loader/ResourceLoadNotifier.h
Source/WebCore/loader/ResourceLoader.cpp
Source/WebCore/loader/mac/ResourceLoaderMac.mm

index 8ec09e9..3e7de5b 100644 (file)
@@ -1,5 +1,16 @@
 2013-10-06  Andreas Kling  <akling@apple.com>
 
+        CTTE: FrameLoader::notifier() should return a reference.
+        <https://webkit.org/b/122424>
+
+        Reviewed by Anders Carlsson.
+
+        It was just returning the address of an inline member, so we should
+        use a reference instead. Also made the backpointer to Frame in
+        ResourceLoadNotifier a reference.
+
+2013-10-06  Andreas Kling  <akling@apple.com>
+
         Avoid layout in window.scroll{Y,X} when at topmost/leftmost position.
         <https://webkit.org/b/122423>
 
index 875f132..72da450 100644 (file)
@@ -371,7 +371,7 @@ void DocumentLoader::finishedLoading(double finishTime)
         // cancel the already-finished substitute load.
         unsigned long identifier = m_identifierForLoadWithoutResourceLoader;
         m_identifierForLoadWithoutResourceLoader = 0;
-        frameLoader()->notifier()->dispatchDidFinishLoading(this, identifier, finishTime);
+        frameLoader()->notifier().dispatchDidFinishLoading(this, identifier, finishTime);
     }
 
 #if USE(CONTENT_FILTERING)
@@ -613,7 +613,7 @@ void DocumentLoader::responseReceived(CachedResource* resource, const ResourceRe
 
     if (m_identifierForLoadWithoutResourceLoader) {
         addResponse(m_response);
-        frameLoader()->notifier()->dispatchDidReceiveResponse(this, m_identifierForLoadWithoutResourceLoader, m_response, 0);
+        frameLoader()->notifier().dispatchDidReceiveResponse(this, m_identifierForLoadWithoutResourceLoader, m_response, 0);
     }
 
     ASSERT(!m_waitingForContentPolicy);
@@ -852,7 +852,7 @@ void DocumentLoader::dataReceived(CachedResource* resource, const char* data, in
 #endif
 
     if (m_identifierForLoadWithoutResourceLoader)
-        frameLoader()->notifier()->dispatchDidReceiveData(this, m_identifierForLoadWithoutResourceLoader, data, length, -1);
+        frameLoader()->notifier().dispatchDidReceiveData(this, m_identifierForLoadWithoutResourceLoader, data, length, -1);
 
     m_applicationCacheHost->mainResourceDataReceived(data, length, -1, false);
     m_timeOfLastDataReceived = monotonicallyIncreasingTime();
@@ -1378,8 +1378,8 @@ void DocumentLoader::startLoadingMainResource()
 
     if (m_substituteData.isValid()) {
         m_identifierForLoadWithoutResourceLoader = m_frame->page()->progress().createUniqueIdentifier();
-        frameLoader()->notifier()->assignIdentifierToInitialRequest(m_identifierForLoadWithoutResourceLoader, this, m_request);
-        frameLoader()->notifier()->dispatchWillSendRequest(this, m_identifierForLoadWithoutResourceLoader, m_request, ResourceResponse());
+        frameLoader()->notifier().assignIdentifierToInitialRequest(m_identifierForLoadWithoutResourceLoader, this, m_request);
+        frameLoader()->notifier().dispatchWillSendRequest(this, m_identifierForLoadWithoutResourceLoader, m_request, ResourceResponse());
         handleSubstituteDataLoadSoon();
         return;
     }
@@ -1401,8 +1401,8 @@ void DocumentLoader::startLoadingMainResource()
 
     if (!mainResourceLoader()) {
         m_identifierForLoadWithoutResourceLoader = m_frame->page()->progress().createUniqueIdentifier();
-        frameLoader()->notifier()->assignIdentifierToInitialRequest(m_identifierForLoadWithoutResourceLoader, this, request);
-        frameLoader()->notifier()->dispatchWillSendRequest(this, m_identifierForLoadWithoutResourceLoader, request, ResourceResponse());
+        frameLoader()->notifier().assignIdentifierToInitialRequest(m_identifierForLoadWithoutResourceLoader, this, request);
+        frameLoader()->notifier().dispatchWillSendRequest(this, m_identifierForLoadWithoutResourceLoader, request, ResourceResponse());
     }
     m_mainResource->addClient(this);
 
index c918f13..42f1661 100644 (file)
@@ -216,7 +216,7 @@ FrameLoader::FrameLoader(Frame& frame, FrameLoaderClient& client)
     , m_client(client)
     , m_policyChecker(adoptPtr(new PolicyChecker(frame)))
     , m_history(adoptPtr(new HistoryController(frame)))
-    , m_notifer(&frame)
+    , m_notifier(frame)
     , m_subframeLoader(adoptPtr(new SubframeLoader(frame)))
     , m_icon(adoptPtr(new IconController(frame)))
     , m_mixedContentChecker(&frame)
@@ -1445,7 +1445,7 @@ bool FrameLoader::willLoadMediaElementURL(URL& url)
     unsigned long identifier;
     ResourceError error;
     requestFromDelegate(request, identifier, error);
-    notifier()->sendRemainingDelegateMessages(m_documentLoader.get(), identifier, request, ResourceResponse(url, String(), -1, String(), String()), 0, -1, -1, error);
+    notifier().sendRemainingDelegateMessages(m_documentLoader.get(), identifier, request, ResourceResponse(url, String(), -1, String(), String()), 0, -1, -1, error);
 
     url = request.url();
 
@@ -1781,7 +1781,7 @@ void FrameLoader::commitProvisionalLoad()
             // FIXME: If we get a resource with more than 2B bytes, this code won't do the right thing.
             // However, with today's computers and networking speeds, this won't happen in practice.
             // Could be an issue with a giant local file.
-            notifier()->sendRemainingDelegateMessages(m_documentLoader.get(), identifier, request, response, 0, static_cast<int>(response.expectedContentLength()), 0, error);
+            notifier().sendRemainingDelegateMessages(m_documentLoader.get(), identifier, request, response, 0, static_cast<int>(response.expectedContentLength()), 0, error);
         }
 
         // FIXME: Why only this frame and not parent frames?
@@ -2594,7 +2594,7 @@ unsigned long FrameLoader::loadResourceSynchronously(const ResourceRequest& requ
             documentLoader()->applicationCacheHost()->maybeLoadFallbackSynchronously(newRequest, error, response, data);
         }
     }
-    notifier()->sendRemainingDelegateMessages(m_documentLoader.get(), identifier, request, response, data.data(), data.size(), -1, error);
+    notifier().sendRemainingDelegateMessages(m_documentLoader.get(), identifier, request, response, data.data(), data.size(), -1, error);
     return identifier;
 }
 
@@ -2916,11 +2916,11 @@ void FrameLoader::requestFromDelegate(ResourceRequest& request, unsigned long& i
     identifier = 0;
     if (Page* page = m_frame.page()) {
         identifier = page->progress().createUniqueIdentifier();
-        notifier()->assignIdentifierToInitialRequest(identifier, m_documentLoader.get(), request);
+        notifier().assignIdentifierToInitialRequest(identifier, m_documentLoader.get(), request);
     }
 
     ResourceRequest newRequest(request);
-    notifier()->dispatchWillSendRequest(m_documentLoader.get(), identifier, newRequest, ResourceResponse());
+    notifier().dispatchWillSendRequest(m_documentLoader.get(), identifier, newRequest, ResourceResponse());
 
     if (newRequest.isNull())
         error = cancelledError(request);
@@ -2962,7 +2962,7 @@ void FrameLoader::loadedResourceFromMemoryCache(CachedResource* resource, Resour
     ResourceError error;
     requestFromDelegate(newRequest, identifier, error);
     InspectorInstrumentation::markResourceAsCached(page, identifier);
-    notifier()->sendRemainingDelegateMessages(m_documentLoader.get(), identifier, newRequest, resource->response(), 0, resource->encodedSize(), 0, error);
+    notifier().sendRemainingDelegateMessages(m_documentLoader.get(), identifier, newRequest, resource->response(), 0, resource->encodedSize(), 0, error);
 }
 
 void FrameLoader::applyUserAgent(ResourceRequest& request)
index 2ef8124..6e95f77 100644 (file)
@@ -94,7 +94,7 @@ public:
 
     PolicyChecker& policyChecker() const { return *m_policyChecker; }
     HistoryController& history() const { return *m_history; }
-    ResourceLoadNotifier* notifier() const { return &m_notifer; }
+    ResourceLoadNotifier& notifier() const { return m_notifier; }
     SubframeLoader& subframeLoader() const { return *m_subframeLoader; }
     IconController& icon() const { return *m_icon; }
     MixedContentChecker& mixedContentChecker() const { return m_mixedContentChecker; }
@@ -387,7 +387,7 @@ private:
     // Some of these could be lazily created for memory savings on devices.
     const OwnPtr<PolicyChecker> m_policyChecker;
     const OwnPtr<HistoryController> m_history;
-    mutable ResourceLoadNotifier m_notifer;
+    mutable ResourceLoadNotifier m_notifier;
     const OwnPtr<SubframeLoader> m_subframeLoader;
     mutable FrameLoaderStateMachine m_stateMachine;
     const OwnPtr<IconController> m_icon;
index 2cc1085..8368be0 100644 (file)
 
 namespace WebCore {
 
-ResourceLoadNotifier::ResourceLoadNotifier(Frame* frame)
+ResourceLoadNotifier::ResourceLoadNotifier(Frame& frame)
     : m_frame(frame)
 {
 }
 
 void ResourceLoadNotifier::didReceiveAuthenticationChallenge(ResourceLoader* loader, const AuthenticationChallenge& currentWebChallenge)
 {
-    m_frame->loader().client().dispatchDidReceiveAuthenticationChallenge(loader->documentLoader(), loader->identifier(), currentWebChallenge);
+    m_frame.loader().client().dispatchDidReceiveAuthenticationChallenge(loader->documentLoader(), loader->identifier(), currentWebChallenge);
 }
 
 void ResourceLoadNotifier::didCancelAuthenticationChallenge(ResourceLoader* loader, const AuthenticationChallenge& currentWebChallenge)
 {
-    m_frame->loader().client().dispatchDidCancelAuthenticationChallenge(loader->documentLoader(), loader->identifier(), currentWebChallenge);
+    m_frame.loader().client().dispatchDidCancelAuthenticationChallenge(loader->documentLoader(), loader->identifier(), currentWebChallenge);
 }
 
 void ResourceLoadNotifier::willSendRequest(ResourceLoader* loader, ResourceRequest& clientRequest, const ResourceResponse& redirectResponse)
 {
-    m_frame->loader().applyUserAgent(clientRequest);
+    m_frame.loader().applyUserAgent(clientRequest);
 
     dispatchWillSendRequest(loader->documentLoader(), loader->identifier(), clientRequest, redirectResponse);
 }
@@ -68,7 +68,7 @@ void ResourceLoadNotifier::didReceiveResponse(ResourceLoader* loader, const Reso
 {
     loader->documentLoader()->addResponse(r);
 
-    if (Page* page = m_frame->page())
+    if (Page* page = m_frame.page())
         page->progress().incrementProgress(loader->identifier(), r);
 
     dispatchDidReceiveResponse(loader->documentLoader(), loader->identifier(), r, loader);
@@ -76,7 +76,7 @@ void ResourceLoadNotifier::didReceiveResponse(ResourceLoader* loader, const Reso
 
 void ResourceLoadNotifier::didReceiveData(ResourceLoader* loader, const char* data, int dataLength, int encodedDataLength)
 {
-    if (Page* page = m_frame->page())
+    if (Page* page = m_frame.page())
         page->progress().incrementProgress(loader->identifier(), data, dataLength);
 
     dispatchDidReceiveData(loader->documentLoader(), loader->identifier(), data, dataLength, encodedDataLength);
@@ -84,39 +84,39 @@ void ResourceLoadNotifier::didReceiveData(ResourceLoader* loader, const char* da
 
 void ResourceLoadNotifier::didFinishLoad(ResourceLoader* loader, double finishTime)
 {    
-    if (Page* page = m_frame->page())
+    if (Page* page = m_frame.page())
         page->progress().completeProgress(loader->identifier());
     dispatchDidFinishLoading(loader->documentLoader(), loader->identifier(), finishTime);
 }
 
 void ResourceLoadNotifier::didFailToLoad(ResourceLoader* loader, const ResourceError& error)
 {
-    if (Page* page = m_frame->page())
+    if (Page* page = m_frame.page())
         page->progress().completeProgress(loader->identifier());
 
     if (!error.isNull())
-        m_frame->loader().client().dispatchDidFailLoading(loader->documentLoader(), loader->identifier(), error);
+        m_frame.loader().client().dispatchDidFailLoading(loader->documentLoader(), loader->identifier(), error);
 
-    InspectorInstrumentation::didFailLoading(m_frame, loader->documentLoader(), loader->identifier(), error);
+    InspectorInstrumentation::didFailLoading(&m_frame, loader->documentLoader(), loader->identifier(), error);
 }
 
 void ResourceLoadNotifier::assignIdentifierToInitialRequest(unsigned long identifier, DocumentLoader* loader, const ResourceRequest& request)
 {
-    m_frame->loader().client().assignIdentifierToInitialRequest(identifier, loader, request);
+    m_frame.loader().client().assignIdentifierToInitialRequest(identifier, loader, request);
 }
 
 void ResourceLoadNotifier::dispatchWillSendRequest(DocumentLoader* loader, unsigned long identifier, ResourceRequest& request, const ResourceResponse& redirectResponse)
 {
     String oldRequestURL = request.url().string();
-    m_frame->loader().documentLoader()->didTellClientAboutLoad(request.url());
+    m_frame.loader().documentLoader()->didTellClientAboutLoad(request.url());
 
-    m_frame->loader().client().dispatchWillSendRequest(loader, identifier, request, redirectResponse);
+    m_frame.loader().client().dispatchWillSendRequest(loader, identifier, request, redirectResponse);
 
     // If the URL changed, then we want to put that new URL in the "did tell client" set too.
     if (!request.isNull() && oldRequestURL != request.url().string())
-        m_frame->loader().documentLoader()->didTellClientAboutLoad(request.url());
+        m_frame.loader().documentLoader()->didTellClientAboutLoad(request.url());
 
-    InspectorInstrumentation::willSendRequest(m_frame, identifier, loader, request, redirectResponse);
+    InspectorInstrumentation::willSendRequest(&m_frame, identifier, loader, request, redirectResponse);
 
     // Report WebTiming for all frames.
     if (loader && !request.isNull() && request.url() == loader->requestURL())
@@ -129,30 +129,30 @@ void ResourceLoadNotifier::dispatchWillSendRequest(DocumentLoader* loader, unsig
 
 void ResourceLoadNotifier::dispatchDidReceiveResponse(DocumentLoader* loader, unsigned long identifier, const ResourceResponse& r, ResourceLoader* resourceLoader)
 {
-    InspectorInstrumentationCookie cookie = InspectorInstrumentation::willReceiveResourceResponse(m_frame, identifier, r);
-    m_frame->loader().client().dispatchDidReceiveResponse(loader, identifier, r);
+    InspectorInstrumentationCookie cookie = InspectorInstrumentation::willReceiveResourceResponse(&m_frame, identifier, r);
+    m_frame.loader().client().dispatchDidReceiveResponse(loader, identifier, r);
     InspectorInstrumentation::didReceiveResourceResponse(cookie, identifier, loader, r, resourceLoader);
 }
 
 void ResourceLoadNotifier::dispatchDidReceiveData(DocumentLoader* loader, unsigned long identifier, const char* data, int dataLength, int encodedDataLength)
 {
-    m_frame->loader().client().dispatchDidReceiveContentLength(loader, identifier, dataLength);
+    m_frame.loader().client().dispatchDidReceiveContentLength(loader, identifier, dataLength);
 
-    InspectorInstrumentation::didReceiveData(m_frame, identifier, data, dataLength, encodedDataLength);
+    InspectorInstrumentation::didReceiveData(&m_frame, identifier, data, dataLength, encodedDataLength);
 }
 
 void ResourceLoadNotifier::dispatchDidFinishLoading(DocumentLoader* loader, unsigned long identifier, double finishTime)
 {
-    m_frame->loader().client().dispatchDidFinishLoading(loader, identifier);
+    m_frame.loader().client().dispatchDidFinishLoading(loader, identifier);
 
-    InspectorInstrumentation::didFinishLoading(m_frame, loader, identifier, finishTime);
+    InspectorInstrumentation::didFinishLoading(&m_frame, loader, identifier, finishTime);
 }
 
 void ResourceLoadNotifier::dispatchDidFailLoading(DocumentLoader* loader, unsigned long identifier, const ResourceError& error)
 {
-    m_frame->loader().client().dispatchDidFailLoading(loader, identifier, error);
+    m_frame.loader().client().dispatchDidFailLoading(loader, identifier, error);
 
-    InspectorInstrumentation::didFailLoading(m_frame, loader, identifier, error);
+    InspectorInstrumentation::didFailLoading(&m_frame, loader, identifier, error);
 }
 
 void ResourceLoadNotifier::sendRemainingDelegateMessages(DocumentLoader* loader, unsigned long identifier, const ResourceRequest& request, const ResourceResponse& response, const char* data, int dataLength, int encodedDataLength, const ResourceError& error)
index 4044111..ffb2895 100644 (file)
@@ -46,7 +46,7 @@ class ResourceRequest;
 class ResourceLoadNotifier {
     WTF_MAKE_NONCOPYABLE(ResourceLoadNotifier);
 public:
-    ResourceLoadNotifier(Frame*);
+    explicit ResourceLoadNotifier(Frame&);
 
     void didReceiveAuthenticationChallenge(ResourceLoader*, const AuthenticationChallenge&);
     void didCancelAuthenticationChallenge(ResourceLoader*, const AuthenticationChallenge&);
@@ -67,7 +67,7 @@ public:
     void sendRemainingDelegateMessages(DocumentLoader*, unsigned long identifier, const ResourceRequest&, const ResourceResponse&, const char* data, int dataLength, int encodedDataLength, const ResourceError&);
 
 private:
-    Frame* m_frame;
+    Frame& m_frame;
 };
 
 } // namespace WebCore
index 2577523..f628d49 100644 (file)
@@ -238,9 +238,9 @@ void ResourceLoader::willSendRequest(ResourceRequest& request, const ResourceRes
 
     if (m_options.sendLoadCallbacks == SendCallbacks) {
         if (createdResourceIdentifier)
-            frameLoader()->notifier()->assignIdentifierToInitialRequest(m_identifier, documentLoader(), request);
+            frameLoader()->notifier().assignIdentifierToInitialRequest(m_identifier, documentLoader(), request);
 
-        frameLoader()->notifier()->willSendRequest(this, request, redirectResponse);
+        frameLoader()->notifier().willSendRequest(this, request, redirectResponse);
     }
 #if ENABLE(INSPECTOR)
     else
@@ -274,7 +274,7 @@ void ResourceLoader::didReceiveResponse(const ResourceResponse& r)
         data->removeGeneratedFilesIfNeeded();
         
     if (m_options.sendLoadCallbacks == SendCallbacks)
-        frameLoader()->notifier()->didReceiveResponse(this, m_response);
+        frameLoader()->notifier().didReceiveResponse(this, m_response);
 }
 
 void ResourceLoader::didReceiveData(const char* data, int length, long long encodedDataLength, DataPayloadType dataPayloadType)
@@ -309,7 +309,7 @@ void ResourceLoader::didReceiveDataOrBuffer(const char* data, int length, PassRe
     // However, with today's computers and networking speeds, this won't happen in practice.
     // Could be an issue with a giant local file.
     if (m_options.sendLoadCallbacks == SendCallbacks && m_frame)
-        frameLoader()->notifier()->didReceiveData(this, buffer ? buffer->data() : data, buffer ? buffer->size() : length, static_cast<int>(encodedDataLength));
+        frameLoader()->notifier().didReceiveData(this, buffer ? buffer->data() : data, buffer ? buffer->size() : length, static_cast<int>(encodedDataLength));
 }
 
 void ResourceLoader::willStopBufferingData(const char* data, int length)
@@ -344,7 +344,7 @@ void ResourceLoader::didFinishLoadingOnePart(double finishTime)
         return;
     m_notifiedLoadComplete = true;
     if (m_options.sendLoadCallbacks == SendCallbacks)
-        frameLoader()->notifier()->didFinishLoad(this, finishTime);
+        frameLoader()->notifier().didFinishLoad(this, finishTime);
 }
 
 void ResourceLoader::didFail(const ResourceError& error)
@@ -370,7 +370,7 @@ void ResourceLoader::cleanupForError(const ResourceError& error)
         return;
     m_notifiedLoadComplete = true;
     if (m_options.sendLoadCallbacks == SendCallbacks && m_identifier)
-        frameLoader()->notifier()->didFailToLoad(this, error);
+        frameLoader()->notifier().didFailToLoad(this, error);
 }
 
 void ResourceLoader::didChangePriority(ResourceLoadPriority loadPriority)
@@ -523,7 +523,7 @@ void ResourceLoader::didReceiveAuthenticationChallenge(const AuthenticationChall
 
     if (m_options.allowCredentials == AllowStoredCredentials) {
         if (m_options.clientCredentialPolicy == AskClientForAllCredentials || (m_options.clientCredentialPolicy == DoNotAskClientForCrossOriginCredentials && m_frame->document()->securityOrigin()->canRequest(originalRequest().url()))) {
-            frameLoader()->notifier()->didReceiveAuthenticationChallenge(this, challenge);
+            frameLoader()->notifier().didReceiveAuthenticationChallenge(this, challenge);
             return;
         }
     }
@@ -542,7 +542,7 @@ void ResourceLoader::didCancelAuthenticationChallenge(const AuthenticationChalle
     // Protect this in this delegate method since the additional processing can do
     // anything including possibly derefing this; one example of this is Radar 3266216.
     Ref<ResourceLoader> protect(*this);
-    frameLoader()->notifier()->didCancelAuthenticationChallenge(this, challenge);
+    frameLoader()->notifier().didCancelAuthenticationChallenge(this, challenge);
 }
 
 #if USE(PROTECTION_SPACE_AUTH_CALLBACK)
index e2ec03a..24d8950 100644 (file)
@@ -91,7 +91,7 @@ void ResourceLoader::didReceiveDataArray(CFArrayRef dataArray)
         // However, with today's computers and networking speeds, this won't happen in practice.
         // Could be an issue with a giant local file.
         if (m_options.sendLoadCallbacks == SendCallbacks && m_frame)
-            frameLoader()->notifier()->didReceiveData(this, reinterpret_cast<const char*>(CFDataGetBytePtr(data)), dataLen, dataLen);
+            frameLoader()->notifier().didReceiveData(this, reinterpret_cast<const char*>(CFDataGetBytePtr(data)), dataLen, dataLen);
     }
 }