diff options
author | stevenjb@chromium.org <stevenjb@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-05-16 21:56:08 +0000 |
---|---|---|
committer | stevenjb@chromium.org <stevenjb@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-05-16 21:56:08 +0000 |
commit | 5092ea8719a8f0d64ca808e3ccbbf1b310987f92 (patch) | |
tree | ef55ba03c4710e6890ed57f351c3b87793ab722a /chromeos | |
parent | 77d6e643095351e50717b44d9c69823431404a3d (diff) | |
download | chromium_src-5092ea8719a8f0d64ca808e3ccbbf1b310987f92.zip chromium_src-5092ea8719a8f0d64ca808e3ccbbf1b310987f92.tar.gz chromium_src-5092ea8719a8f0d64ca808e3ccbbf1b310987f92.tar.bz2 |
Re-factor network_event_log
BUG=239652
For webui:
TBR=gauravsh@chromium.org, pneubeck@chromium.org
Review URL: https://codereview.chromium.org/14876021
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@200632 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chromeos')
-rw-r--r-- | chromeos/network/managed_network_configuration_handler.cc | 4 | ||||
-rw-r--r-- | chromeos/network/managed_state.cc | 9 | ||||
-rw-r--r-- | chromeos/network/network_configuration_handler.cc | 12 | ||||
-rw-r--r-- | chromeos/network/network_connection_handler.cc | 12 | ||||
-rw-r--r-- | chromeos/network/network_event_log.cc | 182 | ||||
-rw-r--r-- | chromeos/network/network_event_log.h | 80 | ||||
-rw-r--r-- | chromeos/network/network_event_log_unittest.cc | 183 | ||||
-rw-r--r-- | chromeos/network/network_handler_callbacks.cc | 13 | ||||
-rw-r--r-- | chromeos/network/network_handler_callbacks.h | 3 | ||||
-rw-r--r-- | chromeos/network/network_state.cc | 28 | ||||
-rw-r--r-- | chromeos/network/network_state_handler.cc | 110 | ||||
-rw-r--r-- | chromeos/network/network_state_handler.h | 4 | ||||
-rw-r--r-- | chromeos/network/network_state_handler_unittest.cc | 2 | ||||
-rw-r--r-- | chromeos/network/onc/onc_certificate_importer.cc | 16 | ||||
-rw-r--r-- | chromeos/network/shill_property_handler.cc | 45 | ||||
-rw-r--r-- | chromeos/network/shill_property_handler.h | 5 | ||||
-rw-r--r-- | chromeos/network/shill_property_handler_unittest.cc | 2 |
17 files changed, 481 insertions, 229 deletions
diff --git a/chromeos/network/managed_network_configuration_handler.cc b/chromeos/network/managed_network_configuration_handler.cc index 122bdcd..bf44b9f 100644 --- a/chromeos/network/managed_network_configuration_handler.cc +++ b/chromeos/network/managed_network_configuration_handler.cc @@ -43,8 +43,6 @@ namespace chromeos { namespace { -const char kLogModule[] = "ManagedNetworkConfigurationHandler"; - // These are error strings used for error callbacks. None of these error // messages are user-facing: they should only appear in logs. const char kInvalidUserSettingsMessage[] = "User settings are invalid."; @@ -78,7 +76,7 @@ void RunErrorCallback(const std::string& service_path, const std::string& error_name, const std::string& error_message, const network_handler::ErrorCallback& error_callback) { - network_event_log::AddEntry(kLogModule, error_name, error_message); + NET_LOG_ERROR(error_name, error_message); error_callback.Run( error_name, make_scoped_ptr( diff --git a/chromeos/network/managed_state.cc b/chromeos/network/managed_state.cc index a4b8de5..b91d4a0 100644 --- a/chromeos/network/managed_state.cc +++ b/chromeos/network/managed_state.cc @@ -7,6 +7,7 @@ #include "base/logging.h" #include "base/values.h" #include "chromeos/network/device_state.h" +#include "chromeos/network/network_event_log.h" #include "chromeos/network/network_state.h" #include "third_party/cros_system_api/dbus/service_constants.h" @@ -22,7 +23,7 @@ ManagedState::~ManagedState() { } ManagedState* ManagedState::Create(ManagedType type, const std::string& path) { - switch(type) { + switch (type) { case MANAGED_TYPE_NETWORK: return new NetworkState(path); case MANAGED_TYPE_DEVICE: @@ -61,7 +62,7 @@ bool ManagedState::GetBooleanValue(const std::string& key, bool* out_value) { bool new_value; if (!value.GetAsBoolean(&new_value)) { - LOG(WARNING) << "Failed to parse boolean value for:" << key; + NET_LOG_ERROR("Error parsing state value", path() + "." + key); return false; } if (*out_value == new_value) @@ -75,7 +76,7 @@ bool ManagedState::GetIntegerValue(const std::string& key, int* out_value) { int new_value; if (!value.GetAsInteger(&new_value)) { - LOG(WARNING) << "Failed to parse integer value for:" << key; + NET_LOG_ERROR("Error parsing state value", path() + "." + key); return false; } if (*out_value == new_value) @@ -89,7 +90,7 @@ bool ManagedState::GetStringValue(const std::string& key, std::string* out_value) { std::string new_value; if (!value.GetAsString(&new_value)) { - LOG(WARNING) << "Failed to parse string value for:" << key; + NET_LOG_ERROR("Error parsing state value", path() + "." + key); return false; } if (*out_value == new_value) diff --git a/chromeos/network/network_configuration_handler.cc b/chromeos/network/network_configuration_handler.cc index b38fb56..22a8998b 100644 --- a/chromeos/network/network_configuration_handler.cc +++ b/chromeos/network/network_configuration_handler.cc @@ -23,8 +23,6 @@ namespace chromeos { namespace { -const char kLogModule[] = "NetworkConfigurationHandler"; - NetworkConfigurationHandler* g_configuration_handler_instance = NULL; // None of these error messages are user-facing: they should only appear in @@ -148,7 +146,7 @@ void NetworkConfigurationHandler::SetProperties( properties, base::Bind(&IgnoreObjectPathCallback, callback), base::Bind(&network_handler::ShillErrorCallbackFunction, - kLogModule, service_path, error_callback)); + service_path, error_callback)); } void NetworkConfigurationHandler::ClearProperties( @@ -165,7 +163,7 @@ void NetworkConfigurationHandler::ClearProperties( callback, error_callback), base::Bind(&network_handler::ShillErrorCallbackFunction, - kLogModule, service_path, error_callback)); + service_path, error_callback)); } void NetworkConfigurationHandler::CreateConfiguration( @@ -190,13 +188,13 @@ void NetworkConfigurationHandler::CreateConfiguration( properties, base::Bind(&RunCreateNetworkCallback, callback), base::Bind(&network_handler::ShillErrorCallbackFunction, - kLogModule, "", error_callback)); + "", error_callback)); } else { manager->GetService( properties, base::Bind(&RunCreateNetworkCallback, callback), base::Bind(&network_handler::ShillErrorCallbackFunction, - kLogModule, "", error_callback)); + "", error_callback)); } } @@ -208,7 +206,7 @@ void NetworkConfigurationHandler::RemoveConfiguration( dbus::ObjectPath(service_path), callback, base::Bind(&network_handler::ShillErrorCallbackFunction, - kLogModule, service_path, error_callback)); + service_path, error_callback)); } NetworkConfigurationHandler::NetworkConfigurationHandler() { diff --git a/chromeos/network/network_connection_handler.cc b/chromeos/network/network_connection_handler.cc index 0d98a458..f8035a5 100644 --- a/chromeos/network/network_connection_handler.cc +++ b/chromeos/network/network_connection_handler.cc @@ -26,14 +26,12 @@ namespace chromeos { namespace { -const char kLogModule[] = "NetworkConnectionHandler"; - void InvokeErrorCallback( const std::string& service_path, const network_handler::ErrorCallback& error_callback, const std::string& error_name) { network_handler::ShillErrorCallbackFunction( - kLogModule, service_path, error_callback, error_name, "Connect Error"); + service_path, error_callback, error_name, "Connect Error"); } bool NetworkMayNeedCredentials(const NetworkState* network) { @@ -218,7 +216,7 @@ void NetworkConnectionHandler::CallShillConnect( // TODO(stevenjb): Remove SetConnectingNetwork and use this class to maintain // the connecting network(s) once NetworkLibrary path is eliminated. NetworkStateHandler::Get()->SetConnectingNetwork(service_path); - network_event_log::AddEntry(kLogModule, "Connect Request", service_path); + NET_LOG_EVENT("Connect Request", service_path); DBusThreadManager::Get()->GetShillServiceClient()->Connect( dbus::ObjectPath(service_path), base::Bind(&NetworkConnectionHandler::HandleShillSuccess, @@ -231,7 +229,7 @@ void NetworkConnectionHandler::CallShillDisconnect( const std::string& service_path, const base::Closure& success_callback, const network_handler::ErrorCallback& error_callback) { - network_event_log::AddEntry(kLogModule, "Disconnect Request", service_path); + NET_LOG_EVENT("Disconnect Request", service_path); DBusThreadManager::Get()->GetShillServiceClient()->Disconnect( dbus::ObjectPath(service_path), base::Bind(&NetworkConnectionHandler::HandleShillSuccess, @@ -291,7 +289,7 @@ void NetworkConnectionHandler::HandleShillSuccess( // point (or maybe call it twice, once indicating in-progress, then success // or failure). pending_requests_.erase(service_path); - network_event_log::AddEntry(kLogModule, "Connected", service_path); + NET_LOG_EVENT("Connected", service_path); success_callback.Run(); } @@ -303,7 +301,7 @@ void NetworkConnectionHandler::HandleShillFailure( pending_requests_.erase(service_path); std::string error = "Connect Failure: " + error_name + ": " + error_message; network_handler::ShillErrorCallbackFunction( - kLogModule, service_path, error_callback, error_name, error_message); + service_path, error_callback, error_name, error_message); } } // namespace chromeos diff --git a/chromeos/network/network_event_log.cc b/chromeos/network/network_event_log.cc index 7cb9e84..9fc22a0 100644 --- a/chromeos/network/network_event_log.cc +++ b/chromeos/network/network_event_log.cc @@ -4,11 +4,16 @@ #include "chromeos/network/network_event_log.h" +#include "base/files/file_path.h" #include "base/i18n/time_formatting.h" +#include "base/json/json_writer.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/stringprintf.h" +#include "base/strings/string_tokenizer.h" #include "base/utf_string_conversions.h" +#include "base/values.h" +#include "net/base/escape.h" namespace chromeos { @@ -17,49 +22,132 @@ namespace network_event_log { namespace { struct LogEntry { - LogEntry(const std::string& module, + LogEntry(const std::string& file, + int file_line, + LogLevel log_level, const std::string& event, const std::string& description); - std::string ToString() const; + std::string ToString(bool show_time, + bool show_file, + bool show_desc, + bool format_html) const; + + std::string GetNormalText(bool show_desc) const; + std::string GetHtmlText(bool show_desc) const; + + void SendToVLogOrErrorLog() const; bool ContentEquals(const LogEntry& other) const; - std::string module; + std::string file; + int file_line; + LogLevel log_level; std::string event; std::string description; base::Time time; int count; }; -LogEntry::LogEntry(const std::string& module, +LogEntry::LogEntry(const std::string& file, + int file_line, + LogLevel log_level, const std::string& event, const std::string& description) - : module(module), + : file(file), + file_line(file_line), + log_level(log_level), event(event), description(description), time(base::Time::Now()), count(1) { } -std::string LogEntry::ToString() const { +std::string LogEntry::ToString(bool show_time, + bool show_file, + bool show_desc, + bool format_html) const { std::string line; - line += "[" + UTF16ToUTF8(base::TimeFormatShortDateAndTime(time)) + "]"; - line += " " + module + ":" + event; - if (!description.empty()) - line += ": " + description; + if (show_time) + line += "[" + UTF16ToUTF8(base::TimeFormatShortDateAndTime(time)) + "] "; + if (show_file) { + std::string filestr = format_html ? net::EscapeForHTML(file) : file; + line += base::StringPrintf("%s:%d ", filestr.c_str(), file_line); + } + line += format_html ? GetHtmlText(show_desc) : GetNormalText(show_desc); if (count > 1) line += base::StringPrintf(" (%d)", count); line += "\n"; return line; } +std::string LogEntry::GetNormalText(bool show_desc) const { + std::string text = event; + if (show_desc && !description.empty()) + text += ": " + description; + return text; +} + +std::string LogEntry::GetHtmlText(bool show_desc) const { + std::string text; + if (log_level == LOG_LEVEL_DEBUG) + text += "<i>"; + else if (log_level == LOG_LEVEL_ERROR) + text += "<b>"; + + text += event; + if (show_desc && !description.empty()) + text += ": " + net::EscapeForHTML(description); + + if (log_level == LOG_LEVEL_DEBUG) + text += "</i>"; + else if (log_level == LOG_LEVEL_ERROR) + text += "</b>"; + return text; +} + +void LogEntry::SendToVLogOrErrorLog() const { + const bool show_time = true; + const bool show_file = true; + const bool show_desc = true; + const bool format_html = false; + std::string output = ToString(show_time, show_file, show_desc, format_html); + if (log_level == LOG_LEVEL_ERROR) + LOG(ERROR) << output; + else + VLOG(1) << output; +} + bool LogEntry::ContentEquals(const LogEntry& other) const { - return module == other.module && + return file == other.file && + file_line == other.file_line && event == other.event && description == other.description; } +void GetFormat(const std::string& format_string, + bool* show_time, + bool* show_file, + bool* show_desc, + bool* format_html) { + base::StringTokenizer tokens(format_string, ","); + *show_time = false; + *show_file = false; + *show_desc = false; + *format_html = false; + while (tokens.GetNext()) { + std::string tok(tokens.token()); + if (tok == "time") + *show_time = true; + if (tok == "file") + *show_file = true; + if (tok == "desc") + *show_desc = true; + if (tok == "html") + *format_html = true; + } +} + typedef std::deque<LogEntry> LogEntryList; class NetworkEventLog { @@ -67,9 +155,11 @@ class NetworkEventLog { NetworkEventLog() {} ~NetworkEventLog() {} - void AddEntry(const LogEntry& entry); + void AddLogEntry(const LogEntry& entry); std::string GetAsString(StringOrder order, + const std::string& format, + LogLevel max_level, size_t max_events); private: @@ -78,12 +168,13 @@ class NetworkEventLog { DISALLOW_COPY_AND_ASSIGN(NetworkEventLog); }; -void NetworkEventLog::AddEntry(const LogEntry& entry) { +void NetworkEventLog::AddLogEntry(const LogEntry& entry) { if (!entries_.empty()) { LogEntry& last = entries_.back(); if (last.ContentEquals(entry)) { // Update count and time for identical events to avoid log spam. ++last.count; + last.log_level = std::min(last.log_level, entry.log_level); last.time = base::Time::Now(); return; } @@ -91,29 +182,50 @@ void NetworkEventLog::AddEntry(const LogEntry& entry) { if (entries_.size() >= kMaxNetworkEventLogEntries) entries_.pop_front(); entries_.push_back(entry); - VLOG(1) << entry.ToString(); + entry.SendToVLogOrErrorLog(); } std::string NetworkEventLog::GetAsString(StringOrder order, + const std::string& format, + LogLevel max_level, size_t max_events) { if (entries_.empty()) return "No Log Entries."; + bool show_time, show_file, show_desc, format_html; + GetFormat(format, &show_time, &show_file, &show_desc, &format_html); + std::string result; if (order == OLDEST_FIRST) { size_t offset = 0; - if (max_events > 0 && max_events < entries_.size()) - offset = entries_.size() - max_events; + if (max_events > 0 && max_events < entries_.size()) { + // Iterate backwards through the list skipping uninteresting entries to + // determine the first entry to include. + size_t shown_events = 0; + LogEntryList::const_reverse_iterator riter; + for (riter = entries_.rbegin(); riter != entries_.rend(); ++riter) { + if (riter->log_level > max_level) + continue; + if (++shown_events >= max_events) + break; + } + if (riter != entries_.rend()) + offset = entries_.rend() - riter - 1; + } for (LogEntryList::const_iterator iter = entries_.begin() + offset; iter != entries_.end(); ++iter) { - result += (*iter).ToString(); + if (iter->log_level > max_level) + continue; + result += (*iter).ToString(show_time, show_file, show_desc, format_html); } } else { size_t nlines = 0; // Iterate backwards through the list to show the most recent entries first. for (LogEntryList::const_reverse_iterator riter = entries_.rbegin(); riter != entries_.rend(); ++riter) { - result += (*riter).ToString(); + if (riter->log_level > max_level) + continue; + result += (*riter).ToString(show_time, show_file, show_desc, format_html); if (max_events > 0 && ++nlines >= max_events) break; } @@ -124,6 +236,7 @@ std::string NetworkEventLog::GetAsString(StringOrder order, } // namespace NetworkEventLog* g_network_event_log = NULL; +const LogLevel kDefaultLogLevel = LOG_LEVEL_EVENT; const size_t kMaxNetworkEventLogEntries = 1000; void Initialize() { @@ -141,21 +254,40 @@ bool IsInitialized() { return g_network_event_log != NULL; } -void AddEntry(const std::string& module, +namespace internal { + +void AddEntry(const char* file, + int file_line, + LogLevel log_level, const std::string& event, const std::string& description) { - LogEntry entry(module, event, description); + std::string filestr; + if (file) + filestr = base::FilePath(std::string(file)).BaseName().value(); + LogEntry entry(filestr, file_line, log_level, event, description); if (!g_network_event_log) { - VLOG(1) << entry.ToString(); + entry.SendToVLogOrErrorLog(); return; } - g_network_event_log->AddEntry(entry); + g_network_event_log->AddLogEntry(entry); } -std::string GetAsString(StringOrder order, size_t max_events) { +} // namespace internal + +std::string GetAsString(StringOrder order, + const std::string& format, + LogLevel max_level, + size_t max_events) { if (!g_network_event_log) - return "NetworkEventLog not intitialized."; - return g_network_event_log->GetAsString(order, max_events); + return "NetworkEventLog not initialized."; + return g_network_event_log->GetAsString(order, format, max_level, max_events); +} + +std::string ValueAsString(const base::Value& value) { + std::string vstr; + base::JSONWriter::WriteWithOptions( + &value, base::JSONWriter::OPTIONS_OMIT_BINARY_VALUES, &vstr); + return vstr.empty() ? "''" : vstr; } } // namespace network_event_log diff --git a/chromeos/network/network_event_log.h b/chromeos/network/network_event_log.h index 293144c..301e806 100644 --- a/chromeos/network/network_event_log.h +++ b/chromeos/network/network_event_log.h @@ -13,6 +13,10 @@ #include "base/time.h" #include "chromeos/chromeos_export.h" +namespace base { +class Value; +} + namespace chromeos { // Namespace for functions for logging network events. @@ -24,9 +28,19 @@ enum StringOrder { NEWEST_FIRST }; +// Used to set the detail level for logging. +enum LogLevel { + LOG_LEVEL_ERROR = 0, + LOG_LEVEL_EVENT = 1, + LOG_LEVEL_DEBUG = 2 +}; + // Maximum number of event log entries, exported for testing. CHROMEOS_EXPORT extern const size_t kMaxNetworkEventLogEntries; +// Default log level. +CHROMEOS_EXPORT extern const LogLevel kDefaultLogLevel; + // Initializes / shuts down network event logging. Calling Initialize more than // once will reset the log. CHROMEOS_EXPORT void Initialize(); @@ -35,29 +49,55 @@ CHROMEOS_EXPORT void Shutdown(); // Returns true if network event logging has been initialized. CHROMEOS_EXPORT bool IsInitialized(); -// Adds an entry to the event log. A maximum number of events are recorded -// after which new events replace old ones. Does nothing unless Initialize() -// has been called. -CHROMEOS_EXPORT void AddEntry(const std::string& module, +namespace internal { + +// Adds an entry to the event log at level specified by |log_level|. +// A maximum number of events are recorded after which new events replace +// old ones. Does nothing unless Initialize() has been called. +// NOTE: Generally use NET_LOG instead. +CHROMEOS_EXPORT void AddEntry(const char* file, + int file_line, + LogLevel log_level, const std::string& event, const std::string& description); -// Outputs the log to a formatted string. |order| determines which order to -// output the events. If |max_events| > 0, limits how many events are output. -CHROMEOS_EXPORT std::string GetAsString(StringOrder order, size_t max_events); - -// Macros to make logging format more consistent. -#define NET_LOG(module, message) \ - ::chromeos::network_event_log::AddEntry( \ - module, \ - std::string(__FILE__) + ":" + ::base::StringPrintf("%d",__LINE__) + \ - " (" + std::string(__func__) + ")", \ - message) - -#define NET_LOG_WARNING(module, message) \ - NET_LOG(module, std::string("WARNING:") + message) -#define NET_LOG_ERROR(module, message) \ - NET_LOG(module, std::string("ERROR:") + message) +} // namespace internal + +// Outputs the log to a formatted string. +// |order| determines which order to output the events. +// |format| is a string that determines which elements to show. Elements +// must be comma-separated, e.g. "time,desc". +// Note: order of the format strings does not affect the output. +// "time" - Include a timestamp. +// "file" - Include file and line number. +// "desc" - Include the description. +// "html" - Include html tags. +// Only events with |log_level| <= |max_level| are included in the output. +// If |max_events| > 0, limits how many events are output. +CHROMEOS_EXPORT std::string GetAsString(StringOrder order, + const std::string& format, + LogLevel max_level, + size_t max_events); + +// Helper function for displaying a value as a string. +CHROMEOS_EXPORT std::string ValueAsString(const base::Value& value); + +// Errors +#define NET_LOG_ERROR(event, desc) NET_LOG_LEVEL( \ + ::chromeos::network_event_log::LOG_LEVEL_ERROR, event, desc) + +// Important events, e.g. connection request +#define NET_LOG_EVENT(event, desc) NET_LOG_LEVEL( \ + ::chromeos::network_event_log::LOG_LEVEL_EVENT, event, desc) + +// Non-essential debugging events +#define NET_LOG_DEBUG(event, desc) NET_LOG_LEVEL( \ + ::chromeos::network_event_log::LOG_LEVEL_DEBUG, event, desc) + +// Macro to include file and line number info in the event log. +#define NET_LOG_LEVEL(log_level, event, description) \ + ::chromeos::network_event_log::internal::AddEntry( \ + __FILE__, __LINE__, log_level, event, description) } // namespace network_event_log diff --git a/chromeos/network/network_event_log_unittest.cc b/chromeos/network/network_event_log_unittest.cc index 8654bb3..9f37994 100644 --- a/chromeos/network/network_event_log_unittest.cc +++ b/chromeos/network/network_event_log_unittest.cc @@ -14,6 +14,13 @@ #include "testing/gtest/include/gtest/gtest.h" namespace chromeos { +namespace network_event_log { + +namespace { + +network_event_log::LogLevel kDefaultLevel = network_event_log::LOG_LEVEL_EVENT; + +} // namespace class NetworkEventLogTest : public testing::Test { public: @@ -36,7 +43,9 @@ class NetworkEventLogTest : public testing::Test { for (size_t i = 0; i < lines.size(); ++i) { size_t n = lines[i].find(']'); if (n != std::string::npos) - output += lines[i].substr(n+2) + '\n'; + output += "[time] " + lines[i].substr(n+2) + '\n'; + else + output += lines[i]; } return output; } @@ -45,65 +54,167 @@ class NetworkEventLogTest : public testing::Test { return std::count(input.begin(), input.end(), '\n'); } + std::string GetLogString(StringOrder order, size_t max_events) { + return network_event_log::GetAsString( + order, "file,desc", kDefaultLevel, max_events); + } + private: DISALLOW_COPY_AND_ASSIGN(NetworkEventLogTest); }; TEST_F(NetworkEventLogTest, TestNetworkEvents) { - std::string output_none = network_event_log::GetAsString( - network_event_log::OLDEST_FIRST, 0); + std::string output_none = GetLogString(OLDEST_FIRST, 0); EXPECT_EQ("No Log Entries.", output_none); - network_event_log::AddEntry("module1", "event1", "description1"); - network_event_log::AddEntry("module2", "event2", "description2"); - network_event_log::AddEntry("module3", "event3", "description3"); - network_event_log::AddEntry("module3", "event3", "description3"); + LogLevel level = kDefaultLevel; + network_event_log::internal::AddEntry( + "file1", 1, level, "event1", "description1"); + network_event_log::internal::AddEntry( + "file2", 2, level, "event2", "description2"); + network_event_log::internal::AddEntry( + "file3", 3, level, "event3", "description3"); + network_event_log::internal::AddEntry( + "file3", 3, level, "event3", "description3"); const std::string expected_output_oldest_first( - "module1:event1: description1\n" - "module2:event2: description2\n" - "module3:event3: description3 (2)\n"); - std::string output_oldest_first = network_event_log::GetAsString( - network_event_log::OLDEST_FIRST, 0); - output_oldest_first = SkipTime(output_oldest_first); + "file1:1 event1: description1\n" + "file2:2 event2: description2\n" + "file3:3 event3: description3 (2)\n"); + std::string output_oldest_first = GetLogString(OLDEST_FIRST, 0); EXPECT_EQ(expected_output_oldest_first, output_oldest_first); const std::string expected_output_oldest_first_short( - "module2:event2: description2\n" - "module3:event3: description3 (2)\n"); - std::string output_oldest_first_short = network_event_log::GetAsString( - network_event_log::OLDEST_FIRST, 2); - output_oldest_first_short = SkipTime(output_oldest_first_short); + "file2:2 event2: description2\n" + "file3:3 event3: description3 (2)\n"); + std::string output_oldest_first_short = GetLogString(OLDEST_FIRST, 2); EXPECT_EQ(expected_output_oldest_first_short, output_oldest_first_short); const std::string expected_output_newest_first( - "module3:event3: description3 (2)\n" - "module2:event2: description2\n" - "module1:event1: description1\n"); - std::string output_newest_first = network_event_log::GetAsString( - network_event_log::NEWEST_FIRST, 0); - output_newest_first = SkipTime(output_newest_first); + "file3:3 event3: description3 (2)\n" + "file2:2 event2: description2\n" + "file1:1 event1: description1\n"); + std::string output_newest_first = GetLogString(NEWEST_FIRST, 0); EXPECT_EQ(expected_output_newest_first, output_newest_first); const std::string expected_output_newest_first_short( - "module3:event3: description3 (2)\n" - "module2:event2: description2\n"); - std::string output_newest_first_short = network_event_log::GetAsString( - network_event_log::NEWEST_FIRST, 2); - output_newest_first_short = SkipTime(output_newest_first_short); + "file3:3 event3: description3 (2)\n" + "file2:2 event2: description2\n"); + std::string output_newest_first_short = GetLogString(NEWEST_FIRST, 2); EXPECT_EQ(expected_output_newest_first_short, output_newest_first_short); } TEST_F(NetworkEventLogTest, TestMaxNetworkEvents) { const size_t entries_to_add = - network_event_log::kMaxNetworkEventLogEntries + 3; - for (size_t i = 0; i < entries_to_add; ++i) - network_event_log::AddEntry("test", - base::StringPrintf("event_%"PRIuS, i), ""); - - std::string output = GetAsString(network_event_log::OLDEST_FIRST, 0); + kMaxNetworkEventLogEntries + 3; + for (size_t i = 0; i < entries_to_add; ++i) { + network_event_log::internal::AddEntry( + "test", 1, LOG_LEVEL_EVENT, + base::StringPrintf("event_%"PRIuS, i), ""); + } + std::string output = GetLogString(OLDEST_FIRST, 0); size_t output_lines = CountLines(output); - EXPECT_EQ(network_event_log::kMaxNetworkEventLogEntries, output_lines); + EXPECT_EQ(kMaxNetworkEventLogEntries, output_lines); +} + +TEST_F(NetworkEventLogTest, TestStringFormat) { + network_event_log::internal::AddEntry( + "file", 0, LOG_LEVEL_ERROR, "event0", "description"); + EXPECT_EQ("file:0 event0\n", network_event_log::GetAsString( + OLDEST_FIRST, "file", kDefaultLevel, 1)); + EXPECT_EQ("[time] event0\n", SkipTime(network_event_log::GetAsString( + OLDEST_FIRST, "time", kDefaultLevel, 1))); + EXPECT_EQ("event0: description\n", network_event_log::GetAsString( + OLDEST_FIRST, "desc", kDefaultLevel, 1)); + EXPECT_EQ("event0\n", network_event_log::GetAsString( + OLDEST_FIRST, "", kDefaultLevel, 1)); + EXPECT_EQ("<b>event0</b>\n", network_event_log::GetAsString( + OLDEST_FIRST, "html", kDefaultLevel, 1)); + EXPECT_EQ("[time] file:0 event0: description\n", + SkipTime(network_event_log::GetAsString( + OLDEST_FIRST, "file,time,desc", kDefaultLevel, 1))); + + network_event_log::internal::AddEntry( + "file", 0, LOG_LEVEL_DEBUG, "event1", "description"); + EXPECT_EQ("[time] file:0 <i>event1: description</i>\n", + SkipTime(network_event_log::GetAsString( + OLDEST_FIRST, "file,time,desc,html", LOG_LEVEL_DEBUG, 1))); +} + +namespace { + +void AddTestEvent(LogLevel level, const std::string& event) { + network_event_log::internal::AddEntry("file", 0, level, event, "description"); +} + +} // namespace + +TEST_F(NetworkEventLogTest, TestLogLevel) { + AddTestEvent(LOG_LEVEL_ERROR, "error1"); + AddTestEvent(LOG_LEVEL_ERROR, "error2"); + AddTestEvent(LOG_LEVEL_EVENT, "event3"); + AddTestEvent(LOG_LEVEL_ERROR, "error4"); + AddTestEvent(LOG_LEVEL_EVENT, "event5"); + AddTestEvent(LOG_LEVEL_DEBUG, "debug6"); + + std::string output; + output = network_event_log::GetAsString( + OLDEST_FIRST, "", LOG_LEVEL_ERROR, 0); + EXPECT_EQ(3u, CountLines(output)); + output = network_event_log::GetAsString( + OLDEST_FIRST, "", LOG_LEVEL_EVENT, 0); + EXPECT_EQ(5u, CountLines(output)); + output = network_event_log::GetAsString( + OLDEST_FIRST, "", LOG_LEVEL_DEBUG, 0); + EXPECT_EQ(6u, CountLines(output)); + + // Test max_level. Get only the ERROR entries. + output = network_event_log::GetAsString( + OLDEST_FIRST, "", LOG_LEVEL_ERROR, 0); + EXPECT_EQ("error1\nerror2\nerror4\n", output); +} + +TEST_F(NetworkEventLogTest, TestMaxEvents) { + AddTestEvent(LOG_LEVEL_EVENT, "event1"); + AddTestEvent(LOG_LEVEL_ERROR, "error2"); + AddTestEvent(LOG_LEVEL_EVENT, "event3"); + AddTestEvent(LOG_LEVEL_ERROR, "error4"); + AddTestEvent(LOG_LEVEL_EVENT, "event5"); + + // Oldest first + EXPECT_EQ("error4\n", network_event_log::GetAsString( + OLDEST_FIRST, "", LOG_LEVEL_ERROR, 1)); + + EXPECT_EQ("error2\nerror4\n", network_event_log::GetAsString( + OLDEST_FIRST, "", LOG_LEVEL_ERROR, 2)); + + EXPECT_EQ("event3\nerror4\nevent5\n", network_event_log::GetAsString( + OLDEST_FIRST, "", LOG_LEVEL_EVENT, 3)); + + EXPECT_EQ("error2\nevent3\nerror4\nevent5\n", network_event_log::GetAsString( + OLDEST_FIRST, "", LOG_LEVEL_EVENT, 4)); + + EXPECT_EQ("event1\nerror2\nevent3\nerror4\nevent5\n", + network_event_log::GetAsString( + OLDEST_FIRST, "", LOG_LEVEL_EVENT, 5)); + + // Newest first + EXPECT_EQ("error4\n", network_event_log::GetAsString( + NEWEST_FIRST, "", LOG_LEVEL_ERROR, 1)); + + EXPECT_EQ("error4\nerror2\n", network_event_log::GetAsString( + NEWEST_FIRST, "", LOG_LEVEL_ERROR, 2)); + + EXPECT_EQ("event5\nerror4\nevent3\n", network_event_log::GetAsString( + NEWEST_FIRST, "", LOG_LEVEL_EVENT, 3)); + + EXPECT_EQ("event5\nerror4\nevent3\nerror2\n", network_event_log::GetAsString( + NEWEST_FIRST, "", LOG_LEVEL_EVENT, 4)); + + EXPECT_EQ("event5\nerror4\nevent3\nerror2\nevent1\n", + network_event_log::GetAsString( + NEWEST_FIRST, "", LOG_LEVEL_EVENT, 5)); } +} // namespace network_event_log } // namespace chromeos diff --git a/chromeos/network/network_handler_callbacks.cc b/chromeos/network/network_handler_callbacks.cc index f10a12f..597b8bd 100644 --- a/chromeos/network/network_handler_callbacks.cc +++ b/chromeos/network/network_handler_callbacks.cc @@ -11,8 +11,6 @@ namespace chromeos { namespace network_handler { -const char kLogModule[] = "ShillError"; - // These are names of fields in the error data dictionary for ErrorCallback. const char kErrorName[] = "errorName"; const char kErrorMessage[] = "errorMessage"; @@ -29,17 +27,16 @@ base::DictionaryValue* CreateErrorData(const std::string& service_path, return error_data; } -void ShillErrorCallbackFunction(const std::string& module, - const std::string& path, +void ShillErrorCallbackFunction(const std::string& path, const ErrorCallback& error_callback, const std::string& error_name, const std::string& error_message) { - std::string error = "Shill Error in " + module; + std::string error = "Shill Error"; if (!path.empty()) error += " For " + path; - error += ": " + error_name + " : " + error_message; - LOG(ERROR) << error; - network_event_log::AddEntry(kLogModule, module, error); + error += ": " + error_name; + NET_LOG_ERROR(error, error_message); + LOG(ERROR) << error << " : " << error_message; if (error_callback.is_null()) return; scoped_ptr<base::DictionaryValue> error_data( diff --git a/chromeos/network/network_handler_callbacks.h b/chromeos/network/network_handler_callbacks.h index 7618757..8caf804 100644 --- a/chromeos/network/network_handler_callbacks.h +++ b/chromeos/network/network_handler_callbacks.h @@ -39,8 +39,7 @@ CHROMEOS_EXPORT base::DictionaryValue* CreateErrorData( // Callback for Shill errors. |path| may be blank if not relevant. // Logs an error and calls |error_callback| if not null. -void ShillErrorCallbackFunction(const std::string& module, - const std::string& path, +void ShillErrorCallbackFunction(const std::string& path, const ErrorCallback& error_callback, const std::string& error_name, const std::string& error_message); diff --git a/chromeos/network/network_state.cc b/chromeos/network/network_state.cc index 7092c21..3c24999 100644 --- a/chromeos/network/network_state.cc +++ b/chromeos/network/network_state.cc @@ -16,8 +16,6 @@ namespace { -const char kLogModule[] = "NetworkState"; - bool ConvertListValueToStringVector(const base::ListValue& string_list, std::vector<std::string>* result) { for (size_t i = 0; i < string_list.GetSize(); ++i) { @@ -196,9 +194,8 @@ void NetworkState::UpdateName() { std::string valid_ssid = ValidateUTF8(name()); if (valid_ssid != name()) { set_name(valid_ssid); - network_event_log::AddEntry( - kLogModule, "UpdateName", - base::StringPrintf("%s: UTF8: %s", path().c_str(), name().c_str())); + NET_LOG_DEBUG("UpdateName", base::StringPrintf( + "%s: UTF8: %s", path().c_str(), name().c_str())); } return; } @@ -210,7 +207,7 @@ void NetworkState::UpdateName() { } else { std::string desc = base::StringPrintf("%s: Error processing: %s", path().c_str(), hex_ssid_.c_str()); - network_event_log::AddEntry(kLogModule, "UpdateName", desc); + NET_LOG_DEBUG("UpdateName", desc); LOG(ERROR) << desc; ssid = name(); } @@ -218,9 +215,8 @@ void NetworkState::UpdateName() { if (IsStringUTF8(ssid)) { if (ssid != name()) { set_name(ssid); - network_event_log::AddEntry( - kLogModule, "UpdateName", - base::StringPrintf("%s: UTF8: %s", path().c_str(), name().c_str())); + NET_LOG_DEBUG("UpdateName", base::StringPrintf( + "%s: UTF8: %s", path().c_str(), name().c_str())); } return; } @@ -236,10 +232,9 @@ void NetworkState::UpdateName() { std::string utf8_ssid; if (base::ConvertToUtf8AndNormalize(ssid, encoding, &utf8_ssid)) { set_name(utf8_ssid); - network_event_log::AddEntry( - kLogModule, "UpdateName", - base::StringPrintf("%s: Encoding=%s: %s", path().c_str(), - encoding.c_str(), name().c_str())); + NET_LOG_DEBUG("UpdateName", base::StringPrintf( + "%s: Encoding=%s: %s", path().c_str(), + encoding.c_str(), name().c_str())); return; } } @@ -247,10 +242,9 @@ void NetworkState::UpdateName() { // Unrecognized encoding. Only use raw bytes if name_ is empty. if (name().empty()) set_name(ssid); - network_event_log::AddEntry( - kLogModule, "UpdateName", - base::StringPrintf("%s: Unrecognized Encoding=%s: %s", path().c_str(), - encoding.c_str(), name().c_str())); + NET_LOG_DEBUG("UpdateName", base::StringPrintf( + "%s: Unrecognized Encoding=%s: %s", path().c_str(), + encoding.c_str(), name().c_str())); } // static diff --git a/chromeos/network/network_state_handler.cc b/chromeos/network/network_state_handler.cc index a1909a4..9bf1afb 100644 --- a/chromeos/network/network_state_handler.cc +++ b/chromeos/network/network_state_handler.cc @@ -20,8 +20,6 @@ namespace { -const char kLogModule[] = "NetworkStateHandler"; - // Returns true if |network->type()| == |match_type|, or it matches one of the // following special match types: // * kMatchTypeDefault matches any network (i.e. the first instance) @@ -50,27 +48,6 @@ bool ManagedStateMatchesType(const chromeos::ManagedState* managed, return false; } -std::string ValueAsString(const base::Value& value) { - if (value.GetType() == base::Value::TYPE_BOOLEAN) { - bool bval = false; - value.GetAsBoolean(&bval); - return bval ? "true" : "false"; - } else if (value.GetType() == base::Value::TYPE_INTEGER) { - int intval = 0; - value.GetAsInteger(&intval); - return base::StringPrintf("%d", intval); - } else if (value.GetType() == base::Value::TYPE_DOUBLE) { - double dval = 0; - value.GetAsDouble(&dval); - return base::StringPrintf("%g", dval); - } else if (value.GetType() == base::Value::TYPE_STRING) { - std::string vstr; - value.GetAsString(&vstr); - return vstr; - } - return ""; -} - bool ConnectionStateChanged(chromeos::NetworkState* network, const std::string& prev_connection_state) { return (network->connection_state() != prev_connection_state) && @@ -78,6 +55,13 @@ bool ConnectionStateChanged(chromeos::NetworkState* network, !prev_connection_state.empty()); } +std::string GetManagedStateLogName(const chromeos::ManagedState* state) { + if (!state) + return "None"; + return base::StringPrintf("%s (%s)", state->name().c_str(), + state->path().c_str()); +} + } // namespace namespace chromeos { @@ -160,12 +144,12 @@ void NetworkStateHandler::SetTechnologyEnabled( bool enabled, const network_handler::ErrorCallback& error_callback) { std::string technology = GetTechnologyForType(type); - network_event_log::AddEntry( - kLogModule, "SetTechnologyEnabled", - base::StringPrintf("%s:%d", technology.c_str(), enabled)); + NET_LOG_EVENT("SetTechnologyEnabled", + base::StringPrintf("%s:%d", technology.c_str(), enabled)); shill_property_handler_->SetTechnologyEnabled( technology, enabled, error_callback); - ManagerPropertyChanged(); // Technology state changed -> ENABLING + // Signal Technology state changed -> ENABLING + NotifyManagerPropertyChanged(); } const DeviceState* NetworkStateHandler::GetDeviceState( @@ -292,7 +276,7 @@ void NetworkStateHandler::GetNetworkList(NetworkStateList* list) const { } void NetworkStateHandler::RequestScan() const { - network_event_log::AddEntry(kLogModule, "RequestScan", ""); + NET_LOG_EVENT("RequestScan", ""); shill_property_handler_->RequestScan(); } @@ -304,7 +288,7 @@ void NetworkStateHandler::WaitForScan(const std::string& type, } void NetworkStateHandler::ConnectToBestWifiNetwork() { - network_event_log::AddEntry(kLogModule, "ConnectToBestWifiNetwork", ""); + NET_LOG_EVENT("ConnectToBestWifiNetwork", ""); WaitForScan(flimflam::kTypeWifi, base::Bind(&internal::ShillPropertyHandler::ConnectToBestServices, shill_property_handler_->AsWeakPtr())); @@ -313,8 +297,11 @@ void NetworkStateHandler::ConnectToBestWifiNetwork() { void NetworkStateHandler::SetConnectingNetwork( const std::string& service_path) { connecting_network_ = service_path; - network_event_log::AddEntry( - kLogModule, "SetConnectingNetwork", service_path); + const NetworkState* network = GetNetworkState(service_path); + if (network) + NET_LOG_EVENT("SetConnectingNetwork", GetManagedStateLogName(network)); + else + NET_LOG_ERROR("SetConnectingNetwork to unknown network", service_path); } void NetworkStateHandler::GetNetworkStatePropertiesForTest( @@ -375,8 +362,7 @@ void NetworkStateHandler::UpdateManagedList(ManagedState::ManagedType type, } void NetworkStateHandler::ProfileListChanged() { - network_event_log::AddEntry( - kLogModule, "ProfileListChanged", "Re-Requesting Network Properties"); + NET_LOG_EVENT("ProfileListChanged", "Re-Requesting Network Properties"); for (ManagedStateList::iterator iter = network_list_.begin(); iter != network_list_.end(); ++iter) { shill_property_handler_->RequestProperties( @@ -407,9 +393,7 @@ void NetworkStateHandler::UpdateManagedStateProperties( } } managed->InitialPropertiesReceived(); - network_event_log::AddEntry( - kLogModule, "PropertiesReceived", - base::StringPrintf("%s (%s)", path.c_str(), managed->name().c_str())); + NET_LOG_DEBUG("PropertiesReceived", GetManagedStateLogName(managed)); // Notify observers. if (network_property_updated) { NetworkState* network = managed->AsNetworkState(); @@ -443,10 +427,12 @@ void NetworkStateHandler::UpdateNetworkServiceProperty( OnDefaultNetworkChanged(); } std::string detail = network->name() + "." + key; - std::string vstr = ValueAsString(value); - if (!vstr.empty()) - detail += " = " + vstr; - network_event_log::AddEntry(kLogModule, "NetworkPropertyUpdated", detail); + detail += " = " + network_event_log::ValueAsString(value); + network_event_log::LogLevel log_level = + (key == flimflam::kSignalStrengthProperty) + ? network_event_log::LOG_LEVEL_DEBUG + : network_event_log::LOG_LEVEL_EVENT; + NET_LOG_LEVEL(log_level, "NetworkPropertyUpdated", detail); } NetworkPropertiesUpdated(network); } @@ -461,10 +447,8 @@ void NetworkStateHandler::UpdateDeviceProperty(const std::string& device_path, return; std::string detail = device->name() + "." + key; - std::string vstr = ValueAsString(value); - if (!vstr.empty()) - detail += " = " + vstr; - network_event_log::AddEntry(kLogModule, "DevicePropertyUpdated", detail); + detail += " = " + network_event_log::ValueAsString(value); + NET_LOG_EVENT("DevicePropertyUpdated", detail); FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_, DeviceListChanged()); @@ -473,8 +457,8 @@ void NetworkStateHandler::UpdateDeviceProperty(const std::string& device_path, ScanCompleted(device->type()); } -void NetworkStateHandler::ManagerPropertyChanged() { - network_event_log::AddEntry(kLogModule, "NetworkManagerChanged", ""); +void NetworkStateHandler::NotifyManagerPropertyChanged() { + NET_LOG_DEBUG("NotifyManagerPropertyChanged", ""); FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_, NetworkManagerChanged()); } @@ -483,18 +467,16 @@ void NetworkStateHandler::ManagedStateListChanged( ManagedState::ManagedType type) { if (type == ManagedState::MANAGED_TYPE_NETWORK) { // Notify observers that the list of networks has changed. - network_event_log::AddEntry( - kLogModule, "NetworkListChanged", - base::StringPrintf("Size:%"PRIuS, network_list_.size())); + NET_LOG_EVENT("NetworkListChanged", + base::StringPrintf("Size:%"PRIuS, network_list_.size())); FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_, NetworkListChanged()); // The list order may have changed, so check if the default network changed. if (CheckDefaultNetworkChanged()) OnDefaultNetworkChanged(); } else if (type == ManagedState::MANAGED_TYPE_DEVICE) { - network_event_log::AddEntry( - kLogModule, "DeviceListChanged", - base::StringPrintf("Size:%"PRIuS, device_list_.size())); + NET_LOG_DEBUG("DeviceListChanged", + base::StringPrintf("Size:%"PRIuS, device_list_.size())); FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_, DeviceListChanged()); } else { @@ -549,10 +531,9 @@ NetworkStateHandler::ManagedStateList* NetworkStateHandler::GetManagedList( void NetworkStateHandler::OnNetworkConnectionStateChanged( NetworkState* network) { DCHECK(network); - network_event_log::AddEntry( - kLogModule, "NetworkConnectionStateChanged", - base::StringPrintf("%s:%s", network->path().c_str(), - network->connection_state().c_str())); + NET_LOG_EVENT("NetworkConnectionStateChanged", base::StringPrintf( + "%s:%s", GetManagedStateLogName(network).c_str(), + network->connection_state().c_str())); FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_, NetworkConnectionStateChanged(network)); if (CheckDefaultNetworkChanged() || network->path() == default_network_path_) @@ -572,9 +553,8 @@ bool NetworkStateHandler::CheckDefaultNetworkChanged() { void NetworkStateHandler::OnDefaultNetworkChanged() { const NetworkState* default_network = DefaultNetwork(); - network_event_log::AddEntry( - kLogModule, "DefaultNetworkChanged", - default_network ? default_network->path() : "None"); + NET_LOG_EVENT("DefaultNetworkChanged", + GetManagedStateLogName(default_network)); FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_, DefaultNetworkChanged(default_network)); } @@ -589,18 +569,16 @@ void NetworkStateHandler::NetworkPropertiesUpdated( !network->IsConnectingState() && network->connection_state() != flimflam::kStateIdle) { connecting_network_.clear(); - network_event_log::AddEntry( - kLogModule, "ClearConnectingNetwork", - base::StringPrintf("%s:%s", network->path().c_str(), - network->connection_state().c_str())); + NET_LOG_EVENT("ClearConnectingNetwork", base::StringPrintf( + "%s:%s", GetManagedStateLogName(network).c_str(), + network->connection_state().c_str())); } } void NetworkStateHandler::ScanCompleted(const std::string& type) { size_t num_callbacks = scan_complete_callbacks_.count(type); - network_event_log::AddEntry( - kLogModule, "ScanCompleted", - base::StringPrintf("%s:%"PRIuS, type.c_str(), num_callbacks)); + NET_LOG_EVENT("ScanCompleted", + base::StringPrintf("%s:%"PRIuS, type.c_str(), num_callbacks)); if (num_callbacks == 0) return; ScanCallbackList& callback_list = scan_complete_callbacks_[type]; diff --git a/chromeos/network/network_state_handler.h b/chromeos/network/network_state_handler.h index a68e33a..b7619a2 100644 --- a/chromeos/network/network_state_handler.h +++ b/chromeos/network/network_state_handler.h @@ -205,8 +205,8 @@ class CHROMEOS_EXPORT NetworkStateHandler const std::string& key, const base::Value& value) OVERRIDE; - // Sends NetworkManagerChanged() to observers. - virtual void ManagerPropertyChanged() OVERRIDE; + // Sends NetworkManagerChanged() to observers and logs an event. + virtual void NotifyManagerPropertyChanged() OVERRIDE; // Called by |shill_property_handler_| when the service or device list has // changed and all entries have been updated. This updates the list and diff --git a/chromeos/network/network_state_handler_unittest.cc b/chromeos/network/network_state_handler_unittest.cc index a987ba5..af39bc7 100644 --- a/chromeos/network/network_state_handler_unittest.cc +++ b/chromeos/network/network_state_handler_unittest.cc @@ -275,7 +275,7 @@ TEST_F(NetworkStateHandlerTest, ServicePropertyChanged) { network_state_handler_->GetNetworkState(eth0)->security()); EXPECT_EQ(2, test_observer_->PropertyUpdatesForService(eth0)); - // Changing a service to the exsiting value should not trigger an update. + // Changing a service to the existing value should not trigger an update. DBusThreadManager::Get()->GetShillServiceClient()->SetProperty( dbus::ObjectPath(eth0), flimflam::kSecurityProperty, security_value, diff --git a/chromeos/network/onc/onc_certificate_importer.cc b/chromeos/network/onc/onc_certificate_importer.cc index 266250f..fe8e66b 100644 --- a/chromeos/network/onc/onc_certificate_importer.cc +++ b/chromeos/network/onc/onc_certificate_importer.cc @@ -19,8 +19,10 @@ #include "net/cert/pem_tokenizer.h" #include "net/cert/x509_certificate.h" -#define ONC_LOG_WARNING(message) NET_LOG_WARNING("ONC", message) -#define ONC_LOG_ERROR(message) NET_LOG_ERROR("ONC", message) +#define ONC_LOG_WARNING(message) \ + NET_LOG_DEBUG("ONC Certificate Import Warning", message) +#define ONC_LOG_ERROR(message) \ + NET_LOG_ERROR("ONC Certificate Import Error", message) namespace { @@ -297,8 +299,9 @@ bool CertificateImporter::ParseServerOrCaCertificate( } if (!failures.empty()) { - ONC_LOG_ERROR("Error (" + net::ErrorToString(failures[0].net_error) + - ") importing " + cert_type + " certificate"); + ONC_LOG_ERROR(base::StringPrintf("Error ( %s ) importing %s certificate", + net::ErrorToString(failures[0].net_error), + cert_type.c_str())); return false; } if (!success) { @@ -338,8 +341,9 @@ bool CertificateImporter::ParseClientCertificate( int import_result = cert_database->ImportFromPKCS12( module.get(), decoded_pkcs12, string16(), false, &imported_certs); if (import_result != net::OK) { - ONC_LOG_ERROR("Unable to import client certificate (error " + - net::ErrorToString(import_result) + ")."); + ONC_LOG_ERROR( + base::StringPrintf("Unable to import client certificate (error %s)", + net::ErrorToString(import_result))); return false; } diff --git a/chromeos/network/shill_property_handler.cc b/chromeos/network/shill_property_handler.cc index 968b9bc..81d9221 100644 --- a/chromeos/network/shill_property_handler.cc +++ b/chromeos/network/shill_property_handler.cc @@ -22,8 +22,6 @@ namespace { -const char kLogModule[] = "ShillPropertyHandler"; - // Limit the number of services or devices we observe. Since they are listed in // priority order, it should be reasonable to ignore services past this. const size_t kMaxObserved = 100; @@ -160,7 +158,7 @@ void ShillPropertyHandler::SetTechnologyEnabled( technology, base::Bind(&base::DoNothing), base::Bind(&network_handler::ShillErrorCallbackFunction, - kLogModule, technology, error_callback)); + technology, error_callback)); } } @@ -169,15 +167,15 @@ void ShillPropertyHandler::RequestScan() const { "", base::Bind(&base::DoNothing), base::Bind(&network_handler::ShillErrorCallbackFunction, - kLogModule, "", network_handler::ErrorCallback())); + "", network_handler::ErrorCallback())); } void ShillPropertyHandler::ConnectToBestServices() const { - network_event_log::AddEntry(kLogModule, "ConnectToBestServices", ""); + NET_LOG_EVENT("ConnectToBestServices", ""); shill_manager_->ConnectToBestServices( base::Bind(&base::DoNothing), base::Bind(&network_handler::ShillErrorCallbackFunction, - kLogModule, "", network_handler::ErrorCallback())); + "", network_handler::ErrorCallback())); } void ShillPropertyHandler::RequestProperties(ManagedState::ManagedType type, @@ -203,8 +201,12 @@ void ShillPropertyHandler::RequestProperties(ManagedState::ManagedType type, void ShillPropertyHandler::OnPropertyChanged(const std::string& key, const base::Value& value) { - if (ManagerPropertyChanged(key, value)) - listener_->ManagerPropertyChanged(); + if (ManagerPropertyChanged(key, value)) { + std::string detail = key; + detail += " = " + network_event_log::ValueAsString(value); + NET_LOG_DEBUG("ManagerPropertyChanged", detail); + listener_->NotifyManagerPropertyChanged(); + } // If the service or device list changed and there are no pending // updates, signal the state list changed callback. if ((key == flimflam::kServicesProperty) && @@ -224,9 +226,11 @@ void ShillPropertyHandler::ManagerPropertiesCallback( DBusMethodCallStatus call_status, const base::DictionaryValue& properties) { if (call_status != DBUS_METHOD_CALL_SUCCESS) { - LOG(ERROR) << "Failed to get Manager properties:" << call_status; + NET_LOG_ERROR("ManagerPropertiesCallback", + base::StringPrintf("Failed: %d", call_status)); return; } + NET_LOG_EVENT("ManagerPropertiesCallback", "Success"); bool notify = false; bool update_service_list = false; for (base::DictionaryValue::Iterator iter(properties); @@ -245,7 +249,7 @@ void ShillPropertyHandler::ManagerPropertiesCallback( notify |= ManagerPropertyChanged(flimflam::kServicesProperty, *value); } if (notify) - listener_->ManagerPropertyChanged(); + listener_->NotifyManagerPropertyChanged(); // If there are no pending updates, signal the state list changed callbacks. if (pending_updates_[ManagedState::MANAGED_TYPE_NETWORK].size() == 0) listener_->ManagedStateListChanged(ManagedState::MANAGED_TYPE_NETWORK); @@ -318,7 +322,7 @@ void ShillPropertyHandler::UpdateObserved(ManagedState::ManagedType type, new_observed[path] = new ShillPropertyObserver( type, path, base::Bind( &ShillPropertyHandler::PropertyChangedCallback, AsWeakPtr())); - network_event_log::AddEntry(kLogModule, "StartObserving", path); + NET_LOG_DEBUG("StartObserving", path); } observer_map.erase(path); // Limit the number of observed services. @@ -328,7 +332,7 @@ void ShillPropertyHandler::UpdateObserved(ManagedState::ManagedType type, // Delete network service observers still in observer_map. for (ShillPropertyObserverMap::iterator iter = observer_map.begin(); iter != observer_map.end(); ++iter) { - network_event_log::AddEntry(kLogModule, "StopObserving", iter->first); + NET_LOG_DEBUG("StopObserving", iter->first); delete iter->second; } observer_map.swap(new_observed); @@ -337,9 +341,8 @@ void ShillPropertyHandler::UpdateObserved(ManagedState::ManagedType type, void ShillPropertyHandler::UpdateAvailableTechnologies( const base::ListValue& technologies) { available_technologies_.clear(); - network_event_log::AddEntry( - kLogModule, "AvailableTechnologiesChanged", - base::StringPrintf("Size: %"PRIuS, technologies.GetSize())); + NET_LOG_EVENT("AvailableTechnologiesChanged", + base::StringPrintf("Size: %"PRIuS, technologies.GetSize())); for (base::ListValue::const_iterator iter = technologies.begin(); iter != technologies.end(); ++iter) { std::string technology; @@ -352,9 +355,8 @@ void ShillPropertyHandler::UpdateAvailableTechnologies( void ShillPropertyHandler::UpdateEnabledTechnologies( const base::ListValue& technologies) { enabled_technologies_.clear(); - network_event_log::AddEntry( - kLogModule, "EnabledTechnologiesChanged", - base::StringPrintf("Size: %"PRIuS, technologies.GetSize())); + NET_LOG_EVENT("EnabledTechnologiesChanged", + base::StringPrintf("Size: %"PRIuS, technologies.GetSize())); for (base::ListValue::const_iterator iter = technologies.begin(); iter != technologies.end(); ++iter) { std::string technology; @@ -368,9 +370,8 @@ void ShillPropertyHandler::UpdateEnabledTechnologies( void ShillPropertyHandler::UpdateUninitializedTechnologies( const base::ListValue& technologies) { uninitialized_technologies_.clear(); - network_event_log::AddEntry( - kLogModule, "UninitializedTechnologiesChanged", - base::StringPrintf("Size: %"PRIuS, technologies.GetSize())); + NET_LOG_EVENT("UninitializedTechnologiesChanged", + base::StringPrintf("Size: %"PRIuS, technologies.GetSize())); for (base::ListValue::const_iterator iter = technologies.begin(); iter != technologies.end(); ++iter) { std::string technology; @@ -387,7 +388,7 @@ void ShillPropertyHandler::EnableTechnologyFailed( const std::string& error_message) { enabling_technologies_.erase(technology); network_handler::ShillErrorCallbackFunction( - kLogModule, technology, error_callback, error_name, error_message); + technology, error_callback, error_name, error_message); } void ShillPropertyHandler::GetPropertiesCallback( diff --git a/chromeos/network/shill_property_handler.h b/chromeos/network/shill_property_handler.h index 33627ef..eea30fd 100644 --- a/chromeos/network/shill_property_handler.h +++ b/chromeos/network/shill_property_handler.h @@ -73,7 +73,7 @@ class CHROMEOS_EXPORT ShillPropertyHandler // Called when one or more manager properties (e.g. a technology list) // changes. - virtual void ManagerPropertyChanged() = 0; + virtual void NotifyManagerPropertyChanged() = 0; // Called when a managed state list has changed, after properties for any // new entries in the list have been received and @@ -128,7 +128,8 @@ class CHROMEOS_EXPORT ShillPropertyHandler const base::DictionaryValue& properties); // Called form OnPropertyChanged() and ManagerPropertiesCallback(). // Returns true if observers should be notified. - bool ManagerPropertyChanged(const std::string& key, const base::Value& value); + bool ManagerPropertyChanged(const std::string& key, + const base::Value& value); // Updates the Shill property observers to observe any entries for |type|. void UpdateObserved(ManagedState::ManagedType type, diff --git a/chromeos/network/shill_property_handler_unittest.cc b/chromeos/network/shill_property_handler_unittest.cc index c41bbdb..a3ee1d4 100644 --- a/chromeos/network/shill_property_handler_unittest.cc +++ b/chromeos/network/shill_property_handler_unittest.cc @@ -67,7 +67,7 @@ class TestListener : public internal::ShillPropertyHandler::Listener { AddPropertyUpdate(flimflam::kDevicesProperty, device_path); } - virtual void ManagerPropertyChanged() OVERRIDE { + virtual void NotifyManagerPropertyChanged() OVERRIDE { ++manager_updates_; } |