Show content settings exceptions in correct order

Show content settings exceptions in correct order

The patterns are internally sorted from the lowest precedence to highest (i.e.
from the less specific pattern to the most specific, though the rules are more
complicated than that), and they were shown to the user in this order too,
which was confusing and making them think it's not sorted at all. Showing the
highest precedence patterns first should be more intuitive and also sort the
equivalent patterns more-or-less alphabetically.

BUG=450580
Committed: https://crrev.com/ea8dd94e6cb9ff95a9698adcc67da0e635a293e6
Cr-Commit-Position: refs/heads/master@{#324392}

[mkollaro, 2015-04-01 07:48:48]

martina.kollarova@intel.com changed reviewers:
+ dbeam@chromium.org, estade@chromium.org

[Dan Beam, 2015-04-01 20:53:50]

lgtm

[Evan Stade, 2015-04-01 23:22:06]

estade@chromium.org changed reviewers:
+ vabr@chromium.org

[Evan Stade, 2015-04-01 23:22:07]

+vabr instead of estade

[vabr (Chromium), 2015-04-02 08:20:59]

vabr@chromium.org changed reviewers:
+ markusheintz@chromium.org

[vabr (Chromium), 2015-04-02 08:21:00]

Markus,
Could you please OWNER-rubber-stamp
components/content_settings/core/common/content_settings_pattern_unittest.cc ?

Martina,
Thanks, LGTM, this is a nice improvement!

Could we also have tests for the right order in the UI? By a quick look, in
content_settings_exception_area_browsertest.* it should be possible to add them.

Cheers,
Václav

P.S. Full disclosure: I'm not actually an OWNER of any files here. :)

[Evan Stade, 2015-04-02 18:32:09]

On 2015/04/02 08:21:00, vabr (Chromium) wrote:
> Markus,
> Could you please OWNER-rubber-stamp
> components/content_settings/core/common/content_settings_pattern_unittest.cc ?
> 
> Martina,
> Thanks, LGTM, this is a nice improvement!
> 
> Could we also have tests for the right order in the UI? By a quick look, in
> content_settings_exception_area_browsertest.* it should be possible to add
them.
> 
> Cheers,
> Václav
> 
> P.S. Full disclosure: I'm not actually an OWNER of any files here. :)

oops, I guess the relevant OWNERS file is components/content_settings/OWNERS

[mkollaro, 2015-04-07 08:08:36]

martina.kollarova@intel.com changed reviewers:
+ bauerb@chromium.org

[mkollaro, 2015-04-07 10:57:34]

On 2015/04/02 08:21:00, vabr (Chromium) wrote:
> Markus,
> Could you please OWNER-rubber-stamp
> components/content_settings/core/common/content_settings_pattern_unittest.cc ?
> 
> Martina,
> Thanks, LGTM, this is a nice improvement!
> 
> Could we also have tests for the right order in the UI? By a quick look, in
> content_settings_exception_area_browsertest.* it should be possible to add
them.
> 
> Cheers,
> Václav
> 
> P.S. Full disclosure: I'm not actually an OWNER of any files here. :)

I created issue #474528 to add a browser test for this, I will probably do that
later.

[Bernhard Bauer, 2015-04-07 11:25:41]

lgtm

[vabr (Chromium), 2015-04-07 11:30:24]

On 2015/04/07 10:57:34, mkollaro wrote:
> On 2015/04/02 08:21:00, vabr (Chromium) wrote:
> > Markus,
> > Could you please OWNER-rubber-stamp
> > components/content_settings/core/common/content_settings_pattern_unittest.cc
?
> > 
> > Martina,
> > Thanks, LGTM, this is a nice improvement!
> > 
> > Could we also have tests for the right order in the UI? By a quick look, in
> > content_settings_exception_area_browsertest.* it should be possible to add
> them.
> > 
> > Cheers,
> > Václav
> > 
> > P.S. Full disclosure: I'm not actually an OWNER of any files here. :)
> 
> I created issue #474528 to add a browser test for this, I will probably do
that
> later.

OK, thanks for the issue ID.

Happy landing of this CL! :)

Václav

[mkollaro, 2015-04-08 07:49:56]

The CQ bit was checked by martina.kollarova@intel.com

[commit-bot: I haz the power, 2015-04-08 07:50:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1047153002/20001

[commit-bot: I haz the power, 2015-04-08 08:02:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-08 08:02:11]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[mkollaro, 2015-04-09 07:14:06]

The CQ bit was checked by martina.kollarova@intel.com

[commit-bot: I haz the power, 2015-04-09 07:14:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1047153002/20001

[commit-bot: I haz the power, 2015-04-09 07:18:55]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-04-09 07:19:35]

Patchset 2 (id:??) landed as
https://crrev.com/ea8dd94e6cb9ff95a9698adcc67da0e635a293e6
Cr-Commit-Position: refs/heads/master@{#324392}