Title: [218914] trunk/Source/WebCore
Revision
218914
Author
cdu...@apple.com
Date
2017-06-28 20:55:28 -0700 (Wed, 28 Jun 2017)

Log Message

ResourceLoadObserver clean up
https://bugs.webkit.org/show_bug.cgi?id=173955

Reviewed by Sam Weinig and Brent Fulgham.

ResourceLoadObserver clean up: Modernize code a bit and get rid of unused variables.

* loader/ResourceLoadObserver.cpp:
(WebCore::ResourceLoadObserver::clearInMemoryStore):
(WebCore::ResourceLoadObserver::clearInMemoryAndPersistentStore):
(WebCore::ResourceLoadObserver::shouldLog):
(WebCore::ResourceLoadObserver::logFrameNavigation):
(WebCore::ResourceLoadObserver::logSubresourceLoading):
(WebCore::ResourceLoadObserver::logWebSocketLoading):
(WebCore::ResourceLoadObserver::logUserInteractionWithReducedTimeResolution):
(WebCore::ResourceLoadObserver::logUserInteraction):
(WebCore::ResourceLoadObserver::setSubframeUnderTopFrameOrigin):
(WebCore::ResourceLoadObserver::setSubresourceUnderTopFrameOrigin):
(WebCore::ResourceLoadObserver::setSubresourceUniqueRedirectTo):
(WebCore::ResourceLoadObserver::fireDataModificationHandler):
(WebCore::ResourceLoadObserver::fireShouldPartitionCookiesHandler):
(WebCore::ResourceLoadObserver::primaryDomain):
(WebCore::ResourceLoadObserver::statisticsForOrigin):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (218913 => 218914)


--- trunk/Source/WebCore/ChangeLog	2017-06-29 03:54:05 UTC (rev 218913)
+++ trunk/Source/WebCore/ChangeLog	2017-06-29 03:55:28 UTC (rev 218914)
@@ -1,3 +1,29 @@
+2017-06-28  Chris Dumez  <cdu...@apple.com>
+
+        ResourceLoadObserver clean up
+        https://bugs.webkit.org/show_bug.cgi?id=173955
+
+        Reviewed by Sam Weinig and Brent Fulgham.
+
+        ResourceLoadObserver clean up: Modernize code a bit and get rid of unused variables.
+
+        * loader/ResourceLoadObserver.cpp:
+        (WebCore::ResourceLoadObserver::clearInMemoryStore):
+        (WebCore::ResourceLoadObserver::clearInMemoryAndPersistentStore):
+        (WebCore::ResourceLoadObserver::shouldLog):
+        (WebCore::ResourceLoadObserver::logFrameNavigation):
+        (WebCore::ResourceLoadObserver::logSubresourceLoading):
+        (WebCore::ResourceLoadObserver::logWebSocketLoading):
+        (WebCore::ResourceLoadObserver::logUserInteractionWithReducedTimeResolution):
+        (WebCore::ResourceLoadObserver::logUserInteraction):
+        (WebCore::ResourceLoadObserver::setSubframeUnderTopFrameOrigin):
+        (WebCore::ResourceLoadObserver::setSubresourceUnderTopFrameOrigin):
+        (WebCore::ResourceLoadObserver::setSubresourceUniqueRedirectTo):
+        (WebCore::ResourceLoadObserver::fireDataModificationHandler):
+        (WebCore::ResourceLoadObserver::fireShouldPartitionCookiesHandler):
+        (WebCore::ResourceLoadObserver::primaryDomain):
+        (WebCore::ResourceLoadObserver::statisticsForOrigin):
+
 2017-06-28  Zalan Bujtas  <za...@apple.com>
 
         Move RenderEmbeddedObject::isReplacementObscured to HTMLPlugInElement

Modified: trunk/Source/WebCore/loader/ResourceLoadObserver.cpp (218913 => 218914)


--- trunk/Source/WebCore/loader/ResourceLoadObserver.cpp	2017-06-29 03:54:05 UTC (rev 218913)
+++ trunk/Source/WebCore/loader/ResourceLoadObserver.cpp	2017-06-29 03:55:28 UTC (rev 218914)
@@ -77,7 +77,7 @@
         return;
     
     ASSERT(m_queue);
-    m_queue->dispatch([this] () {
+    m_queue->dispatch([this] {
         m_store->clearInMemory();
     });
 }
@@ -88,7 +88,7 @@
         return;
     
     ASSERT(m_queue);
-    m_queue->dispatch([this] () {
+    m_queue->dispatch([this] {
         m_store->clearInMemoryAndPersistent();
     });
 }
@@ -110,9 +110,8 @@
     // FIXME: Err on the safe side until we have sorted out what to do in worker contexts
     if (!page)
         return false;
-    return Settings::resourceLoadStatisticsEnabled()
-        && !page->usesEphemeralSession()
-        && m_store;
+
+    return Settings::resourceLoadStatisticsEnabled() && !page->usesEphemeralSession() && m_store;
 }
 
 void ResourceLoadObserver::logFrameNavigation(const Frame& frame, const Frame& topFrame, const ResourceRequest& newRequest, const ResourceResponse& redirectResponse)
@@ -126,9 +125,9 @@
 
     bool isRedirect = is3xxRedirect(redirectResponse);
     bool isMainFrame = frame.isMainFrame();
-    const URL& sourceURL = frame.document()->url();
-    const URL& targetURL = newRequest.url();
-    const URL& mainFrameURL = topFrame.document()->url();
+    auto& sourceURL = frame.document()->url();
+    auto& targetURL = newRequest.url();
+    auto& mainFrameURL = topFrame.document()->url();
     
     if (!targetURL.isValid() || !mainFrameURL.isValid())
         return;
@@ -147,9 +146,7 @@
         return;
     
     ASSERT(m_queue);
-    m_queue->dispatch([this, isMainFrame, isRedirect, sourcePrimaryDomain = sourcePrimaryDomain.isolatedCopy(), mainFramePrimaryDomain = mainFramePrimaryDomain.isolatedCopy(), targetURL = CrossThreadCopier<URL>::copy(targetURL), mainFrameURL = CrossThreadCopier<URL>::copy(mainFrameURL), targetPrimaryDomain = targetPrimaryDomain.isolatedCopy()] () {
-        
-        auto targetOrigin = SecurityOrigin::create(targetURL);
+    m_queue->dispatch([this, isMainFrame, isRedirect, sourcePrimaryDomain = sourcePrimaryDomain.isolatedCopy(), mainFramePrimaryDomain = mainFramePrimaryDomain.isolatedCopy(), targetPrimaryDomain = targetPrimaryDomain.isolatedCopy()] {
         bool shouldFireDataModificationHandler = false;
         
         {
@@ -165,7 +162,6 @@
         else {
             targetStatistics.subframeHasBeenLoadedBefore = true;
 
-            auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
             auto subframeUnderTopFrameOriginsResult = targetStatistics.subframeUnderTopFrameOrigins.add(mainFramePrimaryDomain);
             if (subframeUnderTopFrameOriginsResult.isNewEntry)
                 shouldFireDataModificationHandler = true;
@@ -229,10 +225,7 @@
     auto targetHost = targetURL.host();
     auto mainFrameHost = mainFrameURL.host();
 
-    if (targetHost.isEmpty()
-        || mainFrameHost.isEmpty()
-        || targetHost == mainFrameHost
-        || (isRedirect && targetHost == sourceURL.host()))
+    if (targetHost.isEmpty() || mainFrameHost.isEmpty() || targetHost == mainFrameHost || (isRedirect && targetHost == sourceURL.host()))
         return;
 
     auto targetPrimaryDomain = primaryDomain(targetURL);
@@ -243,8 +236,7 @@
         return;
     
     ASSERT(m_queue);
-    m_queue->dispatch([this, isRedirect, sourcePrimaryDomain = sourcePrimaryDomain.isolatedCopy(), mainFramePrimaryDomain = mainFramePrimaryDomain.isolatedCopy(), targetPrimaryDomain = targetPrimaryDomain.isolatedCopy(), mainFrameURL = mainFrameURL.isolatedCopy()] () {
-        
+    m_queue->dispatch([this, isRedirect, sourcePrimaryDomain = sourcePrimaryDomain.isolatedCopy(), mainFramePrimaryDomain = mainFramePrimaryDomain.isolatedCopy(), targetPrimaryDomain = targetPrimaryDomain.isolatedCopy()] {
         bool shouldFireDataModificationHandler = false;
         
         {
@@ -254,7 +246,6 @@
         // Always fire if we have previously removed data records for this domain
         shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
 
-        auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
         auto subresourceUnderTopFrameOriginsResult = targetStatistics.subresourceUnderTopFrameOrigins.add(mainFramePrimaryDomain);
         if (subresourceUnderTopFrameOriginsResult.isNewEntry)
             shouldFireDataModificationHandler = true;
@@ -304,14 +295,12 @@
     if (!shouldLog(frame->page()))
         return;
 
-    const URL& mainFrameURL = frame->mainFrame().document()->url();
+    auto& mainFrameURL = frame->mainFrame().document()->url();
 
     auto targetHost = targetURL.host();
     auto mainFrameHost = mainFrameURL.host();
     
-    if (targetHost.isEmpty()
-        || mainFrameHost.isEmpty()
-        || targetHost == mainFrameHost)
+    if (targetHost.isEmpty() || mainFrameHost.isEmpty() || targetHost == mainFrameHost)
         return;
     
     auto targetPrimaryDomain = primaryDomain(targetURL);
@@ -321,8 +310,7 @@
         return;
 
     ASSERT(m_queue);
-    m_queue->dispatch([this, targetPrimaryDomain = targetPrimaryDomain.isolatedCopy(), mainFramePrimaryDomain = mainFramePrimaryDomain.isolatedCopy(), mainFrameURL = mainFrameURL.isolatedCopy()] () {
-        
+    m_queue->dispatch([this, targetPrimaryDomain = targetPrimaryDomain.isolatedCopy(), mainFramePrimaryDomain = mainFramePrimaryDomain.isolatedCopy()] {
         bool shouldFireDataModificationHandler = false;
         
         {
@@ -332,7 +320,6 @@
         // Always fire if we have previously removed data records for this domain
         shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
         
-        auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
         auto subresourceUnderTopFrameOriginsResult = targetStatistics.subresourceUnderTopFrameOrigins.add(mainFramePrimaryDomain);
         if (subresourceUnderTopFrameOriginsResult.isNewEntry)
             shouldFireDataModificationHandler = true;
@@ -365,10 +352,8 @@
     if (url.isBlankURL() || url.isEmpty())
         return;
 
-    auto primaryDomainString = primaryDomain(url);
-    
     ASSERT(m_queue);
-    m_queue->dispatch([this, primaryDomainString = primaryDomainString.isolatedCopy()] () {
+    m_queue->dispatch([this, primaryDomainString = primaryDomain(url).isolatedCopy()] {
         {
         auto locker = holdLock(m_store->statisticsLock());
         auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomainString);
@@ -389,10 +374,8 @@
     if (url.isBlankURL() || url.isEmpty())
         return;
 
-    auto primaryDomainString = primaryDomain(url);
-
     ASSERT(m_queue);
-    m_queue->dispatch([this, primaryDomainString = primaryDomainString.isolatedCopy()] () {
+    m_queue->dispatch([this, primaryDomainString = primaryDomain(url).isolatedCopy()] {
         {
         auto locker = holdLock(m_store->statisticsLock());
         auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primaryDomainString);
@@ -487,11 +470,8 @@
     if (subframe.isBlankURL() || subframe.isEmpty() || topFrame.isBlankURL() || topFrame.isEmpty())
         return;
     
-    auto primaryTopFrameDomainString = primaryDomain(topFrame);
-    auto primarySubFrameDomainString = primaryDomain(subframe);
-    
     ASSERT(m_queue);
-    m_queue->dispatch([this, primaryTopFrameDomainString = primaryTopFrameDomainString.isolatedCopy(), primarySubFrameDomainString = primarySubFrameDomainString.isolatedCopy()] () {
+    m_queue->dispatch([this, primaryTopFrameDomainString = primaryDomain(topFrame).isolatedCopy(), primarySubFrameDomainString = primaryDomain(subframe).isolatedCopy()] {
         auto locker = holdLock(m_store->statisticsLock());
         auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primarySubFrameDomainString);
         statistics.subframeUnderTopFrameOrigins.add(primaryTopFrameDomainString);
@@ -503,11 +483,8 @@
     if (subresource.isBlankURL() || subresource.isEmpty() || topFrame.isBlankURL() || topFrame.isEmpty())
         return;
     
-    auto primaryTopFrameDomainString = primaryDomain(topFrame);
-    auto primarySubresourceDomainString = primaryDomain(subresource);
-    
     ASSERT(m_queue);
-    m_queue->dispatch([this, primaryTopFrameDomainString = primaryTopFrameDomainString.isolatedCopy(), primarySubresourceDomainString = primarySubresourceDomainString.isolatedCopy()] () {
+    m_queue->dispatch([this, primaryTopFrameDomainString = primaryDomain(topFrame).isolatedCopy(), primarySubresourceDomainString = primaryDomain(subresource).isolatedCopy()] {
         auto locker = holdLock(m_store->statisticsLock());
         auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primarySubresourceDomainString);
         statistics.subresourceUnderTopFrameOrigins.add(primaryTopFrameDomainString);
@@ -519,11 +496,8 @@
     if (subresource.isBlankURL() || subresource.isEmpty() || hostNameRedirectedTo.isBlankURL() || hostNameRedirectedTo.isEmpty())
         return;
     
-    auto primarySubresourceDomainString = primaryDomain(subresource);
-    auto primaryRedirectDomainString = primaryDomain(hostNameRedirectedTo);
-    
     ASSERT(m_queue);
-    m_queue->dispatch([this, primaryRedirectDomainString = primaryRedirectDomainString.isolatedCopy(), primarySubresourceDomainString = primarySubresourceDomainString.isolatedCopy()] () {
+    m_queue->dispatch([this, primaryRedirectDomainString = primaryDomain(hostNameRedirectedTo).isolatedCopy(), primarySubresourceDomainString = primaryDomain(subresource).isolatedCopy()] {
         auto locker = holdLock(m_store->statisticsLock());
         auto& statistics = m_store->ensureResourceStatisticsForPrimaryDomain(primarySubresourceDomainString);
         statistics.subresourceUniqueRedirectsTo.add(primaryRedirectDomainString);
@@ -560,7 +534,7 @@
 {
     // Helper function used by testing system. Should only be called from the main thread.
     ASSERT(isMainThread());
-    m_queue->dispatch([this] () {
+    m_queue->dispatch([this] {
         m_store->fireDataModificationHandler();
     });
 }
@@ -569,7 +543,7 @@
 {
     // Helper function used by testing system. Should only be called from the main thread.
     ASSERT(isMainThread());
-    m_queue->dispatch([this] () {
+    m_queue->dispatch([this] {
         m_store->fireShouldPartitionCookiesHandler();
     });
 }
@@ -578,7 +552,7 @@
 {
     // Helper function used by testing system. Should only be called from the main thread.
     ASSERT(isMainThread());
-    m_queue->dispatch([this, domainsToRemove = CrossThreadCopier<Vector<String>>::copy(domainsToRemove), domainsToAdd = CrossThreadCopier<Vector<String>>::copy(domainsToAdd), clearFirst] () {
+    m_queue->dispatch([this, domainsToRemove = CrossThreadCopier<Vector<String>>::copy(domainsToRemove), domainsToAdd = CrossThreadCopier<Vector<String>>::copy(domainsToAdd), clearFirst] {
         m_store->fireShouldPartitionCookiesHandler(domainsToRemove, domainsToAdd, clearFirst);
     });
 }
@@ -597,31 +571,22 @@
 
 String ResourceLoadObserver::primaryDomain(const String& host)
 {
-    String primaryDomain;
     if (host.isNull() || host.isEmpty())
-        primaryDomain = "nullOrigin";
+        return ASCIILiteral("nullOrigin");
+
 #if ENABLE(PUBLIC_SUFFIX_LIST)
-    else {
-        primaryDomain = topPrivatelyControlledDomain(host);
-        // We will have an empty string here if there is no TLD.
-        // Use the host in such case.
-        if (primaryDomain.isEmpty())
-            primaryDomain = host;
-    }
-#else
-    else
-        primaryDomain = host;
+    String primaryDomain = topPrivatelyControlledDomain(host);
+    // We will have an empty string here if there is no TLD. Use the host as a fallback.
+    if (!primaryDomain.isEmpty())
+        return primaryDomain;
 #endif
 
-    return primaryDomain;
+    return host;
 }
 
 String ResourceLoadObserver::statisticsForOrigin(const String& origin)
 {
-    if (!m_store)
-        return emptyString();
-    
-    return m_store->statisticsForOrigin(origin);
+    return m_store ? m_store->statisticsForOrigin(origin) : emptyString();
 }
 
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to