chrome/browser/extensions/api/webrtc_logging_private/webrtc_event_log_apitest.cc - Issue 2131133002: Reenable the WebRTC EventLog API Tests.

Unified Diff: chrome/browser/extensions/api/webrtc_logging_private/webrtc_event_log_apitest.cc

Issue 2131133002: Reenable the WebRTC EventLog API Tests. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Update to use FilePathWatcher. Created 4 years, 5 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: chrome/browser/extensions/api/webrtc_logging_private/webrtc_event_log_apitest.cc

diff --git a/chrome/browser/extensions/api/webrtc_logging_private/webrtc_event_log_apitest.cc b/chrome/browser/extensions/api/webrtc_logging_private/webrtc_event_log_apitest.cc

index df04767c80b4ce795de6b068d0b481c385830aa9..1fce7af5add8beb1cfffeaae1a2e22728b081836 100644

--- a/chrome/browser/extensions/api/webrtc_logging_private/webrtc_event_log_apitest.cc

+++ b/chrome/browser/extensions/api/webrtc_logging_private/webrtc_event_log_apitest.cc

@@ -6,7 +6,10 @@

#include <utility>

#include "base/command_line.h"

+#include "base/files/file_path_watcher.h"

#include "base/json/json_writer.h"

+#include "base/memory/ref_counted.h"

+#include "base/run_loop.h"

#include "base/strings/string_number_conversions.h"

#include "base/threading/platform_thread.h"

#include "base/time/time.h"

@@ -38,15 +41,14 @@ namespace utils = extension_function_test_utils;

namespace {

// Get the expected EventLog file name. The name will be

-// <temporary path>.<render process id>.event_log.<consumer id>, for example

+// <temporary path>.<render process id>.<peer connection id>, for example

// /tmp/.org.chromium.Chromium.vsygNQ/dnFW8ch/Default/WebRTC

-// Logs/WebRtcEventLog.1.29113.event_log.1

+// Logs/WebRtcEventLog.1.6.1

base::FilePath GetExpectedEventLogFileName(const base::FilePath& base_file,

int render_process_id) {

- static const int kExpectedConsumerId = 1;

+ static const int kExpectedPeerConnectionId = 1;

return base_file.AddExtension(IntToStringType(render_process_id))

- .AddExtension(FILE_PATH_LITERAL("event_log"))

- .AddExtension(IntToStringType(kExpectedConsumerId));

+ .AddExtension(IntToStringType(kExpectedPeerConnectionId));

}

static const char kMainWebrtcTestHtmlPage[] = "/webrtc/webrtc_jsep01_test.html";

@@ -57,6 +59,47 @@ std::string ParamsToString(const base::ListValue& parameters) {

return parameter_string;

}

+class FileWatcher : public base::RefCountedThreadSafe<FileWatcher> {

asargent_no_longer_on_chrome 2016/07/21 23:19:27 nit: using the same name as the class in base can

Ivo-OOO until feb 6 2016/07/22 09:21:04 Good point, I renamed it to FileWaiter.

+ public:

+ explicit FileWatcher(const base::FilePath& path)

+ : found_(false), path_(path) {}

+ bool Start() {

+ if (base::PathExists(path_)) {

+ found_ = true;

+ return true;

+ } else {

+ return watcher_.Watch(path_, false /* recursive */,

+ base::Bind(&FileWatcher::Callback, this));

+ }

+ // Returns true if |path_| became available.

+ bool WaitForFile() {

+ if (!found_) {

+ run_loop_.Run();

+ }

+ return found_;

+ }

+ // implements FilePathWatcher::Callback

+ void Callback(const base::FilePath& path, bool error) {

+ EXPECT_EQ(path, path_);

+ if (!error)

+ found_ = true;

+ run_loop_.Quit();

+ }

+ private:

+ friend class base::RefCountedThreadSafe<FileWatcher>;

+ ~FileWatcher() {}

+ base::RunLoop run_loop_;

+ bool found_;

+ base::FilePath path_;

+ base::FilePathWatcher watcher_;

+ DISALLOW_COPY_AND_ASSIGN(FileWatcher);

+};

class WebrtcEventLogApiTest : public WebRtcTestBase {

protected:

void SetUp() override {

@@ -107,9 +150,7 @@ class WebrtcEventLogApiTest : public WebRtcTestBase {

} // namespace

-// TODO(ivoc): Reenable when the event log functionality in Chrome is updated.

-IN_PROC_BROWSER_TEST_F(WebrtcEventLogApiTest,

- DISABLED_TestStartStopWebRtcEventLogging) {

+IN_PROC_BROWSER_TEST_F(WebrtcEventLogApiTest, TestStartStopWebRtcEventLogging) {

ASSERT_TRUE(embedded_test_server()->Start());

content::WebContents* left_tab =

@@ -178,20 +219,18 @@ IN_PROC_BROWSER_TEST_F(WebrtcEventLogApiTest,

EXPECT_EQ(file_name_start, file_name_stop);

// Check that the file exists and is non-empty.

- base::ProcessId render_process_id =

- base::GetProcId(left_tab->GetRenderProcessHost()->GetHandle());

- EXPECT_NE(render_process_id, base::kNullProcessId);

+ content::RenderProcessHost* render_process_host =

+ left_tab->GetRenderProcessHost();

+ ASSERT_NE(render_process_host, nullptr);

+ int render_process_id = render_process_host->GetID();

base::FilePath full_file_name =

GetExpectedEventLogFileName(file_name_stop, render_process_id);

int64_t file_size = 0;

- while (!(base::PathExists(full_file_name) &&

- base::GetFileSize(full_file_name, &file_size) && file_size > 0)) {

- // This should normally not happen, but is here to prevent the test

- // from becoming flaky on devices with weird timings or when the

- // /webrtc/webrtc_jsep01_test.html changes.

- VLOG(1) << "Waiting for logfile to become available...";

- base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(100));

- }

+ scoped_refptr<FileWatcher> watcher = new FileWatcher(full_file_name);

+ ASSERT_TRUE(watcher->Start()) << "ERROR watching for "

+ << full_file_name.value();

+ ASSERT_TRUE(watcher->WaitForFile());

ASSERT_TRUE(base::PathExists(full_file_name));

EXPECT_TRUE(base::GetFileSize(full_file_name, &file_size));

EXPECT_GT(file_size, 0);

@@ -200,9 +239,8 @@ IN_PROC_BROWSER_TEST_F(WebrtcEventLogApiTest,

base::DeleteFile(full_file_name, false);

}