Cleanup browser blacklist code

Cleanup browser blacklist code


This change also involves some cleanup of how the blacklist is defined in the code.
This will make it easier to add new dll names at compile time.

The module name check is now also case-insensitive.


BUG=329023
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=246965

[robertshield, 2014-01-24 01:50:24]

Change looks good overall. Reading over it, I'm not 100% convinced that the new
storage structure for the blacklist (NULL-terminated array) is better than the
previous (array + cursor). It seems roughly equivalent with the advantage of not
tracking a cursor, but the disadvantage of having the call to BlacklistSize be
O(N). Why is it better?

https://codereview.chromium.org/138903022/diff/1/chrome_elf/blacklist/blackli...
File chrome_elf/blacklist/blacklist.cc (right):

https://codereview.chromium.org/138903022/diff/1/chrome_elf/blacklist/blackli...
chrome_elf/blacklist/blacklist.cc:243: }
nit: no brackets needed for one line body. also space after 'while'

try this:

while (blacklist::g_troublesome_dlls[size++] != NULL);

https://codereview.chromium.org/138903022/diff/1/chrome_elf/blacklist/test/bl...
File chrome_elf/blacklist/test/blacklist_test.cc (right):

https://codereview.chromium.org/138903022/diff/1/chrome_elf/blacklist/test/bl...
chrome_elf/blacklist/test/blacklist_test.cc:161:
EXPECT_TRUE(TestDll_AddDllToBlacklist(uppercase_name.c_str()));
shouldn't only one of the name in the blacklist and the name of the module being
loaded be uppercased? it looks like they are both uppercased here.

[csharp, 2014-01-24 16:22:59]

https://codereview.chromium.org/138903022/diff/1/chrome_elf/blacklist/blackli...
File chrome_elf/blacklist/blacklist.cc (right):

https://codereview.chromium.org/138903022/diff/1/chrome_elf/blacklist/blackli...
chrome_elf/blacklist/blacklist.cc:243: }
On 2014/01/24 01:50:24, robertshield wrote:
> nit: no brackets needed for one line body. also space after 'while'
> 
> try this:
> 
> while (blacklist::g_troublesome_dlls[size++] != NULL);

Removed unneeded brackets.

The function needs to be preop, not post though (we want to compare the next
position to see if we should continue).

https://codereview.chromium.org/138903022/diff/1/chrome_elf/blacklist/test/bl...
File chrome_elf/blacklist/test/blacklist_test.cc (right):

https://codereview.chromium.org/138903022/diff/1/chrome_elf/blacklist/test/bl...
chrome_elf/blacklist/test/blacklist_test.cc:161:
EXPECT_TRUE(TestDll_AddDllToBlacklist(uppercase_name.c_str()));
On 2014/01/24 01:50:24, robertshield wrote:
> shouldn't only one of the name in the blacklist and the name of the module
being
> loaded be uppercased? it looks like they are both uppercased here.

Done.

[robertshield, 2014-01-24 16:38:34]

lgtm

https://codereview.chromium.org/138903022/diff/100001/chrome_elf/blacklist/bl...
File chrome_elf/blacklist/blacklist.cc (right):

https://codereview.chromium.org/138903022/diff/100001/chrome_elf/blacklist/bl...
chrome_elf/blacklist/blacklist.cc:23: L"libsvn_tsvn32.dll",
Do you want to move just the above line to a separate CL for easy tweaking
later?

[csharp, 2014-01-24 16:42:08]

https://codereview.chromium.org/138903022/diff/100001/chrome_elf/blacklist/bl...
File chrome_elf/blacklist/blacklist.cc (right):

https://codereview.chromium.org/138903022/diff/100001/chrome_elf/blacklist/bl...
chrome_elf/blacklist/blacklist.cc:23: L"libsvn_tsvn32.dll",
On 2014/01/24 16:38:35, robertshield wrote:
> Do you want to move just the above line to a separate CL for easy tweaking
> later?

Done.

[commit-bot: I haz the power, 2014-01-24 16:42:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/csharp@chromium.org/138903022/160001

[commit-bot: I haz the power, 2014-01-24 16:42:23]

Failed to apply patch for tools/win/copy-installer.bat:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file tools/win/copy-installer.bat
  Hunk #1 FAILED at 1.
  1 out of 1 hunk FAILED -- saving rejects to file
tools/win/copy-installer.bat.rej

Patch:       tools/win/copy-installer.bat
Index: tools/win/copy-installer.bat
diff --git a/tools/win/copy-installer.bat b/tools/win/copy-installer.bat
index
e5be01ddf66db6b22a439b183c289f2f7373192f..b147a2610b14cb0010970986b1c7c7508eb71f22
100755
--- a/tools/win/copy-installer.bat
+++ b/tools/win/copy-installer.bat
@@ -1,4 +1,4 @@
-ECHO OFF
+ECHO ON
 
 REM Copyright (c) 2012 The Chromium Authors. All rights reserved.
 REM Use of this source code is governed by a BSD-style license that can be

[commit-bot: I haz the power, 2014-01-24 16:45:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/csharp@chromium.org/138903022/170006

[commit-bot: I haz the power, 2014-01-24 18:33:58]

Retried try job too often on win_rel for step(s) chrome_elf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2014-01-24 19:05:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/csharp@chromium.org/138903022/170006

[commit-bot: I haz the power, 2014-01-24 19:39:38]

Failed to trigger a try job on win_rel
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2014-01-24 19:39:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/csharp@chromium.org/138903022/190002

[commit-bot: I haz the power, 2014-01-24 21:17:23]

Change committed as 246965