Chromium Code Reviews
Help | Chromium Project | Sign in
(157)

Issue 5278012: EnumerateModules: Address UI review comments.... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 4 months ago by Finnur
Modified:
2 years, 10 months ago
Reviewers:
huanr, Nico
CC:
chromium-reviews_chromium.org, Paweł Hajdan Jr., arv, ben+cc_chromium.org
Visibility:
Public.

Description

EnumerateModules: Address UI review comments.

Make it a simple list with columns. Sort by status (conflicts at the
top), then by location and module name. Also, convert the type of DLL
to an enum so we can dedup the list (and for non-loaded DLLs show that
they are not loaded yet, but are of type: Shell Extension / WinSock.

Make sure lower bound version specified on the blacklist is inclusive
(first version that broke) and the higher bound version is exclusive
(first version that worked).

This allows us to pointpoint exactly when the failure started and
when the fix was introduced, instead of doing 0.9999 shenanigans.

Specify an upper bound for the idmmbc.dll conflict, since Henry's
outreach produced a fix in version 6.03 of the download manager.

BUG=51105
TEST=Unit test


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=67598

Patch Set 1 #

Total comments: 3

Patch Set 2 : '' #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+105 lines, -89 lines) Lint Patch
M chrome/app/generated_resources.grd View 1 2 chunks +3 lines, -3 lines 0 comments 0 errors Download
M chrome/browser/enumerate_modules_model_unittest_win.cc View 1 1 chunk +11 lines, -1 line 0 comments 0 errors Download
M chrome/browser/enumerate_modules_model_win.h View 1 2 chunks +5 lines, -5 lines 0 comments 0 errors Download
M chrome/browser/enumerate_modules_model_win.cc View 1 7 chunks +31 lines, -24 lines 0 comments 0 errors Download
M chrome/browser/resources/about_conflicts.html View 1 2 chunks +55 lines, -56 lines 3 comments 0 errors Download
Commit:

Messages

Total messages: 7
Finnur
Nico, can you take a look at the about_conflicts.html page (I'll send you a screenshot ...
3 years, 4 months ago #1
Finnur
PS. Huan, you alread have an email with the screenshot, by the way. On 2010/11/26 ...
3 years, 4 months ago #2
Nico
I realize that the '>' was in the old version as well, but I didn't ...
3 years, 4 months ago #3
Finnur
Huan, ping? On 2010/11/26 17:54:36, Nico wrote: > I realize that the '>' was in ...
3 years, 4 months ago #4
Finnur
Nico, please take another look. I've addressed all your comments. Huan, this upload shouldn't invalidate ...
3 years, 4 months ago #5
Nico
LG Maybe you can add a few TODOs for the issues below. http://codereview.chromium.org/5278012/diff/10001/chrome/browser/resources/about_conflicts.html File chrome/browser/resources/about_conflicts.html ...
3 years, 4 months ago #6
huanr
3 years, 4 months ago #7
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6