Simplify DiagnosticLoggingClient call sites
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 21 Jan 2015 02:16:43 +0000 (02:16 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 21 Jan 2015 02:16:43 +0000 (02:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=140701

Reviewed by Andreas Kling.

Simplify DiagnosticLoggingClient call sites by:
- Moving the Settings::diagnosticLoggingEnabled() check inside the
  MainFrame::diagnosticLoggingClient() getter.
- Having MainFrame::diagnosticLoggingClient() return a reference
  instead of a pointer (returning a dummy client if necessary).

Otherwise, each call site needs to both check the setting and do a
null-check on the client which is a bit annoying.

* history/PageCache.cpp:
(WebCore::logPageCacheFailureDiagnosticMessage):
(WebCore::logCanCacheFrameDecision):
(WebCore::logCanCachePageDecision):
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::parseAttribute):
* loader/EmptyClients.h:
* loader/FrameLoader.cpp:
(WebCore::logNavigation):
(WebCore::FrameLoader::checkLoadCompleteForThisFrame):
* loader/SubframeLoader.cpp:
(WebCore::logPluginRequest):
* loader/SubresourceLoader.cpp:
(WebCore::logResourceLoaded):
* page/DiagnosticLoggingClient.h:
(WebCore::DiagnosticLoggingClient::logDiagnosticMessage): Deleted.
(WebCore::DiagnosticLoggingClient::logDiagnosticMessageWithResult): Deleted.
(WebCore::DiagnosticLoggingClient::logDiagnosticMessageWithValue): Deleted.
* page/MainFrame.cpp:
(WebCore::MainFrame::diagnosticLoggingClient):
* page/MainFrame.h:

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

Source/WebCore/ChangeLog
Source/WebCore/history/PageCache.cpp
Source/WebCore/html/HTMLMediaElement.cpp
Source/WebCore/loader/EmptyClients.h
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/SubframeLoader.cpp
Source/WebCore/loader/SubresourceLoader.cpp
Source/WebCore/page/DiagnosticLoggingClient.h
Source/WebCore/page/MainFrame.cpp
Source/WebCore/page/MainFrame.h

index 36447ca..9d16216 100644 (file)
@@ -1,3 +1,41 @@
+2015-01-20  Chris Dumez  <cdumez@apple.com>
+
+        Simplify DiagnosticLoggingClient call sites
+        https://bugs.webkit.org/show_bug.cgi?id=140701
+
+        Reviewed by Andreas Kling.
+
+        Simplify DiagnosticLoggingClient call sites by:
+        - Moving the Settings::diagnosticLoggingEnabled() check inside the
+          MainFrame::diagnosticLoggingClient() getter.
+        - Having MainFrame::diagnosticLoggingClient() return a reference
+          instead of a pointer (returning a dummy client if necessary).
+
+        Otherwise, each call site needs to both check the setting and do a
+        null-check on the client which is a bit annoying.
+
+        * history/PageCache.cpp:
+        (WebCore::logPageCacheFailureDiagnosticMessage):
+        (WebCore::logCanCacheFrameDecision):
+        (WebCore::logCanCachePageDecision):
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::parseAttribute):
+        * loader/EmptyClients.h:
+        * loader/FrameLoader.cpp:
+        (WebCore::logNavigation):
+        (WebCore::FrameLoader::checkLoadCompleteForThisFrame):
+        * loader/SubframeLoader.cpp:
+        (WebCore::logPluginRequest):
+        * loader/SubresourceLoader.cpp:
+        (WebCore::logResourceLoaded):
+        * page/DiagnosticLoggingClient.h:
+        (WebCore::DiagnosticLoggingClient::logDiagnosticMessage): Deleted.
+        (WebCore::DiagnosticLoggingClient::logDiagnosticMessageWithResult): Deleted.
+        (WebCore::DiagnosticLoggingClient::logDiagnosticMessageWithValue): Deleted.
+        * page/MainFrame.cpp:
+        (WebCore::MainFrame::diagnosticLoggingClient):
+        * page/MainFrame.h:
+
 2015-01-20  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Should show dynamic specificity values
 2015-01-20  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Should show dynamic specificity values
index cf65c0f..31c9861 100644 (file)
@@ -96,10 +96,9 @@ enum ReasonFrameCannotBeInPageCache {
 };
 COMPILE_ASSERT(NumberOfReasonsFramesCannotBeInPageCache <= sizeof(unsigned)*8, ReasonFrameCannotBeInPageCacheDoesNotFitInBitmap);
 
 };
 COMPILE_ASSERT(NumberOfReasonsFramesCannotBeInPageCache <= sizeof(unsigned)*8, ReasonFrameCannotBeInPageCacheDoesNotFitInBitmap);
 
-static inline void logPageCacheFailureDiagnosticMessage(DiagnosticLoggingClient* client, const String& reason)
+static inline void logPageCacheFailureDiagnosticMessage(DiagnosticLoggingClient& client, const String& reason)
 {
 {
-    if (client)
-        client->logDiagnosticMessageWithValue(DiagnosticLoggingKeys::pageCacheKey(), DiagnosticLoggingKeys::failureKey(), reason);
+    client.logDiagnosticMessageWithValue(DiagnosticLoggingKeys::pageCacheKey(), DiagnosticLoggingKeys::failureKey(), reason);
 }
 
 static inline void logPageCacheFailureDiagnosticMessage(Page* page, const String& reason)
 }
 
 static inline void logPageCacheFailureDiagnosticMessage(Page* page, const String& reason)
@@ -107,12 +106,10 @@ static inline void logPageCacheFailureDiagnosticMessage(Page* page, const String
     if (!page)
         return;
 
     if (!page)
         return;
 
-    MainFrame& mainFrame = page->mainFrame();
-    if (mainFrame.settings().diagnosticLoggingEnabled())
-        logPageCacheFailureDiagnosticMessage(mainFrame.diagnosticLoggingClient(), reason);
+    logPageCacheFailureDiagnosticMessage(page->mainFrame().diagnosticLoggingClient(), reason);
 }
 
 }
 
-static unsigned logCanCacheFrameDecision(Frame& frame, DiagnosticLoggingClient* diagnosticLoggingClient, int indentLevel)
+static unsigned logCanCacheFrameDecision(Frame& frame, DiagnosticLoggingClient& diagnosticLoggingClient, int indentLevel)
 {
     PCLOG("+---");
     if (!frame.loader().documentLoader()) {
 {
     PCLOG("+---");
     if (!frame.loader().documentLoader()) {
@@ -241,7 +238,7 @@ static void logCanCachePageDecision(Page& page)
     
     unsigned rejectReasons = 0;
     MainFrame& mainFrame = page.mainFrame();
     
     unsigned rejectReasons = 0;
     MainFrame& mainFrame = page.mainFrame();
-    DiagnosticLoggingClient* diagnosticLoggingClient = mainFrame.settings().diagnosticLoggingEnabled() ? mainFrame.diagnosticLoggingClient() : nullptr;
+    DiagnosticLoggingClient& diagnosticLoggingClient = mainFrame.diagnosticLoggingClient();
     unsigned frameRejectReasons = logCanCacheFrameDecision(mainFrame, diagnosticLoggingClient, indentLevel + 1);
     if (frameRejectReasons)
         rejectReasons |= 1 << FrameCannotBeInPageCache;
     unsigned frameRejectReasons = logCanCacheFrameDecision(mainFrame, diagnosticLoggingClient, indentLevel + 1);
     if (frameRejectReasons)
         rejectReasons |= 1 << FrameCannotBeInPageCache;
@@ -291,8 +288,7 @@ static void logCanCachePageDecision(Page& page)
     else
         PCLOG(" Page CAN be cached\n--------");
 
     else
         PCLOG(" Page CAN be cached\n--------");
 
-    if (diagnosticLoggingClient)
-        diagnosticLoggingClient->logDiagnosticMessageWithResult(DiagnosticLoggingKeys::pageCacheKey(), emptyString(), rejectReasons ? DiagnosticLoggingResultFail : DiagnosticLoggingResultPass);
+    diagnosticLoggingClient.logDiagnosticMessageWithResult(DiagnosticLoggingKeys::pageCacheKey(), emptyString(), rejectReasons ? DiagnosticLoggingResultFail : DiagnosticLoggingResultPass);
 }
 
 PageCache* pageCache()
 }
 
 PageCache* pageCache()
index e4b8acd..1fe7b81 100644 (file)
@@ -1829,25 +1829,22 @@ void HTMLMediaElement::mediaPlayerNetworkStateChanged(MediaPlayer*)
 
 static void logMediaLoadRequest(Page* page, const String& mediaEngine, const String& errorMessage, bool succeeded)
 {
 
 static void logMediaLoadRequest(Page* page, const String& mediaEngine, const String& errorMessage, bool succeeded)
 {
-    if (!page || !page->settings().diagnosticLoggingEnabled())
-        return;
-
-    DiagnosticLoggingClient* diagnosticLoggingClient = page->mainFrame().diagnosticLoggingClient();
-    if (!diagnosticLoggingClient)
+    if (!page)
         return;
 
         return;
 
+    DiagnosticLoggingClient& diagnosticLoggingClient = page->mainFrame().diagnosticLoggingClient();
     if (!succeeded) {
     if (!succeeded) {
-        diagnosticLoggingClient->logDiagnosticMessageWithResult(DiagnosticLoggingKeys::mediaLoadingFailedKey(), errorMessage, DiagnosticLoggingResultFail);
+        diagnosticLoggingClient.logDiagnosticMessageWithResult(DiagnosticLoggingKeys::mediaLoadingFailedKey(), errorMessage, DiagnosticLoggingResultFail);
         return;
     }
 
         return;
     }
 
-    diagnosticLoggingClient->logDiagnosticMessage(DiagnosticLoggingKeys::mediaLoadedKey(), mediaEngine);
+    diagnosticLoggingClient.logDiagnosticMessage(DiagnosticLoggingKeys::mediaLoadedKey(), mediaEngine);
 
     if (!page->hasSeenAnyMediaEngine())
 
     if (!page->hasSeenAnyMediaEngine())
-        diagnosticLoggingClient->logDiagnosticMessage(DiagnosticLoggingKeys::pageContainsAtLeastOneMediaEngineKey(), emptyString());
+        diagnosticLoggingClient.logDiagnosticMessage(DiagnosticLoggingKeys::pageContainsAtLeastOneMediaEngineKey(), emptyString());
 
     if (!page->hasSeenMediaEngine(mediaEngine))
 
     if (!page->hasSeenMediaEngine(mediaEngine))
-        diagnosticLoggingClient->logDiagnosticMessage(DiagnosticLoggingKeys::pageContainsMediaEngineKey(), mediaEngine);
+        diagnosticLoggingClient.logDiagnosticMessage(DiagnosticLoggingKeys::pageContainsMediaEngineKey(), mediaEngine);
 
     page->sawMediaEngine(mediaEngine);
 }
 
     page->sawMediaEngine(mediaEngine);
 }
@@ -5786,16 +5783,8 @@ void HTMLMediaElement::mediaPlayerEngineFailedToLoad() const
     if (!m_player)
         return;
 
     if (!m_player)
         return;
 
-    Page* page = document().page();
-    if (!page || !page->settings().diagnosticLoggingEnabled())
-        return;
-
-    DiagnosticLoggingClient* diagnosticLoggingClient = page->mainFrame().diagnosticLoggingClient();
-    if (!diagnosticLoggingClient)
-        return;
-
-    String engine = m_player->engineDescription();
-    diagnosticLoggingClient->logDiagnosticMessageWithValue(DiagnosticLoggingKeys::engineFailedToLoadKey(), engine, String::number(m_player->platformErrorCode()));
+    if (Frame* frame = document().frame())
+        frame->mainFrame().diagnosticLoggingClient().logDiagnosticMessageWithValue(DiagnosticLoggingKeys::engineFailedToLoadKey(), m_player->engineDescription(), String::number(m_player->platformErrorCode()));
 }
 
 double HTMLMediaElement::mediaPlayerRequestedPlaybackRate() const
 }
 
 double HTMLMediaElement::mediaPlayerRequestedPlaybackRate() const
index 6a93f72..dcc34dd 100644 (file)
@@ -631,7 +631,7 @@ class EmptyProgressTrackerClient : public ProgressTrackerClient {
     virtual void progressFinished(Frame&) override { }
 };
 
     virtual void progressFinished(Frame&) override { }
 };
 
-class EmptyDiagnosticLoggingClient : public DiagnosticLoggingClient {
+class EmptyDiagnosticLoggingClient final : public DiagnosticLoggingClient {
     virtual void logDiagnosticMessage(const String&, const String&) override { }
     virtual void logDiagnosticMessageWithResult(const String&, const String&, DiagnosticLoggingResultType) override { }
     virtual void logDiagnosticMessageWithValue(const String&, const String&, const String&) override { }
     virtual void logDiagnosticMessage(const String&, const String&) override { }
     virtual void logDiagnosticMessageWithResult(const String&, const String&, DiagnosticLoggingResultType) override { }
     virtual void logDiagnosticMessageWithValue(const String&, const String&, const String&) override { }
index 700cf54..0374739 100644 (file)
@@ -1400,10 +1400,7 @@ static void logNavigation(MainFrame& frame, FrameLoadType type)
         // Not logging those for now.
         return;
     }
         // Not logging those for now.
         return;
     }
-    if (frame.settings().diagnosticLoggingEnabled()) {
-        if (auto* client = frame.diagnosticLoggingClient())
-            client->logDiagnosticMessage(DiagnosticLoggingKeys::navigationKey(), navigationDescription);
-    }
+    frame.diagnosticLoggingClient().logDiagnosticMessage(DiagnosticLoggingKeys::navigationKey(), navigationDescription);
 }
 
 void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType type, PassRefPtr<FormState> prpFormState, AllowNavigationToInvalidURL allowNavigationToInvalidURL)
 }
 
 void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType type, PassRefPtr<FormState> prpFormState, AllowNavigationToInvalidURL allowNavigationToInvalidURL)
@@ -2277,14 +2274,8 @@ void FrameLoader::checkLoadCompleteForThisFrame()
             if (AXObjectCache* cache = m_frame.document()->existingAXObjectCache())
                 cache->frameLoadingEventNotification(&m_frame, loadingEvent);
 
             if (AXObjectCache* cache = m_frame.document()->existingAXObjectCache())
                 cache->frameLoadingEventNotification(&m_frame, loadingEvent);
 
-            if (!page || !page->settings().diagnosticLoggingEnabled())
-                return;
-
-            DiagnosticLoggingClient* diagnosticLoggingClient = page->mainFrame().diagnosticLoggingClient();
-            if (!diagnosticLoggingClient)
-                return;
-
-            diagnosticLoggingClient->logDiagnosticMessageWithResult(DiagnosticLoggingKeys::pageLoadedKey(), emptyString(), error.isNull() ? DiagnosticLoggingResultPass : DiagnosticLoggingResultFail);
+            if (page)
+                page->mainFrame().diagnosticLoggingClient().logDiagnosticMessageWithResult(DiagnosticLoggingKeys::pageLoadedKey(), emptyString(), error.isNull() ? DiagnosticLoggingResultPass : DiagnosticLoggingResultFail);
 
             return;
         }
 
             return;
         }
index 3a260d7..5ecfc70 100644 (file)
@@ -180,7 +180,7 @@ static String findPluginMIMETypeFromURL(Page* page, const String& url)
 
 static void logPluginRequest(Page* page, const String& mimeType, const String& url, bool success)
 {
 
 static void logPluginRequest(Page* page, const String& mimeType, const String& url, bool success)
 {
-    if (!page || !page->settings().diagnosticLoggingEnabled())
+    if (!page)
         return;
 
     String newMIMEType = mimeType;
         return;
 
     String newMIMEType = mimeType;
@@ -194,17 +194,14 @@ static void logPluginRequest(Page* page, const String& mimeType, const String& u
     String pluginFile = page->pluginData().pluginFileForMimeType(newMIMEType);
     String description = !pluginFile ? newMIMEType : pluginFile;
 
     String pluginFile = page->pluginData().pluginFileForMimeType(newMIMEType);
     String description = !pluginFile ? newMIMEType : pluginFile;
 
-    DiagnosticLoggingClient* diagnosticLoggingClient = page->mainFrame().diagnosticLoggingClient();
-    if (!diagnosticLoggingClient)
-        return;
-
-    diagnosticLoggingClient->logDiagnosticMessage(success ? DiagnosticLoggingKeys::pluginLoadedKey() : DiagnosticLoggingKeys::pluginLoadingFailedKey(), description);
+    DiagnosticLoggingClient& diagnosticLoggingClient = page->mainFrame().diagnosticLoggingClient();
+    diagnosticLoggingClient.logDiagnosticMessage(success ? DiagnosticLoggingKeys::pluginLoadedKey() : DiagnosticLoggingKeys::pluginLoadingFailedKey(), description);
 
     if (!page->hasSeenAnyPlugin())
 
     if (!page->hasSeenAnyPlugin())
-        diagnosticLoggingClient->logDiagnosticMessage(DiagnosticLoggingKeys::pageContainsAtLeastOnePluginKey(), emptyString());
+        diagnosticLoggingClient.logDiagnosticMessage(DiagnosticLoggingKeys::pageContainsAtLeastOnePluginKey(), emptyString());
 
     if (!page->hasSeenPlugin(description))
 
     if (!page->hasSeenPlugin(description))
-        diagnosticLoggingClient->logDiagnosticMessage(DiagnosticLoggingKeys::pageContainsPluginKey(), description);
+        diagnosticLoggingClient.logDiagnosticMessage(DiagnosticLoggingKeys::pageContainsPluginKey(), description);
 
     page->sawPlugin(description);
 }
 
     page->sawPlugin(description);
 }
index ad4a524..fd94457 100644 (file)
@@ -298,7 +298,7 @@ bool SubresourceLoader::checkForHTTPStatusCodeError()
 
 static void logResourceLoaded(Frame* frame, CachedResource::Type type)
 {
 
 static void logResourceLoaded(Frame* frame, CachedResource::Type type)
 {
-    if (!frame || !frame->settings().diagnosticLoggingEnabled())
+    if (!frame)
         return;
 
     String resourceType;
         return;
 
     String resourceType;
@@ -340,8 +340,7 @@ static void logResourceLoaded(Frame* frame, CachedResource::Type type)
         resourceType = DiagnosticLoggingKeys::otherKey();
         break;
     }
         resourceType = DiagnosticLoggingKeys::otherKey();
         break;
     }
-    if (auto* client = frame->mainFrame().diagnosticLoggingClient())
-        client->logDiagnosticMessageWithValue(DiagnosticLoggingKeys::resourceKey(), DiagnosticLoggingKeys::loadedKey(), resourceType);
+    frame->mainFrame().diagnosticLoggingClient().logDiagnosticMessageWithValue(DiagnosticLoggingKeys::resourceKey(), DiagnosticLoggingKeys::loadedKey(), resourceType);
 }
 
 void SubresourceLoader::didFinishLoading(double finishTime)
 }
 
 void SubresourceLoader::didFinishLoading(double finishTime)
index cf2f189..a0da85d 100644 (file)
@@ -33,9 +33,9 @@ namespace WebCore {
 
 class DiagnosticLoggingClient {
 public:
 
 class DiagnosticLoggingClient {
 public:
-    virtual void logDiagnosticMessage(const String& message, const String& description) { UNUSED_PARAM(message); UNUSED_PARAM(description); }
-    virtual void logDiagnosticMessageWithResult(const String& message, const String& description, DiagnosticLoggingResultType) { UNUSED_PARAM(message); UNUSED_PARAM(description); }
-    virtual void logDiagnosticMessageWithValue(const String& message, const String& description, const String& value) { UNUSED_PARAM(message); UNUSED_PARAM(description); UNUSED_PARAM(value); }
+    virtual void logDiagnosticMessage(const String& message, const String& description) = 0;
+    virtual void logDiagnosticMessageWithResult(const String& message, const String& description, DiagnosticLoggingResultType) = 0;
+    virtual void logDiagnosticMessageWithValue(const String& message, const String& description, const String& value) = 0;
 
 protected:
     virtual ~DiagnosticLoggingClient() { }
 
 protected:
     virtual ~DiagnosticLoggingClient() { }
index fda1183..8732301 100644 (file)
 #include "MainFrame.h"
 
 #include "Element.h"
 #include "MainFrame.h"
 
 #include "Element.h"
+#include "EmptyClients.h"
 #include "PageConfiguration.h"
 #include "PageOverlayController.h"
 #include "ScrollLatchingState.h"
 #include "PageConfiguration.h"
 #include "PageOverlayController.h"
 #include "ScrollLatchingState.h"
+#include "Settings.h"
 #include "WheelEventDeltaTracker.h"
 #include "WheelEventDeltaTracker.h"
+#include <wtf/NeverDestroyed.h>
 
 #if PLATFORM(MAC)
 #include "ServicesOverlayController.h"
 
 #if PLATFORM(MAC)
 #include "ServicesOverlayController.h"
@@ -81,6 +84,15 @@ void MainFrame::selfOnlyDeref()
     deref();
 }
 
     deref();
 }
 
+DiagnosticLoggingClient& MainFrame::diagnosticLoggingClient() const
+{
+    static NeverDestroyed<EmptyDiagnosticLoggingClient> dummyClient;
+    if (!settings().diagnosticLoggingEnabled() || !m_diagnosticLoggingClient)
+        return dummyClient;
+
+    return *m_diagnosticLoggingClient;
+}
+
 void MainFrame::dropChildren()
 {
     while (Frame* child = tree().firstChild())
 void MainFrame::dropChildren()
 {
     while (Frame* child = tree().firstChild())
index 31d8852..ec77446 100644 (file)
@@ -61,7 +61,7 @@ public:
     void resetLatchingState();
 #endif // PLATFORM(MAC)
 
     void resetLatchingState();
 #endif // PLATFORM(MAC)
 
-    DiagnosticLoggingClient* diagnosticLoggingClient() const { return m_diagnosticLoggingClient; }
+    DiagnosticLoggingClient& diagnosticLoggingClient() const;
 
 private:
     MainFrame(Page&, PageConfiguration&);
 
 private:
     MainFrame(Page&, PageConfiguration&);