Extracted NetLog class's inner enum types into their own enum classes and

Extracted NetLog class's inner enum types into their own enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

Here are a couple historical notes:

1) In refactoring "NetLog::TYPE_SSL_PRIVATE_KEY_OPERATION" to
"NetLogEventType::SSL_PRIVATE_KEY_OPERATION", the removal of "TYPE_" from
"TYPE_SSL_PRIVATE_KEY_OPERATION" created a compiler error when compiling
an ssl-based object file because it turns out that SSL_PRIVATE_KEY_OPERATION
is a macro definition in the ssl translation unit; this resulted in
a macro collision. To resolve, "NetLogEventType::SSL_PRIVATE_KEY_OPERATION"
was changed to "NetLogEventType::SSL_PRIVATE_KEY_OP".
    - There aren't any dependencies in the [netlog viewer] code
       that depend on the old naming so this should be fine.

2) The new enum classes, as opposed to enums, made it so that
[NetLogEnumClass]::LABEL needed to be statically cast to int at times.
-e.g.- In NetLog::GetEventTypesAsValue()'s for loop to
       iterate through all enums.
       - Also had to do this in 2 or 3 instances outside of net_log.cc.

BUG=473710
R=eroman@chromium.org,tfarina@chromium.org
TBR=asargent@chromium.org,thestig@chromium.org,bengr@chromium.org,avi@chromium.org,zea@chromium.org,rohitrao@chromium.org,sergeyu@chromium.org,mmenke@chromium.org

Committed: https://crrev.com/8b85c43f674d711380f660fea44a19653622ac61
Cr-Commit-Position: refs/heads/master@{#417358}

[mikecirone, 2016-09-06 07:16:45]

Description was changed from

==========
Extracted NetLog class's inner enum types into their enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

BUG=473710
==========

to

==========
Extracted NetLog class's inner enum types into their own enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

BUG=473710
==========

[mikecirone, 2016-09-06 07:20:17]

mikecirone@gmail.com changed reviewers:
+ eroman@chromium.org, mmenke@chromium.org, tfarina@chromium.org

[mikecirone, 2016-09-06 07:47:29]

Here are some notes on the development work:

1) The NetLogEventPhase enum class is inside of the new net_log_event_type.h
file. It seemed like a little overkill to have it inside of its
own header file.

2) The new enum classes, as opposed to enums, made it so that
[NetLogEnumClass]::LABEL needed to be statically cast to int at times
-e.g.- in a for loop to iterate through all enums.
--- Had to do this in 2 or 3 instances outside of net_log.cc.
    
3) The removal of "TYPE_" from "TYPE_SSL_PRIVATE_KEY_OPERATION" created a
compiler error when compiling an ssl-based object file because it turns out
that SSL_PRIVATE_KEY_OPERATION is a macro definition in that translation
unit. To resolve, I changed "NetLogEventType::SSL_PRIVATE_KEY_OPERATION"
to "NetLogEventType::SSL_PRIVATE_KEY_OP".

4) I noticed that I mistakenly indented a distance of 4 spaces as opposed
to 2 spaces a good amount of times over the entire refactoring. I'll clean
this up in follow-up work post review!

5) I'm in favor of fully completing the NetLog extraction of inner classes
before any potential commit to origin/master; it will be good to really
see the new structuring before committing to it. I did want to get a review
in, though, at this good stopping point of having the NetLog
inner enums extracted.

[mmenke, 2016-09-06 18:16:09]

Thanks for doing this!  This is a pretty good sized CL (Though, admittedly, most
changes should be pretty trivial), and I have a fair bit in my queue, so while
I'll try to get to this tomorrow, it's possible I won't get to it until
Thursday.  Apologies for slowness.

[eroman, 2016-09-06 18:42:46]

eroman@chromium.org changed reviewers:
- mmenke@chromium.org

[eroman, 2016-09-06 18:42:47]

mmenke: I can review this (moving you to cc list)

[eroman, 2016-09-06 19:32:21]

Thanks, this change looks good!

* It doesn't look like you have signed the CLA yet -- please find the
instructions at https://www.chromium.org/developers/contributing-code (under
"legal stuff").

* You should also add your name to src/AUTHORS (as part of this changelist).

* After addressing my comments and I have approved, we can "TBR" the changes
outside of //net


Let me know when these items are complete.

https://codereview.chromium.org/2315613002/diff/1/chrome/browser/renderer_hos...
File chrome/browser/renderer_host/safe_browsing_resource_throttle.cc (right):

https://codereview.chromium.org/2315613002/diff/1/chrome/browser/renderer_hos...
chrome/browser/renderer_host/safe_browsing_resource_throttle.cc:116: // Note on
net_log calls: TYPE_SAFE_BROWSING_DEFERRED events must be wholly
Please update this comment to reference the new event name.

https://codereview.chromium.org/2315613002/diff/1/chrome/browser/renderer_hos...
chrome/browser/renderer_host/safe_browsing_resource_throttle.cc:117: // nested
within TYPE_SAFE_BROWSING_CHECKING_URL events.  Synchronous checks
Same here.

https://codereview.chromium.org/2315613002/diff/1/net/http/http_cache_unittes...
File net/http/http_cache_unittest.cc (right):

https://codereview.chromium.org/2315613002/diff/1/net/http/http_cache_unittes...
net/http/http_cache_unittest.cc:596: // test. Note that
TYPE_HTTP_CACHE_WRITE_INFO and TYPE_HTTP_CACHE_*_DATA are
Please update these comments.

https://codereview.chromium.org/2315613002/diff/1/net/log/net_log_event_type_...
File net/log/net_log_event_type_list.h (right):

https://codereview.chromium.org/2315613002/diff/1/net/log/net_log_event_type_...
net/log/net_log_event_type_list.h:478: EVENT_TYPE(SSL_PRIVATE_KEY_OP)
Why this change?

I am assuming this is to prevent a macro collision with OpenSSL:

https://cs.chromium.org/chromium/src/third_party/boringssl/src/include/openss...

For ther record, I confirmed that there aren't any dependencies in the [netlog
viewer] code that depend on the old naming so this should be fine.

https://codereview.chromium.org/2315613002/diff/1/net/proxy/proxy_service_uni...
File net/proxy/proxy_service_unittest.cc (right):

https://codereview.chromium.org/2315613002/diff/1/net/proxy/proxy_service_uni...
net/proxy/proxy_service_unittest.cc:2252: // Note that
TYPE_PROXY_SERVICE_WAITING_FOR_INIT_PAC is never completed before
Please update this comment.

https://codereview.chromium.org/2315613002/diff/1/net/socket/ssl_client_socke...
File net/socket/ssl_client_socket_impl.cc (right):

https://codereview.chromium.org/2315613002/diff/1/net/socket/ssl_client_socke...
net/socket/ssl_client_socket_impl.cc:2236:
net_log_.EndEventWithNetErrorCode(NetLogEventType::SSL_PRIVATE_KEY_OP,
Same question as earlier.
This name change is fine, but please mention it in the CL description.

https://codereview.chromium.org/2315613002/diff/1/net/spdy/spdy_session_unitt...
File net/spdy/spdy_session_unittest.cc (right):

https://codereview.chromium.org/2315613002/diff/1/net/spdy/spdy_session_unitt...
net/spdy/spdy_session_unittest.cc:1539: // Check that we logged
TYPE_HTTP2_SESSION_INITIALIZED correctly.
Same comment here.

[tfarina, 2016-09-06 19:49:13]

Could you copy chromium-reviews@chromium.org as well?

[mikecirone, 2016-09-07 00:07:01]

Description was changed from

==========
Extracted NetLog class's inner enum types into their own enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

BUG=473710
==========

to

==========
Extracted NetLog class's inner enum types into their own enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

Here are a couple historical notes:

1) In refactoring "NetLog::TYPE_SSL_PRIVATE_KEY_OPERATION" to
"NetLogEventType::SSL_PRIVATE_KEY_OPERATION", the removal of "TYPE_" from
"TYPE_SSL_PRIVATE_KEY_OPERATION" created a compiler error when compiling
an ssl-based object file because it turns out that SSL_PRIVATE_KEY_OPERATION
is a macro definition in the ssl translation unit; this resulted in
a macro collision. To resolve, "NetLogEventType::SSL_PRIVATE_KEY_OPERATION"
was changed to "NetLogEventType::SSL_PRIVATE_KEY_OP".

2) The new enum classes, as opposed to enums, made it so that
[NetLogEnumClass]::LABEL needed to be statically cast to int at times.
-e.g.- In NetLog::GetEventTypesAsValue()'s for loop to
       iterate through all enums.
       - Also had to do this in 2 or 3 instances outside of net_log.cc.

BUG=473710
==========

[mikecirone, 2016-09-07 00:21:30]

Description was changed from

==========
Extracted NetLog class's inner enum types into their own enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

Here are a couple historical notes:

1) In refactoring "NetLog::TYPE_SSL_PRIVATE_KEY_OPERATION" to
"NetLogEventType::SSL_PRIVATE_KEY_OPERATION", the removal of "TYPE_" from
"TYPE_SSL_PRIVATE_KEY_OPERATION" created a compiler error when compiling
an ssl-based object file because it turns out that SSL_PRIVATE_KEY_OPERATION
is a macro definition in the ssl translation unit; this resulted in
a macro collision. To resolve, "NetLogEventType::SSL_PRIVATE_KEY_OPERATION"
was changed to "NetLogEventType::SSL_PRIVATE_KEY_OP".

2) The new enum classes, as opposed to enums, made it so that
[NetLogEnumClass]::LABEL needed to be statically cast to int at times.
-e.g.- In NetLog::GetEventTypesAsValue()'s for loop to
       iterate through all enums.
       - Also had to do this in 2 or 3 instances outside of net_log.cc.

BUG=473710
==========

to

==========
Extracted NetLog class's inner enum types into their own enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

Here are a couple historical notes:

1) In refactoring "NetLog::TYPE_SSL_PRIVATE_KEY_OPERATION" to
"NetLogEventType::SSL_PRIVATE_KEY_OPERATION", the removal of "TYPE_" from
"TYPE_SSL_PRIVATE_KEY_OPERATION" created a compiler error when compiling
an ssl-based object file because it turns out that SSL_PRIVATE_KEY_OPERATION
is a macro definition in the ssl translation unit; this resulted in
a macro collision. To resolve, "NetLogEventType::SSL_PRIVATE_KEY_OPERATION"
was changed to "NetLogEventType::SSL_PRIVATE_KEY_OP".
    - There aren't any dependencies in the [netlog viewer] code
       that depend on the old naming so this should be fine.

2) The new enum classes, as opposed to enums, made it so that
[NetLogEnumClass]::LABEL needed to be statically cast to int at times.
-e.g.- In NetLog::GetEventTypesAsValue()'s for loop to
       iterate through all enums.
       - Also had to do this in 2 or 3 instances outside of net_log.cc.

BUG=473710
==========

[mikecirone, 2016-09-07 01:25:34]

On 2016/09/06 19:32:21, eroman wrote:
> Thanks, this change looks good!
> 
> * It doesn't look like you have signed the CLA yet -- please find the
> instructions at https://www.chromium.org/developers/contributing-code (under
> "legal stuff").
> 
> * You should also add your name to src/AUTHORS (as part of this changelist).
> 
> * After addressing my comments and I have approved, we can "TBR" the changes
> outside of //net
> 
> 
> Let me know when these items are complete.
> 
>
https://codereview.chromium.org/2315613002/diff/1/chrome/browser/renderer_hos...
> File chrome/browser/renderer_host/safe_browsing_resource_throttle.cc (right):
> 
>
https://codereview.chromium.org/2315613002/diff/1/chrome/browser/renderer_hos...
> chrome/browser/renderer_host/safe_browsing_resource_throttle.cc:116: // Note
on
> net_log calls: TYPE_SAFE_BROWSING_DEFERRED events must be wholly
> Please update this comment to reference the new event name.
> 
>
https://codereview.chromium.org/2315613002/diff/1/chrome/browser/renderer_hos...
> chrome/browser/renderer_host/safe_browsing_resource_throttle.cc:117: // nested
> within TYPE_SAFE_BROWSING_CHECKING_URL events.  Synchronous checks
> Same here.
> 
>
https://codereview.chromium.org/2315613002/diff/1/net/http/http_cache_unittes...
> File net/http/http_cache_unittest.cc (right):
> 
>
https://codereview.chromium.org/2315613002/diff/1/net/http/http_cache_unittes...
> net/http/http_cache_unittest.cc:596: // test. Note that
> TYPE_HTTP_CACHE_WRITE_INFO and TYPE_HTTP_CACHE_*_DATA are
> Please update these comments.
> 
>
https://codereview.chromium.org/2315613002/diff/1/net/log/net_log_event_type_...
> File net/log/net_log_event_type_list.h (right):
> 
>
https://codereview.chromium.org/2315613002/diff/1/net/log/net_log_event_type_...
> net/log/net_log_event_type_list.h:478: EVENT_TYPE(SSL_PRIVATE_KEY_OP)
> Why this change?
> 
> I am assuming this is to prevent a macro collision with OpenSSL:
> 
>
https://cs.chromium.org/chromium/src/third_party/boringssl/src/include/openss...
> 
> For ther record, I confirmed that there aren't any dependencies in the [netlog
> viewer] code that depend on the old naming so this should be fine.
> 
>
https://codereview.chromium.org/2315613002/diff/1/net/proxy/proxy_service_uni...
> File net/proxy/proxy_service_unittest.cc (right):
> 
>
https://codereview.chromium.org/2315613002/diff/1/net/proxy/proxy_service_uni...
> net/proxy/proxy_service_unittest.cc:2252: // Note that
> TYPE_PROXY_SERVICE_WAITING_FOR_INIT_PAC is never completed before
> Please update this comment.
> 
>
https://codereview.chromium.org/2315613002/diff/1/net/socket/ssl_client_socke...
> File net/socket/ssl_client_socket_impl.cc (right):
> 
>
https://codereview.chromium.org/2315613002/diff/1/net/socket/ssl_client_socke...
> net/socket/ssl_client_socket_impl.cc:2236:
> net_log_.EndEventWithNetErrorCode(NetLogEventType::SSL_PRIVATE_KEY_OP,
> Same question as earlier.
> This name change is fine, but please mention it in the CL description.
> 
>
https://codereview.chromium.org/2315613002/diff/1/net/spdy/spdy_session_unitt...
> File net/spdy/spdy_session_unittest.cc (right):
> 
>
https://codereview.chromium.org/2315613002/diff/1/net/spdy/spdy_session_unitt...
> net/spdy/spdy_session_unittest.cc:1539: // Check that we logged
> TYPE_HTTP2_SESSION_INITIALIZED correctly.
> Same comment here.

Okay, the items are complete.

I applied the changes and updated the CL description to record the change of 
TYPE_SSL_PRIVATE_KEY_OPERATION to SSL_PRIVATE_KEY_OP.

I also added another CL description note, which is about how the enum instances
are now having
to be statically cast to int, at times.

[tfarina, 2016-09-07 19:25:26]

I think Eric will do the real review.

From a couple of files I looked at, it seems correct. I would say as this is
pretty much mechanic, as soon as this passes the trybots and net owner approval,
this will be ready to go.

I only have two tiny nits below, as I can't spare too much time on chromium
anymore.

https://codereview.chromium.org/2315613002/diff/20001/net/log/net_log_event_t...
File net/log/net_log_event_type.h (right):

https://codereview.chromium.org/2315613002/diff/20001/net/log/net_log_event_t...
net/log/net_log_event_type.h:25: }
nit: }  // namespace net

https://codereview.chromium.org/2315613002/diff/20001/net/log/net_log_source_...
File net/log/net_log_source_type.h (right):

https://codereview.chromium.org/2315613002/diff/20001/net/log/net_log_source_...
net/log/net_log_source_type.h:18: }
nit: }  // namespace net

[tfarina, 2016-09-07 19:26:03]

Description was changed from

==========
Extracted NetLog class's inner enum types into their own enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

Here are a couple historical notes:

1) In refactoring "NetLog::TYPE_SSL_PRIVATE_KEY_OPERATION" to
"NetLogEventType::SSL_PRIVATE_KEY_OPERATION", the removal of "TYPE_" from
"TYPE_SSL_PRIVATE_KEY_OPERATION" created a compiler error when compiling
an ssl-based object file because it turns out that SSL_PRIVATE_KEY_OPERATION
is a macro definition in the ssl translation unit; this resulted in
a macro collision. To resolve, "NetLogEventType::SSL_PRIVATE_KEY_OPERATION"
was changed to "NetLogEventType::SSL_PRIVATE_KEY_OP".
    - There aren't any dependencies in the [netlog viewer] code
       that depend on the old naming so this should be fine.

2) The new enum classes, as opposed to enums, made it so that
[NetLogEnumClass]::LABEL needed to be statically cast to int at times.
-e.g.- In NetLog::GetEventTypesAsValue()'s for loop to
       iterate through all enums.
       - Also had to do this in 2 or 3 instances outside of net_log.cc.

BUG=473710
==========

to

==========
Extracted NetLog class's inner enum types into their own enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

Here are a couple historical notes:

1) In refactoring "NetLog::TYPE_SSL_PRIVATE_KEY_OPERATION" to
"NetLogEventType::SSL_PRIVATE_KEY_OPERATION", the removal of "TYPE_" from
"TYPE_SSL_PRIVATE_KEY_OPERATION" created a compiler error when compiling
an ssl-based object file because it turns out that SSL_PRIVATE_KEY_OPERATION
is a macro definition in the ssl translation unit; this resulted in
a macro collision. To resolve, "NetLogEventType::SSL_PRIVATE_KEY_OPERATION"
was changed to "NetLogEventType::SSL_PRIVATE_KEY_OP".
    - There aren't any dependencies in the [netlog viewer] code
       that depend on the old naming so this should be fine.

2) The new enum classes, as opposed to enums, made it so that
[NetLogEnumClass]::LABEL needed to be statically cast to int at times.
-e.g.- In NetLog::GetEventTypesAsValue()'s for loop to
       iterate through all enums.
       - Also had to do this in 2 or 3 instances outside of net_log.cc.

BUG=473710
R=eroman@chromium.org,tfarina@chromium.org
==========

[mikecirone, 2016-09-07 20:02:43]

Ok, got it. Thanks for checking things out a bit!

[mikecirone, 2016-09-07 21:35:42]

Uploaded new patch set that adds "// namespace net" to end of new namespace
blocks.

Also replied to/checked as "done" all review items.

Note: Patch set 3 seems to show extra diffs not associated with this issue, due
to a "git rebase-update" done before patch set 3, I think. Only files actually
changed in patch set 3 are "net_log_event_type.h" and "net_log_source_type.h".

https://codereview.chromium.org/2315613002/diff/1/chrome/browser/renderer_hos...
File chrome/browser/renderer_host/safe_browsing_resource_throttle.cc (right):

https://codereview.chromium.org/2315613002/diff/1/chrome/browser/renderer_hos...
chrome/browser/renderer_host/safe_browsing_resource_throttle.cc:116: // Note on
net_log calls: TYPE_SAFE_BROWSING_DEFERRED events must be wholly
On 2016/09/06 19:32:20, eroman wrote:
> Please update this comment to reference the new event name.

Done.

https://codereview.chromium.org/2315613002/diff/1/chrome/browser/renderer_hos...
chrome/browser/renderer_host/safe_browsing_resource_throttle.cc:117: // nested
within TYPE_SAFE_BROWSING_CHECKING_URL events.  Synchronous checks
On 2016/09/06 19:32:20, eroman wrote:
> Same here.

Done.

https://codereview.chromium.org/2315613002/diff/1/net/http/http_cache_unittes...
File net/http/http_cache_unittest.cc (right):

https://codereview.chromium.org/2315613002/diff/1/net/http/http_cache_unittes...
net/http/http_cache_unittest.cc:596: // test. Note that
TYPE_HTTP_CACHE_WRITE_INFO and TYPE_HTTP_CACHE_*_DATA are
On 2016/09/06 19:32:20, eroman wrote:
> Please update these comments.

Done.

https://codereview.chromium.org/2315613002/diff/1/net/log/net_log_event_type_...
File net/log/net_log_event_type_list.h (right):

https://codereview.chromium.org/2315613002/diff/1/net/log/net_log_event_type_...
net/log/net_log_event_type_list.h:478: EVENT_TYPE(SSL_PRIVATE_KEY_OP)
Yes, it prevents macro collision with OpenSSL.

https://codereview.chromium.org/2315613002/diff/1/net/proxy/proxy_service_uni...
File net/proxy/proxy_service_unittest.cc (right):

https://codereview.chromium.org/2315613002/diff/1/net/proxy/proxy_service_uni...
net/proxy/proxy_service_unittest.cc:2252: // Note that
TYPE_PROXY_SERVICE_WAITING_FOR_INIT_PAC is never completed before
On 2016/09/06 19:32:20, eroman wrote:
> Please update this comment.

Done.

https://codereview.chromium.org/2315613002/diff/1/net/socket/ssl_client_socke...
File net/socket/ssl_client_socket_impl.cc (right):

https://codereview.chromium.org/2315613002/diff/1/net/socket/ssl_client_socke...
net/socket/ssl_client_socket_impl.cc:2236:
net_log_.EndEventWithNetErrorCode(NetLogEventType::SSL_PRIVATE_KEY_OP,
On 2016/09/06 19:32:20, eroman wrote:
> Same question as earlier.
> This name change is fine, but please mention it in the CL description.

Done.

https://codereview.chromium.org/2315613002/diff/1/net/spdy/spdy_session_unitt...
File net/spdy/spdy_session_unittest.cc (right):

https://codereview.chromium.org/2315613002/diff/1/net/spdy/spdy_session_unitt...
net/spdy/spdy_session_unittest.cc:1539: // Check that we logged
TYPE_HTTP2_SESSION_INITIALIZED correctly.
On 2016/09/06 19:32:20, eroman wrote:
> Same comment here.

Done.

https://codereview.chromium.org/2315613002/diff/20001/net/log/net_log_event_t...
File net/log/net_log_event_type.h (right):

https://codereview.chromium.org/2315613002/diff/20001/net/log/net_log_event_t...
net/log/net_log_event_type.h:25: }
On 2016/09/07 19:25:26, tfarina wrote:
> nit: }  // namespace net

Done.

https://codereview.chromium.org/2315613002/diff/20001/net/log/net_log_source_...
File net/log/net_log_source_type.h (right):

https://codereview.chromium.org/2315613002/diff/20001/net/log/net_log_source_...
net/log/net_log_source_type.h:18: }
On 2016/09/07 19:25:26, tfarina wrote:
> nit: }  // namespace net

Done.

[eroman, 2016-09-07 22:00:39]

lgtm

[eroman, 2016-09-07 22:14:50]

Description was changed from

==========
Extracted NetLog class's inner enum types into their own enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

Here are a couple historical notes:

1) In refactoring "NetLog::TYPE_SSL_PRIVATE_KEY_OPERATION" to
"NetLogEventType::SSL_PRIVATE_KEY_OPERATION", the removal of "TYPE_" from
"TYPE_SSL_PRIVATE_KEY_OPERATION" created a compiler error when compiling
an ssl-based object file because it turns out that SSL_PRIVATE_KEY_OPERATION
is a macro definition in the ssl translation unit; this resulted in
a macro collision. To resolve, "NetLogEventType::SSL_PRIVATE_KEY_OPERATION"
was changed to "NetLogEventType::SSL_PRIVATE_KEY_OP".
    - There aren't any dependencies in the [netlog viewer] code
       that depend on the old naming so this should be fine.

2) The new enum classes, as opposed to enums, made it so that
[NetLogEnumClass]::LABEL needed to be statically cast to int at times.
-e.g.- In NetLog::GetEventTypesAsValue()'s for loop to
       iterate through all enums.
       - Also had to do this in 2 or 3 instances outside of net_log.cc.

BUG=473710
R=eroman@chromium.org,tfarina@chromium.org
==========

to

==========
Extracted NetLog class's inner enum types into their own enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

Here are a couple historical notes:

1) In refactoring "NetLog::TYPE_SSL_PRIVATE_KEY_OPERATION" to
"NetLogEventType::SSL_PRIVATE_KEY_OPERATION", the removal of "TYPE_" from
"TYPE_SSL_PRIVATE_KEY_OPERATION" created a compiler error when compiling
an ssl-based object file because it turns out that SSL_PRIVATE_KEY_OPERATION
is a macro definition in the ssl translation unit; this resulted in
a macro collision. To resolve, "NetLogEventType::SSL_PRIVATE_KEY_OPERATION"
was changed to "NetLogEventType::SSL_PRIVATE_KEY_OP".
    - There aren't any dependencies in the [netlog viewer] code
       that depend on the old naming so this should be fine.

2) The new enum classes, as opposed to enums, made it so that
[NetLogEnumClass]::LABEL needed to be statically cast to int at times.
-e.g.- In NetLog::GetEventTypesAsValue()'s for loop to
       iterate through all enums.
       - Also had to do this in 2 or 3 instances outside of net_log.cc.

BUG=473710
R=eroman@chromium.org,tfarina@chromium.org
TBR=asargent@chromium.org,thestig@chromium.org,bengr@chromium.org,avi@chromiu...
==========

[eroman, 2016-09-07 22:15:54]

Adding TBRs for the files outside of //net, which are just simple renames as
follows:

asargent@chromium.org:
  src/chrome/browser/extensions/
  src/extensions/browser/

thestig@chromium.org:
  src/chrome/browser/renderer_host/

bengr@chromium.org:
  src/components/data_reduction_proxy/

avi@chromium.org:
  src/content/browser/

zea@chromium.org:
  src/google_apis/gcm/engine/

rohitrao@chromium.org:
  src/ios/chrome/browser/

sergeyu@chromium.org:
  src/jingle/glue/

[eroman, 2016-09-07 22:16:09]

The CQ bit was checked by eroman@chromium.org

[commit-bot: I haz the power, 2016-09-07 22:16:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-07 23:10:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-07 23:10:08]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[mikecirone, 2016-09-08 00:43:02]

Hmm, curious that the old "TYPE_" naming straggled through, as seen in the
failed windows compilations. I'll (1) fix that one instance that the compiler
errors are pointing at and (2) do some digging to see if I can find any others.

As for the Linux Android issue, I saw and fixed the same issue in a different
place when compiling on my own Mac machine. The (3) fix is to static_cast the
enum to int; then the enum will work with the << operator.

Should I just do items (1-3) so that we can then throw them back at the try
bots?

Or, if desired, I could set up Chromium on my Windows and Linux machines and
work towards getting this cl to compile on Windows and Linux Android. I might
have some trouble doing this for Linux Android, though; I am set up with Arch
Linux and not the more Chromium-friendly Ubuntu.

Please let me know how to proceed.

[eroman, 2016-09-08 02:22:56]

Yes go ahead and make the speculative fixes.
We can send it to the trybots for confirmation.

[mikecirone, 2016-09-08 04:14:09]

Ok! I've done the fixes.

I regex-dug around the codebase with this:
NetLog\w*::(?:TYPE|SOURCE|PHASE)
And I found that lone, old enum. It is fixed now.

I also regex'd for replications of the Linux Android issue,
where I'd need to do the static_cast<int>'ing,
and I couldn't find any besides the places I already
found and fixed while doing my own compilation.

So, looks like we're good!

[eroman, 2016-09-08 05:23:34]

The CQ bit was checked by eroman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 05:23:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[eroman, 2016-09-08 05:26:19]

Description was changed from

==========
Extracted NetLog class's inner enum types into their own enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

Here are a couple historical notes:

1) In refactoring "NetLog::TYPE_SSL_PRIVATE_KEY_OPERATION" to
"NetLogEventType::SSL_PRIVATE_KEY_OPERATION", the removal of "TYPE_" from
"TYPE_SSL_PRIVATE_KEY_OPERATION" created a compiler error when compiling
an ssl-based object file because it turns out that SSL_PRIVATE_KEY_OPERATION
is a macro definition in the ssl translation unit; this resulted in
a macro collision. To resolve, "NetLogEventType::SSL_PRIVATE_KEY_OPERATION"
was changed to "NetLogEventType::SSL_PRIVATE_KEY_OP".
    - There aren't any dependencies in the [netlog viewer] code
       that depend on the old naming so this should be fine.

2) The new enum classes, as opposed to enums, made it so that
[NetLogEnumClass]::LABEL needed to be statically cast to int at times.
-e.g.- In NetLog::GetEventTypesAsValue()'s for loop to
       iterate through all enums.
       - Also had to do this in 2 or 3 instances outside of net_log.cc.

BUG=473710
R=eroman@chromium.org,tfarina@chromium.org
TBR=asargent@chromium.org,thestig@chromium.org,bengr@chromium.org,avi@chromiu...
==========

to

==========
Extracted NetLog class's inner enum types into their own enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

Here are a couple historical notes:

1) In refactoring "NetLog::TYPE_SSL_PRIVATE_KEY_OPERATION" to
"NetLogEventType::SSL_PRIVATE_KEY_OPERATION", the removal of "TYPE_" from
"TYPE_SSL_PRIVATE_KEY_OPERATION" created a compiler error when compiling
an ssl-based object file because it turns out that SSL_PRIVATE_KEY_OPERATION
is a macro definition in the ssl translation unit; this resulted in
a macro collision. To resolve, "NetLogEventType::SSL_PRIVATE_KEY_OPERATION"
was changed to "NetLogEventType::SSL_PRIVATE_KEY_OP".
    - There aren't any dependencies in the [netlog viewer] code
       that depend on the old naming so this should be fine.

2) The new enum classes, as opposed to enums, made it so that
[NetLogEnumClass]::LABEL needed to be statically cast to int at times.
-e.g.- In NetLog::GetEventTypesAsValue()'s for loop to
       iterate through all enums.
       - Also had to do this in 2 or 3 instances outside of net_log.cc.

BUG=473710
R=eroman@chromium.org,tfarina@chromium.org
TBR=asargent@chromium.org,thestig@chromium.org,bengr@chromium.org,avi@chromiu...
==========

[eroman, 2016-09-08 05:26:45]

https://codereview.chromium.org/2315613002/diff/60001/net/socket/tcp_socket_w...
File net/socket/tcp_socket_win.cc (right):

https://codereview.chromium.org/2315613002/diff/60001/net/socket/tcp_socket_w...
net/socket/tcp_socket_win.cc:694: NetLogEventType::TCP_ACCEPT, net_error);
This formatting looks strange.
Did you run "git cl format" ?

(Same comment throughout this file).

[mikecirone, 2016-09-08 06:39:22]

https://codereview.chromium.org/2315613002/diff/60001/net/socket/tcp_socket_w...
File net/socket/tcp_socket_win.cc (right):

https://codereview.chromium.org/2315613002/diff/60001/net/socket/tcp_socket_w...
net/socket/tcp_socket_win.cc:694: NetLogEventType::TCP_ACCEPT, net_error);
Ok, I just ran "git cl format". Patch set 5 shows da changes.

[eroman, 2016-09-08 16:43:49]

patchset#5 LGTM

[eroman, 2016-09-08 16:44:13]

The CQ bit was checked by eroman@chromium.org

[commit-bot: I haz the power, 2016-09-08 16:44:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-08 19:12:24]

Description was changed from

==========
Extracted NetLog class's inner enum types into their own enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

Here are a couple historical notes:

1) In refactoring "NetLog::TYPE_SSL_PRIVATE_KEY_OPERATION" to
"NetLogEventType::SSL_PRIVATE_KEY_OPERATION", the removal of "TYPE_" from
"TYPE_SSL_PRIVATE_KEY_OPERATION" created a compiler error when compiling
an ssl-based object file because it turns out that SSL_PRIVATE_KEY_OPERATION
is a macro definition in the ssl translation unit; this resulted in
a macro collision. To resolve, "NetLogEventType::SSL_PRIVATE_KEY_OPERATION"
was changed to "NetLogEventType::SSL_PRIVATE_KEY_OP".
    - There aren't any dependencies in the [netlog viewer] code
       that depend on the old naming so this should be fine.

2) The new enum classes, as opposed to enums, made it so that
[NetLogEnumClass]::LABEL needed to be statically cast to int at times.
-e.g.- In NetLog::GetEventTypesAsValue()'s for loop to
       iterate through all enums.
       - Also had to do this in 2 or 3 instances outside of net_log.cc.

BUG=473710
R=eroman@chromium.org,tfarina@chromium.org
TBR=asargent@chromium.org,thestig@chromium.org,bengr@chromium.org,avi@chromiu...
==========

to

==========
Extracted NetLog class's inner enum types into their own enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

Here are a couple historical notes:

1) In refactoring "NetLog::TYPE_SSL_PRIVATE_KEY_OPERATION" to
"NetLogEventType::SSL_PRIVATE_KEY_OPERATION", the removal of "TYPE_" from
"TYPE_SSL_PRIVATE_KEY_OPERATION" created a compiler error when compiling
an ssl-based object file because it turns out that SSL_PRIVATE_KEY_OPERATION
is a macro definition in the ssl translation unit; this resulted in
a macro collision. To resolve, "NetLogEventType::SSL_PRIVATE_KEY_OPERATION"
was changed to "NetLogEventType::SSL_PRIVATE_KEY_OP".
    - There aren't any dependencies in the [netlog viewer] code
       that depend on the old naming so this should be fine.

2) The new enum classes, as opposed to enums, made it so that
[NetLogEnumClass]::LABEL needed to be statically cast to int at times.
-e.g.- In NetLog::GetEventTypesAsValue()'s for loop to
       iterate through all enums.
       - Also had to do this in 2 or 3 instances outside of net_log.cc.

BUG=473710
R=eroman@chromium.org,tfarina@chromium.org
TBR=asargent@chromium.org,thestig@chromium.org,bengr@chromium.org,avi@chromiu...
==========

[commit-bot: I haz the power, 2016-09-08 19:12:25]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-08 19:17:32]

Description was changed from

==========
Extracted NetLog class's inner enum types into their own enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

Here are a couple historical notes:

1) In refactoring "NetLog::TYPE_SSL_PRIVATE_KEY_OPERATION" to
"NetLogEventType::SSL_PRIVATE_KEY_OPERATION", the removal of "TYPE_" from
"TYPE_SSL_PRIVATE_KEY_OPERATION" created a compiler error when compiling
an ssl-based object file because it turns out that SSL_PRIVATE_KEY_OPERATION
is a macro definition in the ssl translation unit; this resulted in
a macro collision. To resolve, "NetLogEventType::SSL_PRIVATE_KEY_OPERATION"
was changed to "NetLogEventType::SSL_PRIVATE_KEY_OP".
    - There aren't any dependencies in the [netlog viewer] code
       that depend on the old naming so this should be fine.

2) The new enum classes, as opposed to enums, made it so that
[NetLogEnumClass]::LABEL needed to be statically cast to int at times.
-e.g.- In NetLog::GetEventTypesAsValue()'s for loop to
       iterate through all enums.
       - Also had to do this in 2 or 3 instances outside of net_log.cc.

BUG=473710
R=eroman@chromium.org,tfarina@chromium.org
TBR=asargent@chromium.org,thestig@chromium.org,bengr@chromium.org,avi@chromiu...
==========

to

==========
Extracted NetLog class's inner enum types into their own enum classes and
refactored codebase accordingly. This is the first step towards achieving
the "Don't use so many inner classes for NetLog" cleanup ticket, which
seeks to make the codebase more "Include what you use" clean.

There are many "net/log/net_log.h" includes in the codebase. Since 'net_log.h'
has many includes itself, this leaves the codebase susceptible to the
undesirable scenario where clients of 'net_log.h' could be relying upon
'net_log.h' not for its contents but its inner includes.

Extracting NetLog's inner classes into their own files will help with the
above issue. For instance, once these inner NetLog classes are extracted:
ThreadSafeObserver, Entry, EntryData, NetLog enum types, then
ThreadSafeObserver clients will not need to include "net_log.h".

Here are a couple historical notes:

1) In refactoring "NetLog::TYPE_SSL_PRIVATE_KEY_OPERATION" to
"NetLogEventType::SSL_PRIVATE_KEY_OPERATION", the removal of "TYPE_" from
"TYPE_SSL_PRIVATE_KEY_OPERATION" created a compiler error when compiling
an ssl-based object file because it turns out that SSL_PRIVATE_KEY_OPERATION
is a macro definition in the ssl translation unit; this resulted in
a macro collision. To resolve, "NetLogEventType::SSL_PRIVATE_KEY_OPERATION"
was changed to "NetLogEventType::SSL_PRIVATE_KEY_OP".
    - There aren't any dependencies in the [netlog viewer] code
       that depend on the old naming so this should be fine.

2) The new enum classes, as opposed to enums, made it so that
[NetLogEnumClass]::LABEL needed to be statically cast to int at times.
-e.g.- In NetLog::GetEventTypesAsValue()'s for loop to
       iterate through all enums.
       - Also had to do this in 2 or 3 instances outside of net_log.cc.

BUG=473710
R=eroman@chromium.org,tfarina@chromium.org
TBR=asargent@chromium.org,thestig@chromium.org,bengr@chromium.org,avi@chromiu...

Committed: https://crrev.com/8b85c43f674d711380f660fea44a19653622ac61
Cr-Commit-Position: refs/heads/master@{#417358}
==========

[commit-bot: I haz the power, 2016-09-08 19:17:34]

Patchset 5 (id:??) landed as
https://crrev.com/8b85c43f674d711380f660fea44a19653622ac61
Cr-Commit-Position: refs/heads/master@{#417358}