Blimp: change logger to use switch/case for types.

Blimp: change logger to use switch/case for types.

The current design of the logger allows for logging "modules" to
be loaded into a dynamic map. This functionality is not providing
any benefit at the moment, and it prevents the compiler from performing
enum coverage checks at compile time. This CL changes the dynamic
behavior to use a switch block instead.

R=wez@chromium.org
BUG=

Committed: https://crrev.com/d008c80b9f35110a085fba5970ddc0d96877851b
Cr-Commit-Position: refs/heads/master@{#398433}

[Wez, 2016-06-01 21:35:15]

https://codereview.chromium.org/2026053002/diff/1/blimp/common/logging.cc
File blimp/common/logging.cc (right):

https://codereview.chromium.org/2026053002/diff/1/blimp/common/logging.cc#new...
blimp/common/logging.cc:73: static void ExtractFields(const BlimpMessage&
message, LogFields* output) {
Looks like this, and the other static member methods below, can just become
static LogFooFields functions in this .cc?

[Kevin M, 2016-06-02 18:05:00]

https://codereview.chromium.org/2026053002/diff/1/blimp/common/logging.cc
File blimp/common/logging.cc (right):

https://codereview.chromium.org/2026053002/diff/1/blimp/common/logging.cc#new...
blimp/common/logging.cc:73: static void ExtractFields(const BlimpMessage&
message, LogFields* output) {
On 2016/06/01 21:35:15, Wez wrote:
> Looks like this, and the other static member methods below, can just become
> static LogFooFields functions in this .cc?

Done.

[Wez, 2016-06-03 00:07:54]

LGTM w/ nits

https://codereview.chromium.org/2026053002/diff/40001/blimp/common/logging.cc
File blimp/common/logging.cc (right):

https://codereview.chromium.org/2026053002/diff/40001/blimp/common/logging.cc...
blimp/common/logging.cc:25: typedef std::vector<std::pair<std::string,
std::string>> LogFields;
nit: Style-guide says using using instead of typedef now.

https://codereview.chromium.org/2026053002/diff/40001/blimp/common/logging.cc...
blimp/common/logging.cc:112: default:  // unknown
Replace the default's, here and below, with explicit UNKNOWN case, like we have
at the top-level, so we can trap missing handlers for new message types too.

https://codereview.chromium.org/2026053002/diff/40001/blimp/common/logging.cc...
blimp/common/logging.cc:262: fields.push_back(std::make_pair("type",
"COMPOSITOR"));
What was wrong with AddField() here? Seems shorter and easier to grok,
especially since it's used in the logger functions.

https://codereview.chromium.org/2026053002/diff/40001/blimp/common/logging.cc...
blimp/common/logging.cc:297: fields.push_back(std::make_pair("type",
"<UNKNOWN>"));
Off-topic: This is an interesting limitation of using oneof - no ability to
print out what type a message from a newer sender was.

[Kevin M, 2016-06-07 00:05:51]

https://codereview.chromium.org/2026053002/diff/40001/blimp/common/logging.cc
File blimp/common/logging.cc (right):

https://codereview.chromium.org/2026053002/diff/40001/blimp/common/logging.cc...
blimp/common/logging.cc:25: typedef std::vector<std::pair<std::string,
std::string>> LogFields;
On 2016/06/03 00:07:54, Wez wrote:
> nit: Style-guide says using using instead of typedef now.

Done.

https://codereview.chromium.org/2026053002/diff/40001/blimp/common/logging.cc...
blimp/common/logging.cc:112: default:  // unknown
On 2016/06/03 00:07:54, Wez wrote:
> Replace the default's, here and below, with explicit UNKNOWN case, like we
have
> at the top-level, so we can trap missing handlers for new message types too.

Done.

https://codereview.chromium.org/2026053002/diff/40001/blimp/common/logging.cc...
blimp/common/logging.cc:262: fields.push_back(std::make_pair("type",
"COMPOSITOR"));
On 2016/06/03 00:07:54, Wez wrote:
> What was wrong with AddField() here? Seems shorter and easier to grok,
> especially since it's used in the logger functions.

Done.

https://codereview.chromium.org/2026053002/diff/40001/blimp/common/logging.cc...
blimp/common/logging.cc:297: fields.push_back(std::make_pair("type",
"<UNKNOWN>"));
On 2016/06/03 00:07:54, Wez wrote:
> Off-topic: This is an interesting limitation of using oneof - no ability to
> print out what type a message from a newer sender was.

Yeah, there's no analogue to unknown_fields :(

[Kevin M, 2016-06-07 00:35:24]

The CQ bit was checked by kmarshall@chromium.org

[Kevin M, 2016-06-07 00:35:24]

The patchset sent to the CQ was uploaded after l-g-t-m from wez@chromium.org
Link to the patchset: https://codereview.chromium.org/2026053002/#ps60001
(title: "wez feedback")

[commit-bot: I haz the power, 2016-06-07 00:35:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2026053002/60001

[commit-bot: I haz the power, 2016-06-07 00:38:35]

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

[commit-bot: I haz the power, 2016-06-07 00:38:37]

Try jobs failed on following builders:
  ios-device on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)

[Kevin M, 2016-06-07 23:42:52]

The CQ bit was checked by kmarshall@chromium.org

[Kevin M, 2016-06-07 23:42:52]

The patchset sent to the CQ was uploaded after l-g-t-m from wez@chromium.org
Link to the patchset: https://codereview.chromium.org/2026053002/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-06-07 23:43:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2026053002/80001

[commit-bot: I haz the power, 2016-06-08 00:28:20]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-08 00:29:36]

Description was changed from

==========
Blimp: change logger to use switch/case for types.

The current design of the logger allows for logging "modules" to
be loaded into a dynamic map. This functionality is not providing
any benefit at the moment, and it prevents the compiler from performing
enum coverage checks at compile time. This CL changes the dynamic
behavior to use a switch block instead.

R=wez@chromium.org
BUG=
==========

to

==========
Blimp: change logger to use switch/case for types.

The current design of the logger allows for logging "modules" to
be loaded into a dynamic map. This functionality is not providing
any benefit at the moment, and it prevents the compiler from performing
enum coverage checks at compile time. This CL changes the dynamic
behavior to use a switch block instead.

R=wez@chromium.org
BUG=

Committed: https://crrev.com/d008c80b9f35110a085fba5970ddc0d96877851b
Cr-Commit-Position: refs/heads/master@{#398433}
==========

[commit-bot: I haz the power, 2016-06-08 00:29:37]

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