From 3ea187803b7e9c5a79ee85f6584462d292d9809d Mon Sep 17 00:00:00 2001 From: Jiawei Ou Date: Wed, 31 Oct 2018 23:14:24 -0700 Subject: [PATCH] Add severity into RTC logging callbacks Bug: webrtc:9945 Change-Id: I5022f63103503d2213492d3cd1a6953fe658fda7 Reviewed-on: https://webrtc-review.googlesource.com/c/108981 Commit-Queue: Jiawei Ou Reviewed-by: Anders Carlsson Reviewed-by: Karl Wiberg Cr-Commit-Position: refs/heads/master@{#25510} --- rtc_base/logging.cc | 9 +- rtc_base/logging.h | 2 + sdk/objc/api/logging/RTCCallbackLogger.h | 14 ++- sdk/objc/api/logging/RTCCallbackLogger.mm | 64 ++++++++-- sdk/objc/base/RTCLogging.h | 1 + sdk/objc/base/RTCLogging.mm | 2 + sdk/objc/unittests/RTCCallbackLogger_xctest.m | 118 ++++++++++++++++++ 7 files changed, 195 insertions(+), 15 deletions(-) diff --git a/rtc_base/logging.cc b/rtc_base/logging.cc index 9c4ee4147c..bb4fbfaae2 100644 --- a/rtc_base/logging.cc +++ b/rtc_base/logging.cc @@ -73,7 +73,12 @@ CriticalSection g_log_crit; void LogSink::OnLogMessage(const std::string& msg, LoggingSeverity severity, const char* tag) { - OnLogMessage(tag + (": " + msg)); + OnLogMessage(tag + (": " + msg), severity); +} + +void LogSink::OnLogMessage(const std::string& msg, + LoggingSeverity /* severity */) { + OnLogMessage(msg); } ///////////////////////////////////////////////////////////////////////////// @@ -206,7 +211,7 @@ LogMessage::~LogMessage() { #if defined(WEBRTC_ANDROID) kv.first->OnLogMessage(str, severity_, tag_); #else - kv.first->OnLogMessage(str); + kv.first->OnLogMessage(str, severity_); #endif } } diff --git a/rtc_base/logging.h b/rtc_base/logging.h index 42929715e2..c15c37af0a 100644 --- a/rtc_base/logging.h +++ b/rtc_base/logging.h @@ -115,6 +115,8 @@ class LogSink { virtual void OnLogMessage(const std::string& msg, LoggingSeverity severity, const char* tag); + virtual void OnLogMessage(const std::string& message, + LoggingSeverity severity); virtual void OnLogMessage(const std::string& message) = 0; }; diff --git a/sdk/objc/api/logging/RTCCallbackLogger.h b/sdk/objc/api/logging/RTCCallbackLogger.h index 4867b31e36..2bce03fe0f 100644 --- a/sdk/objc/api/logging/RTCCallbackLogger.h +++ b/sdk/objc/api/logging/RTCCallbackLogger.h @@ -15,6 +15,10 @@ NS_ASSUME_NONNULL_BEGIN +typedef void (^RTCCallbackLoggerMessageHandler)(NSString *message); +typedef void (^RTCCallbackLoggerMessageAndSeverityHandler)(NSString *message, + RTCLoggingSeverity severity); + // This class intercepts WebRTC logs and forwards them to a registered block. // This class is not threadsafe. RTC_OBJC_EXPORT @@ -23,10 +27,12 @@ RTC_OBJC_EXPORT // The severity level to capture. The default is kRTCLoggingSeverityInfo. @property(nonatomic, assign) RTCLoggingSeverity severity; -// The callback will be called on the same thread that does the logging, so -// if the logging callback can be slow it may be a good idea to implement -// dispatching to some other queue. -- (void)start:(nullable void (^)(NSString*))callback; +// The callback handler will be called on the same thread that does the +// logging, so if the logging callback can be slow it may be a good idea +// to implement dispatching to some other queue. +- (void)start:(nullable RTCCallbackLoggerMessageHandler)handler; +- (void)startWithMessageAndSeverityHandler: + (nullable RTCCallbackLoggerMessageAndSeverityHandler)handler; - (void)stop; diff --git a/sdk/objc/api/logging/RTCCallbackLogger.mm b/sdk/objc/api/logging/RTCCallbackLogger.mm index da60c2b9d2..9bbb530434 100644 --- a/sdk/objc/api/logging/RTCCallbackLogger.mm +++ b/sdk/objc/api/logging/RTCCallbackLogger.mm @@ -18,11 +18,8 @@ class CallbackLogSink : public rtc::LogSink { public: - CallbackLogSink(void (^callbackHandler)(NSString *message)) { - callback_handler_ = callbackHandler; - } - - ~CallbackLogSink() override { callback_handler_ = nil; } + CallbackLogSink(RTCCallbackLoggerMessageHandler callbackHandler) + : callback_handler_(callbackHandler) {} void OnLogMessage(const std::string &message) override { if (callback_handler_) { @@ -31,12 +28,47 @@ class CallbackLogSink : public rtc::LogSink { } private: - void (^callback_handler_)(NSString *message); + RTCCallbackLoggerMessageHandler callback_handler_; +}; + +class CallbackWithSeverityLogSink : public rtc::LogSink { + public: + CallbackWithSeverityLogSink(RTCCallbackLoggerMessageAndSeverityHandler callbackHandler) + : callback_handler_(callbackHandler) {} + + void OnLogMessage(const std::string& message) override { RTC_NOTREACHED(); } + + void OnLogMessage(const std::string& message, rtc::LoggingSeverity severity) override { + if (callback_handler_) { + RTCLoggingSeverity loggingSeverity = NativeSeverityToObjcSeverity(severity); + callback_handler_([NSString stringWithUTF8String:message.c_str()], loggingSeverity); + } + } + + private: + static RTCLoggingSeverity NativeSeverityToObjcSeverity(rtc::LoggingSeverity severity) { + switch (severity) { + case rtc::LS_SENSITIVE: + return RTCLoggingSeveritySensitive; + case rtc::LS_VERBOSE: + return RTCLoggingSeverityVerbose; + case rtc::LS_INFO: + return RTCLoggingSeverityInfo; + case rtc::LS_WARNING: + return RTCLoggingSeverityWarning; + case rtc::LS_ERROR: + return RTCLoggingSeverityError; + case rtc::LS_NONE: + return RTCLoggingSeverityNone; + } + } + + RTCCallbackLoggerMessageAndSeverityHandler callback_handler_; }; @implementation RTCCallbackLogger { BOOL _hasStarted; - std::unique_ptr _logSink; + std::unique_ptr _logSink; } @synthesize severity = _severity; @@ -53,12 +85,24 @@ class CallbackLogSink : public rtc::LogSink { [self stop]; } -- (void)start:(nullable void (^)(NSString *))callback { +- (void)start:(nullable RTCCallbackLoggerMessageHandler)handler { if (_hasStarted) { return; } - _logSink.reset(new CallbackLogSink(callback)); + _logSink.reset(new CallbackLogSink(handler)); + + rtc::LogMessage::AddLogToStream(_logSink.get(), [self rtcSeverity]); + _hasStarted = YES; +} + +- (void)startWithMessageAndSeverityHandler: + (nullable RTCCallbackLoggerMessageAndSeverityHandler)handler { + if (_hasStarted) { + return; + } + + _logSink.reset(new CallbackWithSeverityLogSink(handler)); rtc::LogMessage::AddLogToStream(_logSink.get(), [self rtcSeverity]); _hasStarted = YES; @@ -78,6 +122,8 @@ class CallbackLogSink : public rtc::LogSink { - (rtc::LoggingSeverity)rtcSeverity { switch (_severity) { + case RTCLoggingSeveritySensitive: + return rtc::LS_SENSITIVE; case RTCLoggingSeverityVerbose: return rtc::LS_VERBOSE; case RTCLoggingSeverityInfo: diff --git a/sdk/objc/base/RTCLogging.h b/sdk/objc/base/RTCLogging.h index 754945c8f2..b20a4b4d42 100644 --- a/sdk/objc/base/RTCLogging.h +++ b/sdk/objc/base/RTCLogging.h @@ -14,6 +14,7 @@ // Subset of rtc::LoggingSeverity. typedef NS_ENUM(NSInteger, RTCLoggingSeverity) { + RTCLoggingSeveritySensitive, RTCLoggingSeverityVerbose, RTCLoggingSeverityInfo, RTCLoggingSeverityWarning, diff --git a/sdk/objc/base/RTCLogging.mm b/sdk/objc/base/RTCLogging.mm index e8dae02efb..3aae47a240 100644 --- a/sdk/objc/base/RTCLogging.mm +++ b/sdk/objc/base/RTCLogging.mm @@ -14,6 +14,8 @@ rtc::LoggingSeverity RTCGetNativeLoggingSeverity(RTCLoggingSeverity severity) { switch (severity) { + case RTCLoggingSeveritySensitive: + return rtc::LS_SENSITIVE; case RTCLoggingSeverityVerbose: return rtc::LS_VERBOSE; case RTCLoggingSeverityInfo: diff --git a/sdk/objc/unittests/RTCCallbackLogger_xctest.m b/sdk/objc/unittests/RTCCallbackLogger_xctest.m index fbebb99ad7..ceaa762f1f 100644 --- a/sdk/objc/unittests/RTCCallbackLogger_xctest.m +++ b/sdk/objc/unittests/RTCCallbackLogger_xctest.m @@ -49,6 +49,23 @@ [self waitForExpectations:@[ callbackExpectation ] timeout:10.0]; } +- (void)testCallbackWithSeverityGetsCalledForAppropriateLevel { + self.logger.severity = RTCLoggingSeverityWarning; + + XCTestExpectation *callbackExpectation = [self expectationWithDescription:@"callbackWarning"]; + + [self.logger + startWithMessageAndSeverityHandler:^(NSString *message, RTCLoggingSeverity severity) { + XCTAssertTrue([message hasSuffix:@"Horrible error\n"]); + XCTAssertEqual(severity, RTCLoggingSeverityError); + [callbackExpectation fulfill]; + }]; + + RTCLogError("Horrible error"); + + [self waitForExpectations:@[ callbackExpectation ] timeout:10.0]; +} + - (void)testCallbackDoesNotGetCalledForOtherLevels { self.logger.severity = RTCLoggingSeverityError; @@ -66,6 +83,25 @@ [self waitForExpectations:@[ callbackExpectation ] timeout:10.0]; } +- (void)testCallbackWithSeverityDoesNotGetCalledForOtherLevels { + self.logger.severity = RTCLoggingSeverityError; + + XCTestExpectation *callbackExpectation = [self expectationWithDescription:@"callbackError"]; + + [self.logger + startWithMessageAndSeverityHandler:^(NSString *message, RTCLoggingSeverity severity) { + XCTAssertTrue([message hasSuffix:@"Horrible error\n"]); + XCTAssertEqual(severity, RTCLoggingSeverityError); + [callbackExpectation fulfill]; + }]; + + RTCLogInfo("Just some info"); + RTCLogWarning("Warning warning"); + RTCLogError("Horrible error"); + + [self waitForExpectations:@[ callbackExpectation ] timeout:10.0]; +} + - (void)testCallbackDoesNotgetCalledForSeverityNone { self.logger.severity = RTCLoggingSeverityNone; @@ -85,12 +121,38 @@ XCTAssertEqual(result, XCTWaiterResultTimedOut); } +- (void)testCallbackWithSeverityDoesNotgetCalledForSeverityNone { + self.logger.severity = RTCLoggingSeverityNone; + + XCTestExpectation *callbackExpectation = [self expectationWithDescription:@"unexpectedCallback"]; + + [self.logger + startWithMessageAndSeverityHandler:^(NSString *message, RTCLoggingSeverity severity) { + [callbackExpectation fulfill]; + XCTAssertTrue(false); + }]; + + RTCLogInfo("Just some info"); + RTCLogWarning("Warning warning"); + RTCLogError("Horrible error"); + + XCTWaiter *waiter = [[XCTWaiter alloc] init]; + XCTWaiterResult result = [waiter waitForExpectations:@[ callbackExpectation ] timeout:1.0]; + XCTAssertEqual(result, XCTWaiterResultTimedOut); +} + - (void)testStartingWithNilCallbackDoesNotCrash { [self.logger start:nil]; RTCLogError("Horrible error"); } +- (void)testStartingWithNilCallbackWithSeverityDoesNotCrash { + [self.logger startWithMessageAndSeverityHandler:nil]; + + RTCLogError("Horrible error"); +} + - (void)testStopCallbackLogger { XCTestExpectation *callbackExpectation = [self expectationWithDescription:@"stopped"]; @@ -107,6 +169,23 @@ XCTAssertEqual(result, XCTWaiterResultTimedOut); } +- (void)testStopCallbackWithSeverityLogger { + XCTestExpectation *callbackExpectation = [self expectationWithDescription:@"stopped"]; + + [self.logger + startWithMessageAndSeverityHandler:^(NSString *message, RTCLoggingSeverity loggingServerity) { + [callbackExpectation fulfill]; + }]; + + [self.logger stop]; + + RTCLogInfo("Just some info"); + + XCTWaiter *waiter = [[XCTWaiter alloc] init]; + XCTWaiterResult result = [waiter waitForExpectations:@[ callbackExpectation ] timeout:1.0]; + XCTAssertEqual(result, XCTWaiterResultTimedOut); +} + - (void)testDestroyingCallbackLogger { XCTestExpectation *callbackExpectation = [self expectationWithDescription:@"destroyed"]; @@ -123,4 +202,43 @@ XCTAssertEqual(result, XCTWaiterResultTimedOut); } +- (void)testDestroyingCallbackWithSeverityLogger { + XCTestExpectation *callbackExpectation = [self expectationWithDescription:@"destroyed"]; + + [self.logger + startWithMessageAndSeverityHandler:^(NSString *message, RTCLoggingSeverity loggingServerity) { + [callbackExpectation fulfill]; + }]; + + self.logger = nil; + + RTCLogInfo("Just some info"); + + XCTWaiter *waiter = [[XCTWaiter alloc] init]; + XCTWaiterResult result = [waiter waitForExpectations:@[ callbackExpectation ] timeout:1.0]; + XCTAssertEqual(result, XCTWaiterResultTimedOut); +} + +- (void)testCallbackWithSeverityLoggerCannotStartTwice { + self.logger.severity = RTCLoggingSeverityWarning; + + XCTestExpectation *callbackExpectation = [self expectationWithDescription:@"callbackWarning"]; + + [self.logger + startWithMessageAndSeverityHandler:^(NSString *message, RTCLoggingSeverity loggingServerity) { + XCTAssertTrue([message hasSuffix:@"Horrible error\n"]); + XCTAssertEqual(loggingServerity, RTCLoggingSeverityError); + [callbackExpectation fulfill]; + }]; + + [self.logger start:^(NSString *message) { + [callbackExpectation fulfill]; + XCTAssertTrue(false); + }]; + + RTCLogError("Horrible error"); + + [self waitForExpectations:@[ callbackExpectation ] timeout:10.0]; +} + @end