Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(297)

Unified Diff: content/browser/tracing/background_tracing_manager_browsertest.cc

Issue 1148633007: Hooked the trace event argument whitelist up to the background_trace_manager (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Buildfix Created 5 years, 6 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: content/browser/tracing/background_tracing_manager_browsertest.cc
diff --git a/content/browser/tracing/background_tracing_manager_browsertest.cc b/content/browser/tracing/background_tracing_manager_browsertest.cc
index 54a1127c230b13497f9c09c2e25a40bb29988342..13a309ecbc34226ee0ac9cbea781c1bea494eb3a 100644
--- a/content/browser/tracing/background_tracing_manager_browsertest.cc
+++ b/content/browser/tracing/background_tracing_manager_browsertest.cc
@@ -3,12 +3,14 @@
// found in the LICENSE file.
#include "base/bind.h"
+#include "base/trace_event/trace_event.h"
#include "content/public/browser/background_tracing_manager.h"
#include "content/public/browser/background_tracing_preemptive_config.h"
#include "content/public/browser/background_tracing_reactive_config.h"
#include "content/public/test/content_browser_test.h"
#include "content/public/test/content_browser_test_utils.h"
#include "content/public/test/test_utils.h"
+#include "third_party/zlib/zlib.h"
namespace content {
@@ -29,16 +31,42 @@ class BackgroundTracingManagerUploadConfigWrapper {
base::Unretained(this));
}
- void Upload(const base::RefCountedString* file_contents,
+ void Upload(const scoped_refptr<base::RefCountedString>& file_contents,
base::Callback<void()> done_callback) {
receive_count_ += 1;
+ EXPECT_TRUE(file_contents);
+ size_t compressed_length = file_contents->data().length();
+ const size_t kOutputBufferLength = 10 * 1024 * 1024;
+ std::vector<char> output_str(kOutputBufferLength);
+
+ z_stream stream = {0};
+ stream.avail_in = compressed_length;
+ stream.avail_out = kOutputBufferLength;
+ stream.next_in = (Bytef*)&file_contents->data()[0];
+ stream.next_out = (Bytef*)vector_as_array(&output_str);
+
+ // 16 + MAX_WBITS means only decoding gzip encoded streams, and using
+ // the biggest window size, according to zlib.h
+ int result = inflateInit2(&stream, 16 + MAX_WBITS);
+ EXPECT_EQ(Z_OK, result);
+ result = inflate(&stream, Z_FINISH);
+ int bytes_written = kOutputBufferLength - stream.avail_out;
+
+ inflateEnd(&stream);
+ EXPECT_EQ(Z_STREAM_END, result);
+
+ last_file_contents_.assign(vector_as_array(&output_str), bytes_written);
BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
base::Bind(done_callback));
BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
base::Bind(callback_));
}
+ bool TraceHasMatchingString(const char* str) {
+ return last_file_contents_.find(str) != std::string::npos;
+ }
+
int get_receive_count() const { return receive_count_; }
const BackgroundTracingManager::ReceiveCallback& get_receive_callback()
@@ -50,6 +78,7 @@ class BackgroundTracingManagerUploadConfigWrapper {
BackgroundTracingManager::ReceiveCallback receive_callback_;
base::Closure callback_;
int receive_count_;
+ std::string last_file_contents_;
};
void StartedFinalizingCallback(base::Closure callback,
@@ -97,7 +126,8 @@ void SetupBackgroundTracingManager() {
void DisableScenarioWhenIdle() {
BackgroundTracingManager::GetInstance()->SetActiveScenario(
- NULL, BackgroundTracingManager::ReceiveCallback(), false);
+ NULL, BackgroundTracingManager::ReceiveCallback(),
+ BackgroundTracingManager::NO_DATA_FILTERING);
}
// This tests that the endpoint receives the final trace data.
@@ -118,7 +148,8 @@ IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest,
GetInstance()->RegisterTriggerType("preemptive_test");
BackgroundTracingManager::GetInstance()->SetActiveScenario(
- config.Pass(), upload_config_wrapper.get_receive_callback(), true);
+ config.Pass(), upload_config_wrapper.get_receive_callback(),
+ BackgroundTracingManager::NO_DATA_FILTERING);
BackgroundTracingManager::GetInstance()->WhenIdle(
base::Bind(&DisableScenarioWhenIdle));
@@ -150,7 +181,8 @@ IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest,
"preemptive_test");
BackgroundTracingManager::GetInstance()->SetActiveScenario(
- config.Pass(), upload_config_wrapper.get_receive_callback(), true);
+ config.Pass(), upload_config_wrapper.get_receive_callback(),
+ BackgroundTracingManager::NO_DATA_FILTERING);
BackgroundTracingManager::GetInstance()->WhenIdle(
base::Bind(&DisableScenarioWhenIdle));
@@ -166,6 +198,112 @@ IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest,
}
}
+namespace {
+
+bool IsTraceEventArgsWhitelisted(const char* category_group_name,
+ const char* event_name) {
+ if (MatchPattern(category_group_name, "benchmark") &&
+ MatchPattern(event_name, "whitelisted")) {
+ return true;
+ }
+
+ return false;
+}
+
+} // namespace
+
+// This tests that non-whitelisted args get stripped if required.
+IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest,
+ NoWhitelistedArgsStripped) {
+ SetupBackgroundTracingManager();
+
+ base::trace_event::TraceLog::GetInstance()->SetArgumentFilterPredicate(
+ base::Bind(&IsTraceEventArgsWhitelisted));
+
+ base::RunLoop wait_for_upload;
+ BackgroundTracingManagerUploadConfigWrapper upload_config_wrapper(
+ wait_for_upload.QuitClosure());
+
+ scoped_ptr<BackgroundTracingPreemptiveConfig> config =
+ CreatePreemptiveConfig();
+
+ content::BackgroundTracingManager::TriggerHandle handle =
+ content::BackgroundTracingManager::GetInstance()->RegisterTriggerType(
+ "preemptive_test");
+
+ base::RunLoop wait_for_activated;
+ BackgroundTracingManager::GetInstance()->SetTracingEnabledCallbackForTesting(
+ wait_for_activated.QuitClosure());
+ EXPECT_TRUE(BackgroundTracingManager::GetInstance()->SetActiveScenario(
+ config.Pass(), upload_config_wrapper.get_receive_callback(),
+ BackgroundTracingManager::ANONYMIZE_DATA));
+
+ wait_for_activated.Run();
+
+ TRACE_EVENT1("benchmark", "whitelisted", "find_this", 1);
+ TRACE_EVENT1("benchmark", "not_whitelisted", "this_not_found", 1);
+
+ BackgroundTracingManager::GetInstance()->WhenIdle(
+ base::Bind(&DisableScenarioWhenIdle));
+
+ BackgroundTracingManager::GetInstance()->TriggerNamedEvent(
+ handle, base::Bind(&StartedFinalizingCallback, base::Closure(), true));
+
+ wait_for_upload.Run();
+
+ EXPECT_TRUE(upload_config_wrapper.get_receive_count() == 1);
+ EXPECT_TRUE(upload_config_wrapper.TraceHasMatchingString("{"));
+ EXPECT_TRUE(upload_config_wrapper.TraceHasMatchingString("find_this"));
+ EXPECT_TRUE(!upload_config_wrapper.TraceHasMatchingString("this_not_found"));
+}
+
+// This tests subprocesses (like a navigating renderer) which gets told to
+// provide a argument-filtered trace and has no predicate in place to do the
+// filtering (in this case, only the browser process gets it set), will crash
+// rather than return potential PII.
+IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest,
+ CrashWhenSubprocessWithoutArgumentFilter) {
+ SetupBackgroundTracingManager();
+
+ base::trace_event::TraceLog::GetInstance()->SetArgumentFilterPredicate(
+ base::Bind(&IsTraceEventArgsWhitelisted));
+
+ base::RunLoop wait_for_upload;
+ BackgroundTracingManagerUploadConfigWrapper upload_config_wrapper(
+ wait_for_upload.QuitClosure());
+
+ scoped_ptr<BackgroundTracingPreemptiveConfig> config =
+ CreatePreemptiveConfig();
+
+ content::BackgroundTracingManager::TriggerHandle handle =
+ content::BackgroundTracingManager::GetInstance()->RegisterTriggerType(
+ "preemptive_test");
+
+ base::RunLoop wait_for_activated;
+ BackgroundTracingManager::GetInstance()->SetTracingEnabledCallbackForTesting(
+ wait_for_activated.QuitClosure());
+ EXPECT_TRUE(BackgroundTracingManager::GetInstance()->SetActiveScenario(
+ config.Pass(), upload_config_wrapper.get_receive_callback(),
+ BackgroundTracingManager::ANONYMIZE_DATA));
+
+ wait_for_activated.Run();
+
+ NavigateToURL(shell(), GetTestUrl("", "about:blank"));
+
+ BackgroundTracingManager::GetInstance()->WhenIdle(
+ base::Bind(&DisableScenarioWhenIdle));
+
+ BackgroundTracingManager::GetInstance()->TriggerNamedEvent(
+ handle, base::Bind(&StartedFinalizingCallback, base::Closure(), true));
+
+ wait_for_upload.Run();
+
+ EXPECT_TRUE(upload_config_wrapper.get_receive_count() == 1);
+ // We should *not* receive anything at all from the renderer,
+ // the process should've crashed rather than letting that happen.
+ EXPECT_TRUE(!upload_config_wrapper.TraceHasMatchingString("CrRendererMain"));
+}
+
// This tests multiple triggers still only gathers once.
IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest,
CallMultipleTriggersOnlyGatherOnce) {
@@ -194,7 +332,8 @@ IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest,
BackgroundTracingManager::GetInstance()->RegisterTriggerType("test2");
BackgroundTracingManager::GetInstance()->SetActiveScenario(
- config.Pass(), upload_config_wrapper.get_receive_callback(), true);
+ config.Pass(), upload_config_wrapper.get_receive_callback(),
+ BackgroundTracingManager::NO_DATA_FILTERING);
BackgroundTracingManager::GetInstance()->WhenIdle(
base::Bind(&DisableScenarioWhenIdle));
@@ -256,7 +395,8 @@ IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest,
"does_not_exist");
BackgroundTracingManager::GetInstance()->SetActiveScenario(
- config.Pass(), upload_config_wrapper.get_receive_callback(), true);
+ config.Pass(), upload_config_wrapper.get_receive_callback(),
+ BackgroundTracingManager::NO_DATA_FILTERING);
BackgroundTracingManager::GetInstance()->WhenIdle(
base::Bind(&DisableScenarioWhenIdle));
@@ -292,7 +432,8 @@ IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest,
->InvalidateTriggerHandlesForTesting();
BackgroundTracingManager::GetInstance()->SetActiveScenario(
- config.Pass(), upload_config_wrapper.get_receive_callback(), true);
+ config.Pass(), upload_config_wrapper.get_receive_callback(),
+ BackgroundTracingManager::NO_DATA_FILTERING);
BackgroundTracingManager::GetInstance()->WhenIdle(
base::Bind(&DisableScenarioWhenIdle));
@@ -327,7 +468,8 @@ IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest,
config->configs.push_back(rule);
bool result = BackgroundTracingManager::GetInstance()->SetActiveScenario(
- config.Pass(), upload_config_wrapper.get_receive_callback(), true);
+ config.Pass(), upload_config_wrapper.get_receive_callback(),
+ BackgroundTracingManager::NO_DATA_FILTERING);
EXPECT_FALSE(result);
}
@@ -351,7 +493,8 @@ IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest,
GetInstance()->RegisterTriggerType("reactive_test");
BackgroundTracingManager::GetInstance()->SetActiveScenario(
- config.Pass(), upload_config_wrapper.get_receive_callback(), true);
+ config.Pass(), upload_config_wrapper.get_receive_callback(),
+ BackgroundTracingManager::NO_DATA_FILTERING);
BackgroundTracingManager::GetInstance()->WhenIdle(
base::Bind(&DisableScenarioWhenIdle));
@@ -385,7 +528,8 @@ IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest,
GetInstance()->RegisterTriggerType("reactive_test");
BackgroundTracingManager::GetInstance()->SetActiveScenario(
- config.Pass(), upload_config_wrapper.get_receive_callback(), true);
+ config.Pass(), upload_config_wrapper.get_receive_callback(),
+ BackgroundTracingManager::NO_DATA_FILTERING);
BackgroundTracingManager::GetInstance()->WhenIdle(
base::Bind(&DisableScenarioWhenIdle));
@@ -420,7 +564,8 @@ IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest,
GetInstance()->RegisterTriggerType("reactive_test");
BackgroundTracingManager::GetInstance()->SetActiveScenario(
- config.Pass(), upload_config_wrapper.get_receive_callback(), true);
+ config.Pass(), upload_config_wrapper.get_receive_callback(),
+ BackgroundTracingManager::NO_DATA_FILTERING);
BackgroundTracingManager::GetInstance()->WhenIdle(
base::Bind(&DisableScenarioWhenIdle));
« no previous file with comments | « chrome/browser/tracing/background_tracing_field_trial.cc ('k') | content/browser/tracing/background_tracing_manager_impl.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698