[chromedriver] Remove Logger and just use base LOG.

[chromedriver] Remove Logger and just use base LOG.
BUG=none
R=chrisgao@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221521

[kkania, 2013-09-04 23:39:30]

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/capa...
File chrome/test/chromedriver/capabilities.cc (right):

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/capa...
chrome/test/chromedriver/capabilities.cc:226: return Status(kUnknownError,
"capability must be a dictionary");
revert

[chrisgao (Use stgao instead), 2013-09-05 01:20:46]

lgtm with nits.

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
File chrome/test/chromedriver/chrome/adb_impl.cc (right):

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/adb_impl.cc:228: VLOG(1) << "Received adb
response: " << *response;
single line, "{}" may be removed

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
File chrome/test/chromedriver/chrome/adb_impl.h (right):

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/adb_impl.h:19: class Log;
Remove

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
File chrome/test/chromedriver/chrome/devtools_client_impl.cc (right):

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/devtools_client_impl.cc:14: #include
"chrome/test/chromedriver/chrome/log.h"
Removed?

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/devtools_client_impl.cc:206: std::string message
= SerializeValue(&command);
I'm not sure whether we should put SerializeValue in log.h/cc
How about a new util.h/cc?

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/devtools_client_impl.cc:209: <<
SerializeValue(&params);
For javascript, this could print a bunch of log.

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/devtools_client_impl.cc:289: VLOG(1) <<
"Received " << event.method << " "
"Received " -> "Received event "?

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
File chrome/test/chromedriver/chrome/log.cc (right):

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/log.cc:88: json.resize(json.length() - 1);
Why remove the last character?

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/logg...
File chrome/test/chromedriver/logging.cc (right):

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/logg...
chrome/test/chromedriver/logging.cc:154: if (name == kNameToLevel[i].name) {
Can it be case insensitive?

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/logg...
chrome/test/chromedriver/logging.cc:229: cmd_line->AppendSwitchASCII("vmodule",
"*/chrome/test/chromedriver/*=3");
How about adding a comment for this one?

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/logg...
chrome/test/chromedriver/logging.cc:262: WebDriverLog* log = new
WebDriverLog(type, Log::kDebug);
Why not using |level| but Log::kDebug?

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/logg...
chrome/test/chromedriver/logging.cc:290: *out_driver_log = driver_log.Pass();
|out_driver_log| is not contained in/owned by |out_logs|.
But the comment in logging.h says it should be.

[kkania, 2013-09-05 15:15:10]

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/capa...
File chrome/test/chromedriver/capabilities.cc (right):

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/capa...
chrome/test/chromedriver/capabilities.cc:226: return Status(kUnknownError,
"capability must be a dictionary");
On 2013/09/04 23:39:30, kkania wrote:
> revert

Done.

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
File chrome/test/chromedriver/chrome/adb_impl.cc (right):

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/adb_impl.cc:228: VLOG(1) << "Received adb
response: " << *response;
On 2013/09/05 01:20:46, chrisgao wrote:
> single line, "{}" may be removed

usually its customary to do {}, because macros may expand to multiple lines

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
File chrome/test/chromedriver/chrome/adb_impl.h (right):

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/adb_impl.h:19: class Log;
On 2013/09/05 01:20:46, chrisgao wrote:
> Remove

Done.

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
File chrome/test/chromedriver/chrome/devtools_client_impl.cc (right):

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/devtools_client_impl.cc:14: #include
"chrome/test/chromedriver/chrome/log.h"
On 2013/09/05 01:20:46, chrisgao wrote:
> Removed?

Need this for IsVLogOn

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/devtools_client_impl.cc:206: std::string message
= SerializeValue(&command);
On 2013/09/05 01:20:46, chrisgao wrote:
> I'm not sure whether we should put SerializeValue in log.h/cc
> How about a new util.h/cc?

Done.

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/devtools_client_impl.cc:209: <<
SerializeValue(&params);
On 2013/09/05 01:20:46, chrisgao wrote:
> For javascript, this could print a bunch of log.

Done.

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/devtools_client_impl.cc:289: VLOG(1) <<
"Received " << event.method << " "
On 2013/09/05 01:20:46, chrisgao wrote:
> "Received " -> "Received event "?

changed to EVENT

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
File chrome/test/chromedriver/chrome/log.cc (right):

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/log.cc:88: json.resize(json.length() - 1);
On 2013/09/05 01:20:46, chrisgao wrote:
> Why remove the last character?

it's a newline that we don't want, added comment

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/logg...
File chrome/test/chromedriver/logging.cc (right):

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/logg...
chrome/test/chromedriver/logging.cc:154: if (name == kNameToLevel[i].name) {
On 2013/09/05 01:20:46, chrisgao wrote:
> Can it be case insensitive?

the spec isn't clear; i'll leave as is.

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/logg...
chrome/test/chromedriver/logging.cc:229: cmd_line->AppendSwitchASCII("vmodule",
"*/chrome/test/chromedriver/*=3");
On 2013/09/05 01:20:46, chrisgao wrote:
> How about adding a comment for this one?

Done.

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/logg...
chrome/test/chromedriver/logging.cc:262: WebDriverLog* log = new
WebDriverLog(type, Log::kDebug);
On 2013/09/05 01:20:46, chrisgao wrote:
> Why not using |level| but Log::kDebug?

the thinking was that perf logging doesn't really have levels, so it's just an
on/off switch. I'll change it to All to make it more clear.

https://codereview.chromium.org/23566018/diff/1/chrome/test/chromedriver/logg...
chrome/test/chromedriver/logging.cc:290: *out_driver_log = driver_log.Pass();
On 2013/09/05 01:20:46, chrisgao wrote:
> |out_driver_log| is not contained in/owned by |out_logs|.
> But the comment in logging.h says it should be.

Updated comment.

[kkania, 2013-09-05 21:21:44]

Committed patchset #3 manually as r221521 (presubmit successful).