diff options
author | asvitkine@chromium.org <asvitkine@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-13 21:39:37 +0000 |
---|---|---|
committer | asvitkine@chromium.org <asvitkine@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-13 21:39:37 +0000 |
commit | 47b779fca8a5b22ec61e2469d14c33c9cd5bf86c (patch) | |
tree | a4e74de3b8f947c49c7a937bbd2acb14ea64677e | |
parent | 951a7e9c7844f5bd019960e19a08522d258975b6 (diff) | |
download | chromium_src-47b779fca8a5b22ec61e2469d14c33c9cd5bf86c.zip chromium_src-47b779fca8a5b22ec61e2469d14c33c9cd5bf86c.tar.gz chromium_src-47b779fca8a5b22ec61e2469d14c33c9cd5bf86c.tar.bz2 |
Revert 251095 "Revert 250828 "Add a UMA stat to track if the Bro..."
Speculative re-land to see if this causes sync tests failures again.
If it does, then we will know if this is the cause.
> Revert 250828 "Add a UMA stat to track if the Browser blacklist ..."
>
> Speculative revert for failures here:
>
> http://build.chromium.org/p/chromium.win/builders/Win7%20Sync%20x64/builds/11201
>
> > Add a UMA stat to track if the Browser blacklist is Set on the Renderer
> >
> > This shouldn't be happening, but we got some crash reports suggesting it
> > does. Unable to repo locally so this stat will verify it does occur and
> > then can be used to verify our fixes actually fix it.
> >
> > BUG=329023
> >
> > Review URL: https://codereview.chromium.org/140763008
>
> TBR=csharp@chromium.org
>
> Review URL: https://codereview.chromium.org/163633005
TBR=asvitkine@chromium.org
Review URL: https://codereview.chromium.org/164833002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@251134 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/chrome_renderer.gypi | 3 | ||||
-rw-r--r-- | chrome/interactive_ui_tests.isolate | 1 | ||||
-rw-r--r-- | chrome/renderer/DEPS | 1 | ||||
-rw-r--r-- | chrome/renderer/chrome_content_renderer_client.cc | 17 | ||||
-rw-r--r-- | chrome/unit_tests.isolate | 3 | ||||
-rw-r--r-- | chrome_elf/blacklist/blacklist.cc | 11 | ||||
-rw-r--r-- | chrome_elf/blacklist/blacklist.h | 3 | ||||
-rw-r--r-- | chrome_elf/blacklist/test/blacklist_test.cc | 4 | ||||
-rw-r--r-- | chrome_elf/blacklist/test/blacklist_test_main_dll.def | 3 | ||||
-rw-r--r-- | chrome_elf/chrome_elf.def | 1 | ||||
-rw-r--r-- | tools/metrics/histograms/histograms.xml | 7 |
11 files changed, 52 insertions, 2 deletions
diff --git a/chrome/chrome_renderer.gypi b/chrome/chrome_renderer.gypi index b546cd6..c1444cc 100644 --- a/chrome/chrome_renderer.gypi +++ b/chrome/chrome_renderer.gypi @@ -441,6 +441,9 @@ ], }], ['OS=="win"', { + 'dependencies': [ + '../chrome_elf/chrome_elf.gyp:chrome_elf', + ], 'include_dirs': [ '<(DEPTH)/third_party/wtl/include', ], diff --git a/chrome/interactive_ui_tests.isolate b/chrome/interactive_ui_tests.isolate index 248367e..ebd8ebe 100644 --- a/chrome/interactive_ui_tests.isolate +++ b/chrome/interactive_ui_tests.isolate @@ -76,6 +76,7 @@ 'isolate_dependency_tracked': [ '../net/data/ssl/certificates/foaf.me.chromium-test-cert.der', '../net/data/ssl/certificates/mit.davidben.der', + '<(PRODUCT_DIR)/chrome_elf.dll', '<(PRODUCT_DIR)/d3dcompiler_46.dll', '<(PRODUCT_DIR)/ffmpegsumo.dll', '<(PRODUCT_DIR)/libEGL.dll', diff --git a/chrome/renderer/DEPS b/chrome/renderer/DEPS index 14a782f..bf9a831f 100644 --- a/chrome/renderer/DEPS +++ b/chrome/renderer/DEPS @@ -1,4 +1,5 @@ include_rules = [ + "+chrome_elf", "+components/autofill/content/common", "+components/autofill/content/renderer", "+components/autofill/core/common", diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc index 9318da9..35abf40 100644 --- a/chrome/renderer/chrome_content_renderer_client.cc +++ b/chrome/renderer/chrome_content_renderer_client.cc @@ -116,6 +116,10 @@ #include "chrome/renderer/spellchecker/spellcheck_provider.h" #endif +#if defined(OS_WIN) +#include "chrome_elf/blacklist/blacklist.h" +#endif // OS_WIN + using autofill::AutofillAgent; using autofill::PasswordAutofillAgent; using autofill::PasswordGenerationAgent; @@ -353,6 +357,19 @@ void ChromeContentRendererClient::RenderThreadStarted() { extensions::ExtensionsClient::Set( extensions::ChromeExtensionsClient::GetInstance()); + +#if defined(OS_WIN) + // Report if the renderer process has been patched by chrome_elf. + // TODO(csharp): Remove once the renderer is no longer getting + // patched this way. + typedef bool(*IsBlacklistInitializedFunc)(); + IsBlacklistInitializedFunc is_blacklist_initialized = + reinterpret_cast<IsBlacklistInitializedFunc>( + GetProcAddress(GetModuleHandle(L"chrome_elf.dll"), + "IsBlacklistInitialized")); + if (is_blacklist_initialized()) + UMA_HISTOGRAM_BOOLEAN("Blacklist.PatchedInRenderer", true); +#endif } void ChromeContentRendererClient::RenderFrameCreated( diff --git a/chrome/unit_tests.isolate b/chrome/unit_tests.isolate index 0096f1f..93063da 100644 --- a/chrome/unit_tests.isolate +++ b/chrome/unit_tests.isolate @@ -108,7 +108,8 @@ ['OS=="win"', { 'variables': { 'isolate_dependency_tracked': [ - '<(PRODUCT_DIR)/ffmpegsumo.dll', + '<(PRODUCT_DIR)/chrome_elf.dll', + '<(PRODUCT_DIR)/ffmpegsumo.dll', ], 'isolate_dependency_untracked': [ '../ppapi/lib/gl/include/KHR/', diff --git a/chrome_elf/blacklist/blacklist.cc b/chrome_elf/blacklist/blacklist.cc index ea140c4..32d3cb2 100644 --- a/chrome_elf/blacklist/blacklist.cc +++ b/chrome_elf/blacklist/blacklist.cc @@ -60,6 +60,10 @@ enum WOW64Status { WOW64_UNKNOWN, }; +// Record if the blacklist was successfully initialized so processes can easily +// determine if the blacklist is enabled for them. +bool g_blacklist_initialized = false; + WOW64Status GetWOW64StatusForCurrentProcess() { typedef BOOL (WINAPI* IsWow64ProcessFunc)(HANDLE, PBOOL); IsWow64ProcessFunc is_wow64_process = reinterpret_cast<IsWow64ProcessFunc>( @@ -272,6 +276,10 @@ int BlacklistSize() { return size; } +bool IsBlacklistInitialized() { + return g_blacklist_initialized; +} + bool AddDllToBlacklist(const wchar_t* dll_name) { int blacklist_size = BlacklistSize(); // We need to leave one space at the end for the null pointer. @@ -373,6 +381,9 @@ bool Initialize(bool force) { } #endif + // Record that we have initialized the blacklist. + g_blacklist_initialized = true; + BYTE* thunk_storage = reinterpret_cast<BYTE*>(&g_thunk_storage); // Mark the thunk storage as readable and writeable, since we diff --git a/chrome_elf/blacklist/blacklist.h b/chrome_elf/blacklist/blacklist.h index 5237a5c..2e21f20 100644 --- a/chrome_elf/blacklist/blacklist.h +++ b/chrome_elf/blacklist/blacklist.h @@ -61,6 +61,9 @@ bool ResetBeacon(); // Return the size of the current blacklist. int BlacklistSize(); +// Returns if true if the blacklist has been initialized. +extern "C" bool IsBlacklistInitialized(); + // Adds the given dll name to the blacklist. Returns true if the dll name is in // the blacklist when this returns, false on error. Note that this will copy // |dll_name| and will leak it on exit if the string is not subsequently removed diff --git a/chrome_elf/blacklist/test/blacklist_test.cc b/chrome_elf/blacklist/test/blacklist_test.cc index 3a881ad..39db737 100644 --- a/chrome_elf/blacklist/test/blacklist_test.cc +++ b/chrome_elf/blacklist/test/blacklist_test.cc @@ -32,6 +32,7 @@ extern "C" { // functions on the test blacklist dll, not the ones linked into the test // executable itself. __declspec(dllimport) bool TestDll_AddDllToBlacklist(const wchar_t* dll_name); +__declspec(dllimport) bool TestDLL_IsBlacklistInitialized(); __declspec(dllimport) bool TestDll_RemoveDllFromBlacklist( const wchar_t* dll_name); } @@ -120,6 +121,9 @@ TEST_F(BlacklistTest, LoadBlacklistedLibrary) { base::FilePath current_dir; ASSERT_TRUE(PathService::Get(base::DIR_EXE, ¤t_dir)); + // Ensure that the blacklist is loaded. + ASSERT_TRUE(TestDLL_IsBlacklistInitialized()); + // Test that an un-blacklisted DLL can load correctly. base::ScopedNativeLibrary dll1(current_dir.Append(kTestDllName1)); EXPECT_TRUE(dll1.is_valid()); diff --git a/chrome_elf/blacklist/test/blacklist_test_main_dll.def b/chrome_elf/blacklist/test/blacklist_test_main_dll.def index 63522a0..82e0f0e 100644 --- a/chrome_elf/blacklist/test/blacklist_test_main_dll.def +++ b/chrome_elf/blacklist/test/blacklist_test_main_dll.def @@ -6,5 +6,6 @@ LIBRARY "blacklist_test_main_dll.dll" EXPORTS TestDll_AddDllToBlacklist=AddDllToBlacklist + TestDLL_IsBlacklistInitialized=IsBlacklistInitialized TestDll_RemoveDllFromBlacklist=RemoveDllFromBlacklist - InitBlacklistTestDll + InitBlacklistTestDll
\ No newline at end of file diff --git a/chrome_elf/chrome_elf.def b/chrome_elf/chrome_elf.def index 3e88cfa..ee9808f 100644 --- a/chrome_elf/chrome_elf.def +++ b/chrome_elf/chrome_elf.def @@ -6,4 +6,5 @@ LIBRARY "chrome_elf.dll" EXPORTS CreateFileW=CreateFileWRedirect + IsBlacklistInitialized SignalChromeElf diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 79e13f4..9a87944 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -1282,6 +1282,13 @@ other types of suffix sets. </summary> </histogram> +<histogram name="Blacklist.PatchedInRenderer" enum="BooleanHit"> + <summary> + Counts the number of times a renderer process is started with the browser + blacklist patch. This should never be hit. + </summary> +</histogram> + <histogram name="Blacklist.Setup" enum="BlacklistSetup"> <summary> Records the successes and failures when running the browser blacklist setup |