Prefer document->addConsoleMessage to document->domWindow->console->addMessage.
authormkwst@chromium.org <mkwst@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 Oct 2012 18:47:31 +0000 (18:47 +0000)
committermkwst@chromium.org <mkwst@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 Oct 2012 18:47:31 +0000 (18:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=100850

Reviewed by Adam Barth.

For historical reasons, a few places in WebCore talk to Console directly
via 'document()->domWindow()->console()->addMessage(...)'. This is more
safely wrapped by calling 'addConsoleMessage' on the Document itself.

No visible functionality should change; we'll simply avoid potential
null dereferences in the future.

Source/WebCore:

* html/HTMLFormElement.cpp:
(WebCore::HTMLFormElement::validateInteractively):
* html/canvas/WebGLRenderingContext.cpp:
(WebCore):
(WebCore::WebGLRenderingContext::printWarningToConsole):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::reportLocalLoadFailed):
* loader/MixedContentChecker.cpp:
(WebCore::MixedContentChecker::logWarning):
* loader/appcache/ApplicationCacheGroup.cpp:
(WebCore::ApplicationCacheGroup::abort):
(WebCore::ApplicationCacheGroup::didReceiveResponse):
(WebCore::ApplicationCacheGroup::didFinishLoading):
(WebCore::ApplicationCacheGroup::didFail):
(WebCore::ApplicationCacheGroup::didReceiveManifestResponse):
(WebCore::ApplicationCacheGroup::didFinishLoadingManifest):
(WebCore::ApplicationCacheGroup::checkIfLoadIsComplete):
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::printAccessDeniedMessage):

Source/WebKit/chromium:

* src/WebFrameImpl.cpp:
(WebKit::WebFrameImpl::addMessageToConsole):

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

Source/WebCore/ChangeLog
Source/WebCore/dom/ScriptExecutionContext.h
Source/WebCore/html/HTMLFormElement.cpp
Source/WebCore/html/canvas/WebGLRenderingContext.cpp
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/MixedContentChecker.cpp
Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp
Source/WebCore/loader/cache/CachedResourceLoader.cpp
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/src/WebFrameImpl.cpp

index 130bec9..e970328 100644 (file)
@@ -1,3 +1,37 @@
+2012-10-31  Mike West  <mkwst@chromium.org>
+
+        Prefer document->addConsoleMessage to document->domWindow->console->addMessage.
+        https://bugs.webkit.org/show_bug.cgi?id=100850
+
+        Reviewed by Adam Barth.
+
+        For historical reasons, a few places in WebCore talk to Console directly
+        via 'document()->domWindow()->console()->addMessage(...)'. This is more
+        safely wrapped by calling 'addConsoleMessage' on the Document itself.
+
+        No visible functionality should change; we'll simply avoid potential
+        null dereferences in the future.
+
+        * html/HTMLFormElement.cpp:
+        (WebCore::HTMLFormElement::validateInteractively):
+        * html/canvas/WebGLRenderingContext.cpp:
+        (WebCore):
+        (WebCore::WebGLRenderingContext::printWarningToConsole):
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::reportLocalLoadFailed):
+        * loader/MixedContentChecker.cpp:
+        (WebCore::MixedContentChecker::logWarning):
+        * loader/appcache/ApplicationCacheGroup.cpp:
+        (WebCore::ApplicationCacheGroup::abort):
+        (WebCore::ApplicationCacheGroup::didReceiveResponse):
+        (WebCore::ApplicationCacheGroup::didFinishLoading):
+        (WebCore::ApplicationCacheGroup::didFail):
+        (WebCore::ApplicationCacheGroup::didReceiveManifestResponse):
+        (WebCore::ApplicationCacheGroup::didFinishLoadingManifest):
+        (WebCore::ApplicationCacheGroup::checkIfLoadIsComplete):
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::printAccessDeniedMessage):
+
 2012-10-31  Pavel Feldman  <pfeldman@chromium.org>
 
         Web Inspector: frame chooser does not work on subsequent inspector open.
index 34248f4..b8dca29 100644 (file)
@@ -31,6 +31,7 @@
 #include "ActiveDOMObject.h"
 #include "ConsoleTypes.h"
 #include "KURL.h"
+#include "ScriptCallStack.h"
 #include "SecurityContext.h"
 #include "Supplementable.h"
 #include <wtf/Forward.h>
@@ -53,7 +54,6 @@ class EventListener;
 class EventQueue;
 class EventTarget;
 class MessagePort;
-class ScriptCallStack;
 
 #if ENABLE(BLOB)
 class PublicURLManager;
index 0a25141..dde5ffa 100644 (file)
@@ -26,7 +26,6 @@
 #include "HTMLFormElement.h"
 
 #include "Attribute.h"
-#include "Console.h"
 #include "DOMFormData.h"
 #include "DOMWindow.h"
 #include "Document.h"
@@ -257,7 +256,7 @@ bool HTMLFormElement::validateInteractively(Event* event)
                 continue;
             String message("An invalid form control with name='%name' is not focusable.");
             message.replace("%name", unhandledAssociatedElement->name());
-            document()->domWindow()->console()->addMessage(HTMLMessageSource, LogMessageType, ErrorMessageLevel, message, document()->url().string());
+            document()->addConsoleMessage(HTMLMessageSource, LogMessageType, ErrorMessageLevel, message, document()->url().string());
         }
     }
     return false;
index 8874ea9..c70f40f 100644 (file)
@@ -31,7 +31,6 @@
 
 #include "CachedImage.h"
 #include "CheckedInt.h"
-#include "Console.h"
 #include "DOMWindow.h"
 #include "EXTTextureFilterAnisotropic.h"
 #include "ExceptionCode.h"
@@ -5207,20 +5206,10 @@ void WebGLRenderingContext::printWarningToConsole(const String& message)
 {
     if (!canvas())
         return;
-    // FIXME: This giant cascade of null checks seems a bit paranoid.
     Document* document = canvas()->document();
     if (!document)
         return;
-    Frame* frame = document->frame();
-    if (!frame)
-        return;
-    DOMWindow* window = document->domWindow();
-    if (!window)
-        return;
-    Console* console = window->console();
-    if (!console)
-        return;
-    console->addMessage(HTMLMessageSource, LogMessageType, WarningMessageLevel, message, document->url().string());
+    document->addConsoleMessage(HTMLMessageSource, LogMessageType, WarningMessageLevel, message, document->url().string());
 }
 
 bool WebGLRenderingContext::validateFramebufferFuncParameters(const char* functionName, GC3Denum target, GC3Denum attachment)
index e14de0b..c32a5a4 100644 (file)
@@ -1392,7 +1392,7 @@ void FrameLoader::reportLocalLoadFailed(Frame* frame, const String& url)
     if (!frame)
         return;
 
-    frame->document()->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Not allowed to load local resource: " + url);
+    frame->document()->addConsoleMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Not allowed to load local resource: " + url);
 }
 
 const ResourceRequest& FrameLoader::initialRequest() const
index 71fd2b3..ae8a89a 100644 (file)
@@ -98,11 +98,10 @@ bool MixedContentChecker::canRunInsecureContent(SecurityOrigin* securityOrigin,
 
 void MixedContentChecker::logWarning(bool allowed, const String& action, const KURL& target) const
 {
-    Console* console = m_frame->document()->domWindow()->console();
     // FIXME: Why does this message not have a source URL or a line number? webkit.org/b/97979
     String message = String::format("%sThe page at %s %s insecure content from %s.\n",
         (allowed ? "" : "[blocked] "), asUTF8(m_frame->document()->url()).data(), action.utf8().data(), asUTF8(target).data());
-    console->addMessage(HTMLMessageSource, LogMessageType, WarningMessageLevel, message);
+    m_frame->document()->addConsoleMessage(HTMLMessageSource, LogMessageType, WarningMessageLevel, message);
 }
 
 } // namespace WebCore
index 4de3a49..118ee1a 100644 (file)
@@ -474,7 +474,7 @@ void ApplicationCacheGroup::abort(Frame* frame)
     if (m_completionType != None)
         return;
 
-    frame->document()->domWindow()->console()->addMessage(OtherMessageSource, LogMessageType, TipMessageLevel, "Application Cache download process was aborted.");
+    frame->document()->addConsoleMessage(OtherMessageSource, LogMessageType, TipMessageLevel, "Application Cache download process was aborted.");
     cacheUpdateFailed();
 }
 
@@ -568,7 +568,7 @@ void ApplicationCacheGroup::didReceiveResponse(ResourceHandle* handle, const Res
 
     if (response.httpStatusCode() / 100 != 2 || response.url() != m_currentHandle->firstRequest().url()) {
         if ((type & ApplicationCacheResource::Explicit) || (type & ApplicationCacheResource::Fallback)) {
-            m_frame->document()->domWindow()->console()->addMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, "Application Cache update failed, because " + m_currentHandle->firstRequest().url().string() + 
+            m_frame->document()->addConsoleMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, "Application Cache update failed, because " + m_currentHandle->firstRequest().url().string() + 
                 ((response.httpStatusCode() / 100 != 2) ? " could not be fetched." : " was redirected."));
             // Note that cacheUpdateFailed() can cause the cache group to be deleted.
             cacheUpdateFailed();
@@ -645,7 +645,7 @@ void ApplicationCacheGroup::didFinishLoading(ResourceHandle* handle, double fini
     // FIXME: Should we break earlier and prevent redownloading on later page loads?
     if (m_originQuotaExceededPreviously && m_availableSpaceInQuota < m_cacheBeingUpdated->estimatedSizeInStorage()) {
         m_currentResource = 0;
-        m_frame->document()->domWindow()->console()->addMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, "Application Cache update failed, because size quota was exceeded.");
+        m_frame->document()->addConsoleMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, "Application Cache update failed, because size quota was exceeded.");
         cacheUpdateFailed();
         return;
     }
@@ -678,7 +678,7 @@ void ApplicationCacheGroup::didFail(ResourceHandle* handle, const ResourceError&
     m_pendingEntries.remove(url);
 
     if ((type & ApplicationCacheResource::Explicit) || (type & ApplicationCacheResource::Fallback)) {
-        m_frame->document()->domWindow()->console()->addMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, "Application Cache update failed, because " + url.string() + " could not be fetched.");
+        m_frame->document()->addConsoleMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, "Application Cache update failed, because " + url.string() + " could not be fetched.");
         // Note that cacheUpdateFailed() can cause the cache group to be deleted.
         cacheUpdateFailed();
     } else {
@@ -707,13 +707,13 @@ void ApplicationCacheGroup::didReceiveManifestResponse(const ResourceResponse& r
         return;
 
     if (response.httpStatusCode() / 100 != 2) {
-        m_frame->document()->domWindow()->console()->addMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, "Application Cache manifest could not be fetched.");
+        m_frame->document()->addConsoleMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, "Application Cache manifest could not be fetched.");
         cacheUpdateFailed();
         return;
     }
 
     if (response.url() != m_manifestHandle->firstRequest().url()) {
-        m_frame->document()->domWindow()->console()->addMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, "Application Cache manifest could not be fetched, because a redirection was attempted.");
+        m_frame->document()->addConsoleMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, "Application Cache manifest could not be fetched, because a redirection was attempted.");
         cacheUpdateFailed();
         return;
     }
@@ -733,7 +733,7 @@ void ApplicationCacheGroup::didFinishLoadingManifest()
 
     if (!isUpgradeAttempt && !m_manifestResource) {
         // The server returned 304 Not Modified even though we didn't send a conditional request.
-        m_frame->document()->domWindow()->console()->addMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, "Application Cache manifest could not be fetched because of an unexpected 304 Not Modified server response.");
+        m_frame->document()->addConsoleMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, "Application Cache manifest could not be fetched because of an unexpected 304 Not Modified server response.");
         cacheUpdateFailed();
         return;
     }
@@ -759,7 +759,7 @@ void ApplicationCacheGroup::didFinishLoadingManifest()
     Manifest manifest;
     if (!parseManifest(m_manifestURL, m_manifestResource->data()->data(), m_manifestResource->data()->size(), manifest)) {
         // At the time of this writing, lack of "CACHE MANIFEST" signature is the only reason for parseManifest to fail.
-        m_frame->document()->domWindow()->console()->addMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, "Application Cache manifest could not be parsed. Does it start with CACHE MANIFEST?");
+        m_frame->document()->addConsoleMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, "Application Cache manifest could not be parsed. Does it start with CACHE MANIFEST?");
         cacheUpdateFailed();
         return;
     }
@@ -948,7 +948,7 @@ void ApplicationCacheGroup::checkIfLoadIsComplete()
                 // We ran out of space for this origin. Fall down to the normal error handling
                 // after recording this state.
                 m_originQuotaExceededPreviously = true;
-                m_frame->document()->domWindow()->console()->addMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, "Application Cache update failed, because size quota was exceeded.");
+                m_frame->document()->addConsoleMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, "Application Cache update failed, because size quota was exceeded.");
             }
 
             if (failureReason == ApplicationCacheStorage::TotalQuotaReached && !m_calledReachedMaxAppCacheSize) {
index 89c83e0..769fd8d 100644 (file)
@@ -629,7 +629,7 @@ void CachedResourceLoader::printAccessDeniedMessage(const KURL& url) const
         message = "Unsafe attempt to load URL " + url.string() + " from frame with URL " + m_document->url().string() + ". Domains, protocols and ports must match.\n";
 
     // FIXME: provide line number and source URL.
-    frame()->document()->domWindow()->console()->addMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, message);
+    frame()->document()->addConsoleMessage(OtherMessageSource, LogMessageType, ErrorMessageLevel, message);
 }
 
 void CachedResourceLoader::setAutoLoadImages(bool enable)
index 08b5d0c..c378c32 100644 (file)
@@ -1,3 +1,20 @@
+2012-10-31  Mike West  <mkwst@chromium.org>
+
+        Prefer document->addConsoleMessage to document->domWindow->console->addMessage.
+        https://bugs.webkit.org/show_bug.cgi?id=100850
+
+        Reviewed by Adam Barth.
+
+        For historical reasons, a few places in WebCore talk to Console directly
+        via 'document()->domWindow()->console()->addMessage(...)'. This is more
+        safely wrapped by calling 'addConsoleMessage' on the Document itself.
+
+        No visible functionality should change; we'll simply avoid potential
+        null dereferences in the future.
+
+        * src/WebFrameImpl.cpp:
+        (WebKit::WebFrameImpl::addMessageToConsole):
+
 2012-10-31  Kent Tamura  <tkent@chromium.org>
 
         Remove code to hide/reshow text caret for PagePopups
index e11499e..1936476 100644 (file)
@@ -859,7 +859,7 @@ void WebFrameImpl::addMessageToConsole(const WebConsoleMessage& message)
         return;
     }
 
-    frame()->document()->domWindow()->console()->addMessage(OtherMessageSource, LogMessageType, webCoreMessageLevel, message.text);
+    frame()->document()->addConsoleMessage(OtherMessageSource, LogMessageType, webCoreMessageLevel, message.text);
 }
 
 void WebFrameImpl::collectGarbage()