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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 8 months ago by Finnur (OOO - vacation)
Modified:
4 years, 1 month ago
Reviewers:
huanr, Nico (hiding)
CC:
chromium-reviews, Paweł Hajdan Jr., arv (Not doing code reviews), 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) Patch
M chrome/app/generated_resources.grd View 1 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/enumerate_modules_model_unittest_win.cc View 1 1 chunk +11 lines, -1 line 0 comments Download
M chrome/browser/enumerate_modules_model_win.h View 1 2 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/enumerate_modules_model_win.cc View 1 7 chunks +31 lines, -24 lines 0 comments Download
M chrome/browser/resources/about_conflicts.html View 1 2 chunks +55 lines, -56 lines 3 comments Download
Commit: CQ not working?

Messages

Total messages: 7 (0 generated)
Finnur (OOO - vacation)
Nico, can you take a look at the about_conflicts.html page (I'll send you a screenshot ...
4 years, 8 months ago (2010-11-26 11:55:03 UTC) #1
Finnur (OOO - vacation)
PS. Huan, you alread have an email with the screenshot, by the way. On 2010/11/26 ...
4 years, 8 months ago (2010-11-26 11:59:23 UTC) #2
Nico (hiding)
I realize that the '>' was in the old version as well, but I didn't ...
4 years, 8 months ago (2010-11-26 17:54:36 UTC) #3
Finnur (OOO - vacation)
Huan, ping? On 2010/11/26 17:54:36, Nico wrote: > I realize that the '>' was in ...
4 years, 8 months ago (2010-11-29 19:07:05 UTC) #4
Finnur (OOO - vacation)
Nico, please take another look. I've addressed all your comments. Huan, this upload shouldn't invalidate ...
4 years, 8 months ago (2010-11-29 21:54:31 UTC) #5
Nico (hiding)
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 ...
4 years, 8 months ago (2010-11-29 22:02:34 UTC) #6
huanr
4 years, 8 months ago (2010-11-29 22:09:40 UTC) #7
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 5fa3ca5