EnumerateModules: Address UI review comments....

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

[Finnur, 2010-11-26 11:55:03]

Nico, can you take a look at the about_conflicts.html page (I'll send you a
screenshot of how it looks now).

Huan, can you review the rest.

[Finnur, 2010-11-26 11:59:23]

PS. Huan, you alread have an email with the screenshot, by the way.

On 2010/11/26 11:55:03, Finnur wrote:
> Nico, can you take a look at the about_conflicts.html page (I'll send you a
> screenshot of how it looks now).
> 
> Huan, can you review the rest.

[Nico, 2010-11-26 17:54:36]

I realize that the '>' was in the old version as well, but I didn't notice it
before.

I probably won't be able to do another iteration today, I'm afk from now on.

http://codereview.chromium.org/5278012/diff/1/chrome/browser/resources/about_...
File chrome/browser/resources/about_conflicts.html (right):

http://codereview.chromium.org/5278012/diff/1/chrome/browser/resources/about_...
chrome/browser/resources/about_conflicts.html:240: <span jsdisplay="status == 2
|| status == 3"><br><nobr>
can use use css instead of <br> tags for vertical layout? same question for nobr
(it's white-space: nowrap in this case i believe)

http://codereview.chromium.org/5278012/diff/1/chrome/browser/resources/about_...
chrome/browser/resources/about_conflicts.html:247: <a jsdisplay="help_url.length
> 0"
do you need to escape the >, i.e. &gt;? (
http://htmlhelp.com/tools/validator/problems.html#amp ).

http://codereview.chromium.org/5278012/diff/1/chrome/browser/resources/about_...
chrome/browser/resources/about_conflicts.html:248:
jsvalues=".href:help_url"><span
dedent 3?

[Finnur, 2010-11-29 19:07:05]

Huan, ping?

On 2010/11/26 17:54:36, Nico wrote:
> I realize that the '>' was in the old version as well, but I didn't notice it
> before.
> 
> I probably won't be able to do another iteration today, I'm afk from now on.
> 
>
http://codereview.chromium.org/5278012/diff/1/chrome/browser/resources/about_...
> File chrome/browser/resources/about_conflicts.html (right):
> 
>
http://codereview.chromium.org/5278012/diff/1/chrome/browser/resources/about_...
> chrome/browser/resources/about_conflicts.html:240: <span jsdisplay="status ==
2
> || status == 3"><br><nobr>
> can use use css instead of <br> tags for vertical layout? same question for
nobr
> (it's white-space: nowrap in this case i believe)
> 
>
http://codereview.chromium.org/5278012/diff/1/chrome/browser/resources/about_...
> chrome/browser/resources/about_conflicts.html:247: <a
jsdisplay="help_url.length
> > 0"
> do you need to escape the >, i.e. &gt;? (
> http://htmlhelp.com/tools/validator/problems.html#amp ).
> 
>
http://codereview.chromium.org/5278012/diff/1/chrome/browser/resources/about_...
> chrome/browser/resources/about_conflicts.html:248:
> jsvalues=".href:help_url"><span
> dedent 3?

[Finnur, 2010-11-29 21:54:31]

Nico, please take another look. I've addressed all your comments.

Huan, this upload shouldn't invalidate your review since it only changes the
html Nico is reviewing (and pulls in the latest changes from tot that others
have been making).


On 2010/11/29 19:07:05, Finnur wrote:
> Huan, ping?
> 
> On 2010/11/26 17:54:36, Nico wrote:
> > I realize that the '>' was in the old version as well, but I didn't notice
it
> > before.
> > 
> > I probably won't be able to do another iteration today, I'm afk from now on.
> > 
> >
>
http://codereview.chromium.org/5278012/diff/1/chrome/browser/resources/about_...
> > File chrome/browser/resources/about_conflicts.html (right):
> > 
> >
>
http://codereview.chromium.org/5278012/diff/1/chrome/browser/resources/about_...
> > chrome/browser/resources/about_conflicts.html:240: <span jsdisplay="status
==
> 2
> > || status == 3"><br><nobr>
> > can use use css instead of <br> tags for vertical layout? same question for
> nobr
> > (it's white-space: nowrap in this case i believe)
> > 
> >
>
http://codereview.chromium.org/5278012/diff/1/chrome/browser/resources/about_...
> > chrome/browser/resources/about_conflicts.html:247: <a
> jsdisplay="help_url.length
> > > 0"
> > do you need to escape the >, i.e. &gt;? (
> > http://htmlhelp.com/tools/validator/problems.html#amp ).
> > 
> >
>
http://codereview.chromium.org/5278012/diff/1/chrome/browser/resources/about_...
> > chrome/browser/resources/about_conflicts.html:248:
> > jsvalues=".href:help_url"><span
> > dedent 3?

[Nico, 2010-11-29 22:02:34]

LG

Maybe you can add a few TODOs for the issues below.

http://codereview.chromium.org/5278012/diff/10001/chrome/browser/resources/ab...
File chrome/browser/resources/about_conflicts.html (right):

http://codereview.chromium.org/5278012/diff/10001/chrome/browser/resources/ab...
chrome/browser/resources/about_conflicts.html:143: margin-top: -1em;
i'm a bit surprised by the negative top margin. maybe you want box-sizing:
border-box and padding instead of margin…but without known why this is here,
it's hard to guess :-)

http://codereview.chromium.org/5278012/diff/10001/chrome/browser/resources/ab...
chrome/browser/resources/about_conflicts.html:252: class="extra-info-text"><br>
same here

http://codereview.chromium.org/5278012/diff/10001/chrome/browser/resources/ab...
chrome/browser/resources/about_conflicts.html:262: <span
jsdisplay="possibleResolution.length &gt; 0"><br>
this can probably be done with clear:left (or right in rtl) and/or changing
display…maybe you can clean this up in a followup cl

[huanr, 2010-11-29 22:09:40]

LGTM