Warn by default when certain DLL conflicts exist on XP.

Warn by default when certain DLL conflicts exist on XP.

BUG=238378
R=cpu@chromium.org, jhawkins@chromium.org, sky@chromium.org

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

[Finnur, 2013-06-03 22:10:50]

Carlos, can you take this? (having done a related review in the past plus you
understand the urgency of this).

Scott, can you do an OWNERS review for chrome/browser/ui and review the new
files (the bubble).

James, can you do an OWNERS check on the one-liner on 	
chrome/browser/resources/about_conflicts.html
(I'm just removing the text because it is meant as a developer aid, but is seen
briefly by the user while loading so it creates a negative distraction).

[James Hawkins, 2013-06-03 22:40:09]

LGTM

[sky, 2013-06-03 22:58:00]

LGTM

https://codereview.chromium.org/15969017/diff/4018/chrome/browser/ui/views/co...
File chrome/browser/ui/views/conflicting_module_view_win.h (right):

https://codereview.chromium.org/15969017/diff/4018/chrome/browser/ui/views/co...
chrome/browser/ui/views/conflicting_module_view_win.h:1: // Copyright (c) 2012
The Chromium Authors. All rights reserved.
2013

https://codereview.chromium.org/15969017/diff/4018/chrome/browser/ui/views/co...
chrome/browser/ui/views/conflicting_module_view_win.h:5: #ifndef
CHROME_BROWSER_UI_VIEWS_EXTENSIONS_DISABLED_EXTENSIONS_VIEW_WIN_H_
Looks like you copied this.

https://codereview.chromium.org/15969017/diff/4018/chrome/browser/ui/views/co...
chrome/browser/ui/views/conflicting_module_view_win.h:22: // is a 3rd party
module loaded that conflicts with Google Chrome.
Remove 'Google' from here.

https://codereview.chromium.org/15969017/diff/4018/chrome/browser/ui/views/co...
chrome/browser/ui/views/conflicting_module_view_win.h:56: void Observe(
virtual void OVERRIDE?

[cpu_(ooo_6.6-7.5), 2013-06-04 03:01:57]

lgtm

https://codereview.chromium.org/15969017/diff/4018/chrome/browser/ui/toolbar/...
File chrome/browser/ui/toolbar/wrench_menu_model.cc (right):

https://codereview.chromium.org/15969017/diff/4018/chrome/browser/ui/toolbar/...
chrome/browser/ui/toolbar/wrench_menu_model.cc:483:
AddItem(IDC_VIEW_INCOMPATIBILITIES, l10n_util::GetStringUTF16(
l10n_util::GetStringUTF16( in the second line looks prettier, but whatever.

https://codereview.chromium.org/15969017/diff/4018/chrome/browser/ui/views/co...
File chrome/browser/ui/views/conflicting_module_view_win.cc (right):

https://codereview.chromium.org/15969017/diff/4018/chrome/browser/ui/views/co...
chrome/browser/ui/views/conflicting_module_view_win.cc:76:
EnumerateModulesModel* model = EnumerateModulesModel::GetInstance();
is this slow?

https://codereview.chromium.org/15969017/diff/4018/chrome/browser/ui/views/co...
chrome/browser/ui/views/conflicting_module_view_win.cc:140:
upper_columns->AddPaddingColumn(0, 7);
is that 7 one of our constants? Or can it be rephrased as one, kRelatedFoo + 1
kind of thing at least?

[Finnur, 2013-06-04 15:51:16]

All comments addressed.

https://codereview.chromium.org/15969017/diff/4018/chrome/browser/ui/views/co...
File chrome/browser/ui/views/conflicting_module_view_win.cc (right):

https://codereview.chromium.org/15969017/diff/4018/chrome/browser/ui/views/co...
chrome/browser/ui/views/conflicting_module_view_win.cc:76:
EnumerateModulesModel* model = EnumerateModulesModel::GetInstance();
The answer to that is: it's complicated. :)

The actual creation of this object is very fast. I timed this with
GetTickCount() on the debug build on my Win7 machine and it took less than a
millisecond every single time, as I'd expect it to (since it doesn't do much
immediately). However, it sets in motion a process (scanning modules) that runs
on the file thread further down the line and that process is more
time-consuming.

We are fortunate enough, since the scanner is already implemented, to know
exactly how long this takes by looking at UMA data
(Conflicts.EnumerationTotalTime) and it shows we have about ten and a half
thousand timing samples, from people who opted in through about:flags. Results:

For a quarter (25%) of our users it takes less than 230 ms to enumerate and
compare all modules on the system against the blacklist, sort the list and
report back to the observers.
For a slim majority of users this process takes less than 755 ms.
Once we get to the 80th percentile (of users) we're seeing numbers like 3000 ms
and then it starts tapering off into the long tail from there.

The question is, will this have an adverse effect on:
1) Performance testing bots on XP?
2) Perceived performance by users on XP?
3) Performance testing in the wild on XP?

I think we'll need to pick a conservative delay (some minutes after startup) in
an effort to decrease the likelihood that this will affect the above.

[cpu_(ooo_6.6-7.5), 2013-06-04 17:30:39]

sounds reasonable, brettw made it so that is harder to clog the file thread.

I think we go as is (with, yes some delay) and measure.

- RLZ is like 10 seconds
- Several things start at 30 seconds
- Several other crap start at 1 min
- Component updater starts at 6 minutes
- Extension updater starts randomly

I say at about 45 seconds could be a place.

[commit-bot: I haz the power, 2013-06-06 12:08:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/finnur@chromium.org/15969017/50002

[Finnur, 2013-06-06 15:29:38]

Committed patchset #11 manually as r204505 (presubmit successful).