SubresourceLoader::didFail() should only log message if state is Initialized
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 23 Jun 2018 01:22:04 +0000 (01:22 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 23 Jun 2018 01:22:04 +0000 (01:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185124

Patch by Woodrow Wang <woodrow_wang@apple.com> on 2018-06-22
Reviewed by Daniel Bates.

Functionality does not change. Moved console logging to be
after checking state of subresource loader. We only need to
log if the state is initialized. This is consistent with other
functions in the file. We can also remove a null check for the
frame pointer (m_frame). The superclass ResourceLoader constructor
takes an lvalue reference and initializes m_frame, ensuring
m_frame cannot be null. It is only set to null by
ResourceLoader::releaseResources(), which is only called after the
resource finishes loading or fails to load. Thus, in didFail(),
m_frame must be non-null when we're logging and up until the end of the function.

* loader/SubresourceLoader.cpp:
(WebCore::SubresourceLoader::didFail):

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

Source/WebCore/ChangeLog
Source/WebCore/loader/SubresourceLoader.cpp

index 0f463e8..56372f2 100644 (file)
@@ -1,3 +1,24 @@
+2018-06-22  Woodrow Wang  <woodrow_wang@apple.com>
+
+        SubresourceLoader::didFail() should only log message if state is Initialized
+        https://bugs.webkit.org/show_bug.cgi?id=185124
+
+        Reviewed by Daniel Bates.
+
+        Functionality does not change. Moved console logging to be 
+        after checking state of subresource loader. We only need to 
+        log if the state is initialized. This is consistent with other
+        functions in the file. We can also remove a null check for the 
+        frame pointer (m_frame). The superclass ResourceLoader constructor
+        takes an lvalue reference and initializes m_frame, ensuring
+        m_frame cannot be null. It is only set to null by 
+        ResourceLoader::releaseResources(), which is only called after the 
+        resource finishes loading or fails to load. Thus, in didFail(), 
+        m_frame must be non-null when we're logging and up until the end of the function.  
+
+        * loader/SubresourceLoader.cpp:
+        (WebCore::SubresourceLoader::didFail):
+
 2018-06-22  Timothy Hatcher  <timothy@apple.com>
 
         Corner of two scroll bars is white with dark mode enabled.
index a4a15d8..b035d0b 100644 (file)
@@ -648,8 +648,6 @@ void SubresourceLoader::didFinishLoading(const NetworkLoadMetrics& networkLoadMe
 
 void SubresourceLoader::didFail(const ResourceError& error)
 {
-    if (m_frame && m_frame->document() && error.isAccessControl())
-        m_frame->document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, error.localizedDescription());
 
 #if USE(QUICK_LOOK)
     if (auto previewLoader = m_previewLoader.get())
@@ -660,6 +658,9 @@ void SubresourceLoader::didFail(const ResourceError& error)
         return;
     ASSERT(!reachedTerminalState());
     LOG(ResourceLoading, "Failed to load '%s'.\n", m_resource->url().string().latin1().data());
+    if (m_frame->document() && error.isAccessControl())
+        m_frame->document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, error.localizedDescription());
+
 
     Ref<SubresourceLoader> protectedThis(*this);
     CachedResourceHandle<CachedResource> protectResource(m_resource);