Fixed first pass at adding http/backend cache events to the NetLog. ...

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...


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=70628

[mmenke, 2011-01-06 18:03:47]

Broke the release buildbots, as they're more sensitive than the debug trybots. 
Not sure if I really need a re-review for such a small change, but after
breaking trunk, seems best to be on the safe side.

[eroman, 2011-01-06 18:37:15]

Can you explain the fix? All I see in patchset 2 is an extra return
statement, following a NOTREACHED.

We should never be reaching NOTREACHED (regardless of debug/release
mode), so this shouldn't be the correct solution.

On Thu, Jan 6, 2011 at 10:03 AM,  <mmenke@chromium.org> wrote:
> Reviewers: eroman,
>
> Message:
> Broke the release buildbots, as they're more sensitive than the debug
> trybots.
> Not sure if I really need a re-review for such a small change, but after
> breaking trunk, seems best to be on the safe side.
>
> Description:
> 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...
>
>
> Please review this at http://codereview.chromium.org/6125001/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src/
>
> Affected files:
>  M     chrome/browser/net/connection_tester.cc
>  M     chrome/browser/net/passive_log_collector.h
>  M     chrome/browser/net/passive_log_collector.cc
>  M     chrome/browser/resources/net_internals/logviewpainter.js
>  M     chrome/browser/resources/net_internals/main.css
>  M     chrome/browser/resources/net_internals/sourceentry.js
>  M     chrome/service/net/service_url_request_context.cc
>  M     chrome/test/plugin/plugin_test.cpp
>  M     chrome_frame/metrics_service.cc
>  M     chrome_frame/test/test_server_test.cc
>  M     net/base/capturing_net_log.h
>  M     net/base/capturing_net_log.cc
>  M     net/base/net_log.h
>  M     net/base/net_log.cc
>  M     net/base/net_log_event_type_list.h
>  M     net/base/net_log_source_type_list.h
>  M     net/disk_cache/backend_impl.h
>  M     net/disk_cache/backend_impl.cc
>  M     net/disk_cache/backend_unittest.cc
>  M     net/disk_cache/disk_cache.h
>  M     net/disk_cache/disk_cache_perftest.cc
>  M     net/disk_cache/disk_cache_test_base.cc
>  M     net/disk_cache/disk_cache_test_util.cc
>  M     net/disk_cache/entry_impl.h
>  M     net/disk_cache/entry_impl.cc
>  M     net/disk_cache/sparse_control.cc
>  M     net/disk_cache/stress_cache.cc
>  M     net/http/http_cache.h
>  M     net/http/http_cache.cc
>  M     net/http/http_cache_transaction.h
>  M     net/http/http_cache_transaction.cc
>  M     net/http/http_cache_unittest.cc
>  M     net/http/http_network_session.h
>  M     net/proxy/proxy_script_fetcher_impl_unittest.cc
>  M     net/spdy/spdy_test_util.h
>  M     net/tools/crash_cache/crash_cache.cc
>  M     net/tools/dump_cache/upgrade.cc
>  M     net/url_request/url_request_unittest.h
>  M     net/url_request/view_cache_helper_unittest.cc
>  M     webkit/appcache/appcache_disk_cache.cc
>
>
>

[mmenke, 2011-01-06 18:41:43]

On 2011/01/06 18:37:15, eroman wrote:
> Can you explain the fix? All I see in patchset 2 is an extra return
> statement, following a NOTREACHED.
> 
> We should never be reaching NOTREACHED (regardless of debug/release
> mode), so this shouldn't be the correct solution.

That is, in fact, the only change.  The issue is the compilers warn when an
uninitialized value is used, and are set to treat warnings as errors, so
compilation fails.  The warning only occurs on the release builders.

Log from the Linux release build:

cc1plus: warnings being treated as errors
net/disk_cache/sparse_control.cc: In member function 'void
disk_cache::SparseControl::DoChildIOCompleted(int)':
net/disk_cache/sparse_control.cc:146: warning: 'event_type' may be used
uninitialized in this function
net/disk_cache/sparse_control.cc:146: note: 'event_type' was declared here

[eroman, 2011-01-06 18:42:54]

Ah ok then.

LGTM

On Thu, Jan 6, 2011 at 10:41 AM,  <mmenke@chromium.org> wrote:
> On 2011/01/06 18:37:15, eroman wrote:
>>
>> Can you explain the fix? All I see in patchset 2 is an extra return
>> statement, following a NOTREACHED.
>
>> We should never be reaching NOTREACHED (regardless of debug/release
>> mode), so this shouldn't be the correct solution.
>
> That is, in fact, the only change.  The issue is the compilers warn when an
> uninitialized value is used, and are set to treat warnings as errors, so
> compilation fails.  The warning only occurs on the release builders.
>
> Log from the Linux release build:
>
> cc1plus: warnings being treated as errors
> net/disk_cache/sparse_control.cc: In member function 'void
> disk_cache::SparseControl::DoChildIOCompleted(int)':
> net/disk_cache/sparse_control.cc:146: warning: 'event_type' may be used
> uninitialized in this function
> net/disk_cache/sparse_control.cc:146: note: 'event_type' was declared here
>
>
> http://codereview.chromium.org/6125001/
>