Revert r70628. It broke Linux valgrind bots. See for example

http://build.chromium.org/p/chromium.memory/builders/Linux%20Tests%20%28valgrind%29%281%29/builds/963 First pass at adding http/backend cache events to the NetLog. Adds sources and events for ActiveCacheEntry and EntryImpl objects, as well as adding cache read/write events to HttpCacheTransactions. Most new read/write events are only logged when NetLog logging mode is set to log all events. Also, net-internals now merges begin and end events that have parameters, as long as only one of them has parameters. I think this increases readability, at the cost of making it a little more difficult to follow timings with just the "st" values. BUG=59382 TEST=none yet, other than updates to existing tests. Originally applied: http://src.chromium.org/viewvc/chrome?view=rev&revision=70618 Reverted: http://src.chromium.org/viewvc/chrome?view=rev&revision=70619 Fixed and trying again... Review URL: http://codereview.chromium.org/6125001 BUG=none TEST=linux valgrind goes green. Review URL: http://codereview.chromium.org/6142003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@70688 0039d316-1c4b-4281-b951-d872f2087c98
jiongle1 · Jan 7, 2011 · a6ec089 · a6ec089
1 parent 1b4209f
commit a6ec089
Show file tree

Hide file tree

Showing 40 changed files with 142 additions and 619 deletions.
diff --git a/chrome/browser/net/connection_tester.cc b/chrome/browser/net/connection_tester.cc
@@ -71,7 +71,6 @@ class ExperimentURLRequestContext : public URLRequestContext {
             dnsrr_resolver_, NULL /* dns_cert_checker */,
             NULL /* ssl_host_info_factory */, proxy_service_,
             ssl_config_service_, http_auth_handler_factory_, NULL, NULL),
-        NULL /* net_log */,
         net::HttpCache::DefaultBackend::InMemory(0));
     // In-memory cookie store.
     cookie_store_ = new net::CookieMonster(NULL, NULL);

diff --git a/chrome/browser/net/passive_log_collector.cc b/chrome/browser/net/passive_log_collector.cc
@@ -70,7 +70,6 @@ PassiveLogCollector::PassiveLogCollector()
   trackers_[net::NetLog::SOURCE_HOST_RESOLVER_IMPL_REQUEST] =
       &dns_request_tracker_;
   trackers_[net::NetLog::SOURCE_HOST_RESOLVER_IMPL_JOB] = &dns_job_tracker_;
-  trackers_[net::NetLog::SOURCE_DISK_CACHE_ENTRY] = &disk_cache_entry_tracker_;
   // Make sure our mapping is up-to-date.
   for (size_t i = 0; i < arraysize(trackers_); ++i)
     DCHECK(trackers_[i]) << "Unhandled SourceType: " << i;
@@ -561,28 +560,3 @@ PassiveLogCollector::DNSJobTracker::DoAddEntry(const ChromeNetLog::Entry& entry,
     return ACTION_NONE;
   }
 }
-
-//----------------------------------------------------------------------------
-// DiskCacheEntryTracker
-//----------------------------------------------------------------------------
-
-const size_t PassiveLogCollector::DiskCacheEntryTracker::kMaxNumSources = 100;
-const size_t PassiveLogCollector::DiskCacheEntryTracker::kMaxGraveyardSize = 25;
-
-PassiveLogCollector::DiskCacheEntryTracker::DiskCacheEntryTracker()
-    : SourceTracker(kMaxNumSources, kMaxGraveyardSize, NULL) {
-}
-
-PassiveLogCollector::SourceTracker::Action
-PassiveLogCollector::DiskCacheEntryTracker::DoAddEntry(
-    const ChromeNetLog::Entry& entry, SourceInfo* out_info) {
-  AddEntryToSourceInfo(entry, out_info);
-
-  // If the request has ended, move it to the graveyard.
-  if (entry.type == net::NetLog::TYPE_DISK_CACHE_ENTRY &&
-      entry.phase == net::NetLog::PHASE_END) {
-    return ACTION_MOVE_TO_GRAVEYARD;
-  }
-
-  return ACTION_NONE;
-}
diff --git a/chrome/browser/net/passive_log_collector.h b/chrome/browser/net/passive_log_collector.h
@@ -304,22 +304,6 @@ class PassiveLogCollector : public ChromeNetLog::ThreadSafeObserver {
     DISALLOW_COPY_AND_ASSIGN(DNSJobTracker);
   };
 
-  // Tracks the log entries for the last seen SOURCE_DISK_CACHE_ENTRY.
-  class DiskCacheEntryTracker : public SourceTracker {
-   public:
-    static const size_t kMaxNumSources;
-    static const size_t kMaxGraveyardSize;
-
-    DiskCacheEntryTracker();
-
-   protected:
-    virtual Action DoAddEntry(const ChromeNetLog::Entry& entry,
-                              SourceInfo* out_info);
-
-   private:
-    DISALLOW_COPY_AND_ASSIGN(DiskCacheEntryTracker);
-  };
-
   PassiveLogCollector();
   ~PassiveLogCollector();
 
@@ -356,7 +340,6 @@ class PassiveLogCollector : public ChromeNetLog::ThreadSafeObserver {
   SpdySessionTracker spdy_session_tracker_;
   DNSRequestTracker dns_request_tracker_;
   DNSJobTracker dns_job_tracker_;
-  DiskCacheEntryTracker disk_cache_entry_tracker_;
 
   // This array maps each NetLog::SourceType to one of the tracker instances
   // defined above. Use of this array avoid duplicating the list of trackers

diff --git a/chrome/browser/resources/net_internals/logviewpainter.js b/chrome/browser/resources/net_internals/logviewpainter.js
@@ -49,9 +49,10 @@ function addSourceEntry_(node, sourceEntry) {
 function canCollapseBeginWithEnd(beginEntry) {
   return beginEntry &&
          beginEntry.isBegin() &&
+         !beginEntry.orig.params &&
          beginEntry.end &&
          beginEntry.end.index == beginEntry.index + 1 &&
-         (!beginEntry.orig.params || !beginEntry.end.orig.params) &&
+         !beginEntry.end.orig.params &&
          beginEntry.orig.wasPassivelyCaptured ==
              beginEntry.end.orig.wasPassivelyCaptured;
 }
@@ -69,43 +70,43 @@ PrintSourceEntriesAsText = function(sourceEntries, doSecurityStripping) {
   for (var i = 0; i < entries.length; ++i) {
     var entry = entries[i];
 
-    // Avoid printing the END for a BEGIN that was immediately before, unless
-    // both have extra parameters.
-    if (!entry.isEnd() || !canCollapseBeginWithEnd(entry.begin)) {
-      tablePrinter.addRow();
-
-      // Annotate this entry with "(P)" if it was passively captured.
-      tablePrinter.addCell(entry.orig.wasPassivelyCaptured ? '(P) ' : '');
-
-      tablePrinter.addCell('t=');
-      var date = g_browser.convertTimeTicksToDate(entry.orig.time) ;
-      var tCell = tablePrinter.addCell(date.getTime());
-      tCell.alignRight = true;
-      tablePrinter.addCell(' [st=');
-      var stCell = tablePrinter.addCell(date.getTime() - startTime);
-      stCell.alignRight = true;
-      tablePrinter.addCell('] ');
-
-      var indentationStr = makeRepeatedString(' ', entry.getDepth() * 3);
-      var mainCell =
-          tablePrinter.addCell(indentationStr + getTextForEvent(entry));
-      tablePrinter.addCell('  ');
-
-      // Get the elapsed time.
-      if (entry.isBegin()) {
-        tablePrinter.addCell('[dt=');
-        var dt = '?';
-        // Definite time.
-        if (entry.end) {
-          dt = entry.end.orig.time - entry.orig.time;
-        }
-        var dtCell = tablePrinter.addCell(dt);
-        dtCell.alignRight = true;
-
-        tablePrinter.addCell(']');
-      } else {
-        mainCell.allowOverflow = true;
+    // Avoid printing the END for a BEGIN that was immediately before.
+    if (entry.isEnd() && canCollapseBeginWithEnd(entry.begin))
+      continue;
+
+    tablePrinter.addRow();
+
+    // Annotate this entry with "(P)" if it was passively captured.
+    tablePrinter.addCell(entry.orig.wasPassivelyCaptured ? '(P) ' : '');
+
+    tablePrinter.addCell('t=');
+    var date = g_browser.convertTimeTicksToDate(entry.orig.time) ;
+    var tCell = tablePrinter.addCell(date.getTime());
+    tCell.alignRight = true;
+    tablePrinter.addCell(' [st=');
+    var stCell = tablePrinter.addCell(date.getTime() - startTime);
+    stCell.alignRight = true;
+    tablePrinter.addCell('] ');
+
+    var indentationStr = makeRepeatedString(' ', entry.getDepth() * 3);
+    var mainCell =
+        tablePrinter.addCell(indentationStr + getTextForEvent(entry));
+    tablePrinter.addCell('  ');
+
+    // Get the elapsed time.
+    if (entry.isBegin()) {
+      tablePrinter.addCell('[dt=');
+      var dt = '?';
+      // Definite time.
+      if (entry.end) {
+        dt = entry.end.orig.time - entry.orig.time;
       }
+      var dtCell = tablePrinter.addCell(dt);
+      dtCell.alignRight = true;
+
+      tablePrinter.addCell(']');
+    } else {
+      mainCell.allowOverflow = true;
     }
 
     // Output the extra parameters.

diff --git a/chrome/browser/resources/net_internals/main.css b/chrome/browser/resources/net_internals/main.css
@@ -92,10 +92,6 @@ body {
   color: #308080;
 }
 
-#eventsListTableBody .source_DISK_CACHE_ENTRY {
-  color: gray;
-}
-
 #eventsListTableBody .source_SOCKET {
   color: purple;
 }

diff --git a/chrome/browser/resources/net_internals/sourceentry.js b/chrome/browser/resources/net_internals/sourceentry.js
@@ -232,9 +232,6 @@ SourceEntry.prototype.getDescription = function() {
     case LogSourceType.HOST_RESOLVER_IMPL_JOB:
       description = e.params.host;
       break;
-    case LogSourceType.DISK_CACHE_ENTRY:
-      description = e.params.key;
-      break;
     case LogSourceType.SPDY_SESSION:
       if (e.params.host)
         description = e.params.host + ' (' + e.params.proxy + ')';

diff --git a/chrome/service/net/service_url_request_context.cc b/chrome/service/net/service_url_request_context.cc
@@ -135,7 +135,6 @@ ServiceURLRequestContext::ServiceURLRequestContext(
                                            http_auth_handler_factory_,
                                            NULL /* network_delegate */,
                                            NULL /* net_log */),
-      NULL /* net_log */,
       net::HttpCache::DefaultBackend::InMemory(0));
   // In-memory cookie store.
   cookie_store_ = new net::CookieMonster(NULL, NULL);

diff --git a/chrome/test/plugin/plugin_test.cpp b/chrome/test/plugin/plugin_test.cpp
@@ -303,7 +303,6 @@ class PluginInstallerDownloadTest
                                                http_auth_handler_factory_,
                                                network_delegate_,
                                                NULL),
-          NULL /* net_log */,
           net::HttpCache::DefaultBackend::InMemory(0));
     }
 

diff --git a/chrome_frame/metrics_service.cc b/chrome_frame/metrics_service.cc
@@ -190,7 +190,6 @@ class ChromeFrameUploadRequestContext : public URLRequestContext {
                                              http_auth_handler_factory_,
                                              network_delegate_,
                                              NULL),
-        NULL /* net_log */,
         net::HttpCache::DefaultBackend::InMemory(0));
   }
 

diff --git a/chrome_frame/test/test_server_test.cc b/chrome_frame/test/test_server_test.cc
@@ -81,7 +81,6 @@ class URLRequestTestContext : public URLRequestContext {
             http_auth_handler_factory_,
             NULL /* network_delegate */,
             NULL /* net_log */),
-        NULL /* net_log */,
         net::HttpCache::DefaultBackend::InMemory(0));
     // In-memory cookie store.
     cookie_store_ = new net::CookieMonster(NULL, NULL);

diff --git a/net/base/capturing_net_log.cc b/net/base/capturing_net_log.cc
@@ -18,9 +18,7 @@ CapturingNetLog::Entry::Entry(EventType type,
 CapturingNetLog::Entry::~Entry() {}
 
 CapturingNetLog::CapturingNetLog(size_t max_num_entries)
-    : last_id_(-1),
-      max_num_entries_(max_num_entries),
-      log_level_(LOG_ALL_BUT_BYTES) {
+    : last_id_(-1), max_num_entries_(max_num_entries) {
 }
 
 CapturingNetLog::~CapturingNetLog() {}
@@ -41,8 +39,7 @@ uint32 CapturingNetLog::NextID() {
 }
 
 NetLog::LogLevel CapturingNetLog::GetLogLevel() const {
-  AutoLock lock(lock_);
-  return log_level_;
+  return LOG_ALL_BUT_BYTES;
 }
 
 void CapturingNetLog::GetEntries(EntryList* entry_list) const {
@@ -55,11 +52,6 @@ void CapturingNetLog::Clear() {
   entries_.clear();
 }
 
-void CapturingNetLog::SetLogLevel(NetLog::LogLevel log_level) {
-  AutoLock lock(lock_);
-  log_level_ = log_level;
-}
-
 CapturingBoundNetLog::CapturingBoundNetLog(const NetLog::Source& source,
                                            CapturingNetLog* net_log)
     : source_(source), capturing_net_log_(net_log) {
@@ -79,8 +71,4 @@ void CapturingBoundNetLog::Clear() {
   capturing_net_log_->Clear();
 }
 
-void CapturingBoundNetLog::SetLogLevel(NetLog::LogLevel log_level) {
-  capturing_net_log_->SetLogLevel(log_level);
-}
-
 }  // namespace net
diff --git a/net/base/capturing_net_log.h b/net/base/capturing_net_log.h
@@ -61,8 +61,6 @@ class CapturingNetLog : public NetLog {
 
   void Clear();
 
-  void SetLogLevel(NetLog::LogLevel log_level);
-
  private:
   // Needs to be "mutable" so can use it in GetEntries().
   mutable Lock lock_;
@@ -73,8 +71,6 @@ class CapturingNetLog : public NetLog {
   size_t max_num_entries_;
   EntryList entries_;
 
-  NetLog::LogLevel log_level_;
-
   DISALLOW_COPY_AND_ASSIGN(CapturingNetLog);
 };
 
@@ -101,9 +97,6 @@ class CapturingBoundNetLog {
 
   void Clear();
 
-  // Sets the log level of the underlying CapturingNetLog.
-  void SetLogLevel(NetLog::LogLevel log_level);
-
  private:
   NetLog::Source source_;
   scoped_ptr<CapturingNetLog> capturing_net_log_;

diff --git a/net/base/net_log.cc b/net/base/net_log.cc
@@ -3,13 +3,11 @@
 // found in the LICENSE file.
 
 #include "net/base/net_log.h"
-
 #include "base/logging.h"
 #include "base/string_number_conversions.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
-#include "net/base/net_errors.h"
 
 namespace net {
 
@@ -159,18 +157,6 @@ void BoundNetLog::EndEvent(
   AddEntry(event_type, NetLog::PHASE_END, params);
 }
 
-void BoundNetLog::EndEventWithNetErrorCode(NetLog::EventType event_type,
-                                           int net_error) const {
-  DCHECK_NE(net_error, net::ERR_IO_PENDING);
-  if (net_error >= 0) {
-    EndEvent(event_type, NULL);
-  } else {
-    EndEvent(
-        event_type,
-        make_scoped_refptr(new NetLogIntegerParameter("net_error", net_error)));
-  }
-}
-
 // static
 BoundNetLog BoundNetLog::Make(NetLog* net_log,
                               NetLog::SourceType source_type) {

diff --git a/net/base/net_log.h b/net/base/net_log.h
@@ -34,8 +34,6 @@ namespace net {
 // TODO(eroman): Remove the 'const' qualitifer from the BoundNetLog methods.
 // TODO(eroman): Start a new Source each time net::URLRequest redirects
 //               (simpler to reason about each as a separate entity).
-// TODO(mmenke): Replace EndEvent calls with EndEventWithNetErrorCode, where
-//               appropriate.
 
 class NetLog {
  public:
@@ -195,13 +193,6 @@ class BoundNetLog {
   void EndEvent(NetLog::EventType event_type,
                 const scoped_refptr<NetLog::EventParameters>& params) const;
 
-  // Just like EndEvent, except |net_error| is a net error code.  If it's
-  // negative, a parameter called "net_error" with a value of |net_error| is
-  // associated with the event.  Otherwise, the end event has no parameters.
-  // |net_error| must not be ERR_IO_PENDING, as it's not a true error.
-  void EndEventWithNetErrorCode(NetLog::EventType event_type,
-                                int net_error) const;
-
   NetLog::LogLevel GetLogLevel() const;
 
   // Returns true if the log level is LOG_ALL.