Commit 3a584d80 authored by Elad Alon's avatar Elad Alon Committed by Commit Bot

Change default output period of startEventLogging() to 5000ms

Bug: 775415
Change-Id: I44d028c36558df47c0dd33cc853d4e50938c49bc
Reviewed-on: https://chromium-review.googlesource.com/c/1370945Reviewed-by: default avatarGuido Urdaneta <guidou@chromium.org>
Reviewed-by: default avatarHenrik Grunell <grunell@chromium.org>
Commit-Queue: Elad Alon <eladalon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615482}
parent eb8419e0
......@@ -54,7 +54,6 @@ using webrtc_event_logging::kStartRemoteLoggingFailureAlreadyLogging;
using webrtc_event_logging::kStartRemoteLoggingFailureFeatureDisabled;
using webrtc_event_logging::kStartRemoteLoggingFailureMaxSizeTooLarge;
using webrtc_event_logging::kStartRemoteLoggingFailureMaxSizeTooSmall;
using webrtc_event_logging::kStartRemoteLoggingFailureOutputPeriodMsIsNegative;
using webrtc_event_logging::kStartRemoteLoggingFailureOutputPeriodMsTooLarge;
using webrtc_event_logging::
kStartRemoteLoggingFailureUnknownOrInactivePeerConnection;
......@@ -765,19 +764,6 @@ IN_PROC_BROWSER_TEST_F(
expect_success, error_message);
}
IN_PROC_BROWSER_TEST_F(
WebrtcLoggingPrivateApiStartEventLoggingTestFeatureAndPolicyEnabled,
StartEventLoggingWithNegativeOutputPeriodMsFails) {
const std::string peer_connection_id = "id";
SetUpPeerConnection(peer_connection_id);
const int output_period_ms = -1;
constexpr bool expect_success = false;
const std::string error_message =
kStartRemoteLoggingFailureOutputPeriodMsIsNegative;
StartEventLogging(peer_connection_id, kMaxRemoteLogFileSizeBytes,
output_period_ms, kWebAppId, expect_success, error_message);
}
IN_PROC_BROWSER_TEST_F(
WebrtcLoggingPrivateApiStartEventLoggingTestFeatureAndPolicyEnabled,
StartEventLoggingWithTooLargeOutputPeriodMsFails) {
......
......@@ -49,8 +49,6 @@ const char kStartRemoteLoggingFailureUnlimitedSizeDisallowed[] =
const char kStartRemoteLoggingFailureMaxSizeTooSmall[] = "Max size too small.";
const char kStartRemoteLoggingFailureMaxSizeTooLarge[] =
"Excessively large max log size.";
const char kStartRemoteLoggingFailureOutputPeriodMsIsNegative[] =
"Negative output period (ms).";
const char kStartRemoteLoggingFailureOutputPeriodMsTooLarge[] =
"Excessively large output period (ms).";
const char kStartRemoteLoggingFailureIllegalWebAppId[] = "Illegal web-app ID.";
......
......@@ -102,7 +102,6 @@ extern const char kStartRemoteLoggingFailureFeatureDisabled[];
extern const char kStartRemoteLoggingFailureUnlimitedSizeDisallowed[];
extern const char kStartRemoteLoggingFailureMaxSizeTooSmall[];
extern const char kStartRemoteLoggingFailureMaxSizeTooLarge[];
extern const char kStartRemoteLoggingFailureOutputPeriodMsIsNegative[];
extern const char kStartRemoteLoggingFailureOutputPeriodMsTooLarge[];
extern const char kStartRemoteLoggingFailureIllegalWebAppId[];
extern const char kStartRemoteLoggingFailureUnknownOrInactivePeerConnection[];
......
......@@ -28,6 +28,7 @@ namespace webrtc_event_logging {
// issue where we read the entire file into memory.
const size_t kMaxRemoteLogFileSizeBytes = 50000000u;
const int kDefaultOutputPeriodMs = 5000;
const int kMaxOutputPeriodMs = 60000;
namespace {
......@@ -405,6 +406,10 @@ bool WebRtcRemoteEventLogManager::StartRemoteLogging(
DCHECK(error_message);
DCHECK(error_message->empty());
if (output_period_ms < 0) {
output_period_ms = kDefaultOutputPeriodMs;
}
if (!AreLogParametersValid(max_file_size_bytes, output_period_ms, web_app_id,
error_message)) {
// |error_message| will have been set by AreLogParametersValid().
......@@ -659,12 +664,6 @@ bool WebRtcRemoteEventLogManager::AreLogParametersValid(
return false;
}
if (output_period_ms < 0) {
LOG(WARNING) << "Output period (ms) must be non-negative.";
*error_message = kStartRemoteLoggingFailureOutputPeriodMsIsNegative;
return false;
}
if (output_period_ms > kMaxOutputPeriodMs) {
LOG(WARNING) << "Output period (ms) exceeds maximum allowed.";
*error_message = kStartRemoteLoggingFailureOutputPeriodMsTooLarge;
......
......@@ -2019,19 +2019,6 @@ TEST_F(WebRtcEventLogManagerTest,
EXPECT_EQ(error_message, kStartRemoteLoggingFailureMaxSizeTooLarge);
}
TEST_F(WebRtcEventLogManagerTest,
StartRemoteLoggingReturnsFalseIfOutputPeriodMsIsNegative) {
const auto key = GetPeerConnectionKey(rph_.get(), kLid);
const std::string id = "id"; // For explicitness' sake.
ASSERT_TRUE(PeerConnectionAdded(key, id));
std::string error_message;
constexpr int output_period_ms = -1;
EXPECT_FALSE(StartRemoteLogging(key, id, kMaxRemoteLogFileSizeBytes,
output_period_ms, kWebAppId, nullptr,
&error_message));
EXPECT_EQ(error_message, kStartRemoteLoggingFailureOutputPeriodMsIsNegative);
}
TEST_F(WebRtcEventLogManagerTest,
StartRemoteLoggingReturnsFalseIfExcessivelyLargeOutputPeriodMs) {
const auto key = GetPeerConnectionKey(rph_.get(), kLid);
......
......@@ -5,7 +5,7 @@
"name": "Google Hangouts",
// Note: Always update the version number when this file is updated. Chrome
// triggers extension preferences update on the version increase.
"version": "1.3.11",
"version": "1.3.12",
"manifest_version": 2,
"externally_connectable": {
"matches": [
......
......@@ -182,7 +182,7 @@ chrome.runtime.onMessageExternal.addListener(function(
} else if (method == 'logging.startEventLogging') {
const peerConnectionId = message['peerConnectionId'] || '';
const maxLogSizeBytes = message['maxLogSizeBytes'] || 0;
const outputPeriodMs = message['outputPeriodMs'] || 0;
const outputPeriodMs = message['outputPeriodMs'] || -1;
const webAppId = message['webAppId'] || 0;
chrome.webrtcLoggingPrivate.startEventLogging(
requestInfo, origin, peerConnectionId, maxLogSizeBytes,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment