Changing string id for CONTENT_SETTINGS_TYPE_COOKIES for block and allow case.

kBlockedAllowID should not have IDS_BLOCKED_COOKIES_UNBLOCK.
It has been changed to IDS_ALLOWED_COOKIES_NO_ACTION as when blocked id's are allowed which was not blocked earlier so 'unblock' is not appropriate.
Similarly
IDS_BLOCKED_COOKIES_NO_ACTION should get changed to IDS_ALLOWED_COOKIES_BLOCK for kAllowedBlockIDs , as it is allowing to Blocking the cookies.

BUG=496092
Committed: https://crrev.com/11d71f820fff62ad406cd5925c816e61403c70de
Cr-Commit-Position: refs/heads/master@{#333252}

[Deepak, 2015-06-03 12:20:11]

deepak.m1@samsung.com changed reviewers:
+ ankitkumar@chromium.org

[ankitkumar, 2015-06-03 13:03:08]

lgtm

LGTM

[Deepak, 2015-06-04 05:07:15]

deepak.m1@samsung.com changed reviewers:
+ bauerb@chromium.org

[Deepak, 2015-06-04 05:07:15]

@Bernhard
Please review.

[Bernhard Bauer, 2015-06-04 08:08:02]

https://codereview.chromium.org/1150343004/diff/1/chrome/browser/ui/content_s...
File chrome/browser/ui/content_settings/content_setting_bubble_model.cc (right):

https://codereview.chromium.org/1150343004/diff/1/chrome/browser/ui/content_s...
chrome/browser/ui/content_settings/content_setting_bubble_model.cc:307:
{CONTENT_SETTINGS_TYPE_COOKIES, IDS_BLOCKED_COOKIES_ALLOW},
This doesn't really change anything if the string value stays the same. If you
look at the code that uses this, this is the string for the case where cookies
are already allowed, and should stay allowed. This means that the right string
value would be "Continue allowing cookies" (cf. "Continue allowing unsandboxed
plugins" for PPAPI_BROKER below), and the string ID
IDS_ALLOWED_COOKIES_NO_ACTION.

https://codereview.chromium.org/1150343004/diff/1/chrome/browser/ui/content_s...
chrome/browser/ui/content_settings/content_setting_bubble_model.cc:338:
{CONTENT_SETTINGS_TYPE_COOKIES, IDS_BLOCKED_COOKIES_NO_ACTION},
Similarly, here the string ID should be IDS_ALLOWED_COOKIES_BLOCK, and the value
"Never allow <host> to show cookies".

[Deepak, 2015-06-04 10:09:10]

Changes done as suggested.
PTAL

https://codereview.chromium.org/1150343004/diff/1/chrome/browser/ui/content_s...
File chrome/browser/ui/content_settings/content_setting_bubble_model.cc (right):

https://codereview.chromium.org/1150343004/diff/1/chrome/browser/ui/content_s...
chrome/browser/ui/content_settings/content_setting_bubble_model.cc:307:
{CONTENT_SETTINGS_TYPE_COOKIES, IDS_BLOCKED_COOKIES_ALLOW},
On 2015/06/04 08:08:02, Bernhard Bauer wrote:
> This doesn't really change anything if the string value stays the same. If you
> look at the code that uses this, this is the string for the case where cookies
> are already allowed, and should stay allowed. This means that the right string
> value would be "Continue allowing cookies" (cf. "Continue allowing unsandboxed
> plugins" for PPAPI_BROKER below), and the string ID
> IDS_ALLOWED_COOKIES_NO_ACTION.

Done.

https://codereview.chromium.org/1150343004/diff/1/chrome/browser/ui/content_s...
chrome/browser/ui/content_settings/content_setting_bubble_model.cc:338:
{CONTENT_SETTINGS_TYPE_COOKIES, IDS_BLOCKED_COOKIES_NO_ACTION},
On 2015/06/04 08:08:02, Bernhard Bauer wrote:
> Similarly, here the string ID should be IDS_ALLOWED_COOKIES_BLOCK, and the
value
> "Never allow <host> to show cookies".

Done.

[Bernhard Bauer, 2015-06-04 10:15:09]

https://codereview.chromium.org/1150343004/diff/20001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1150343004/diff/20001/chrome/app/generated_re...
chrome/app/generated_resources.grd:2563: +        Never allow to show cookies on
<ph name="HOST">$1<ex>example.com</ex></ph>
The corresponding message to unblock cookies is "Always allow <host> to set
cookies", so this one should be "Never allow <host> to set cookies". I wrote
"show" in my initial comment, but that was an accident.

[Deepak, 2015-06-04 10:38:20]

PTAL.

https://codereview.chromium.org/1150343004/diff/20001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1150343004/diff/20001/chrome/app/generated_re...
chrome/app/generated_resources.grd:2563: +        Never allow to show cookies on
<ph name="HOST">$1<ex>example.com</ex></ph>
On 2015/06/04 10:15:08, Bernhard Bauer wrote:
> The corresponding message to unblock cookies is "Always allow <host> to set
> cookies", so this one should be "Never allow <host> to set cookies". I wrote
> "show" in my initial comment, but that was an accident.

Correct!!.
Changes done..

I also observe that using "Never allow" or "Always block' for blocking is matter
of choice, As

IDS_BLOCKED_DOWNLOAD_UNBLOCK uses  "Always allow.." 
IDS_ALLOWED_DOWNLOAD_BLOCK uses "Always block.."
But 
IDS_BLOCKED_PPAPI_BROKER_UNBLOCK uses  "Always allow"
IDS_ALLOWED_PPAPI_BROKER_BLOCK uses "Never allow"

[Bernhard Bauer, 2015-06-04 11:51:15]

https://codereview.chromium.org/1150343004/diff/20001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1150343004/diff/20001/chrome/app/generated_re...
chrome/app/generated_resources.grd:2563: +        Never allow to show cookies on
<ph name="HOST">$1<ex>example.com</ex></ph>
On 2015/06/04 10:38:20, Deepak wrote:
> On 2015/06/04 10:15:08, Bernhard Bauer wrote:
> > The corresponding message to unblock cookies is "Always allow <host> to set
> > cookies", so this one should be "Never allow <host> to set cookies". I wrote
> > "show" in my initial comment, but that was an accident.
> 
> Correct!!.
> Changes done..
> 
> I also observe that using "Never allow" or "Always block' for blocking is
matter
> of choice, As
> 
> IDS_BLOCKED_DOWNLOAD_UNBLOCK uses  "Always allow.." 
> IDS_ALLOWED_DOWNLOAD_BLOCK uses "Always block.."
> But 
> IDS_BLOCKED_PPAPI_BROKER_UNBLOCK uses  "Always allow"
> IDS_ALLOWED_PPAPI_BROKER_BLOCK uses "Never allow"

Good catch! Now that you mention it, I find more instances of "Always block"
than "Never allow" in the strings file -- PPAPI_BROKER seems to be the only type
where this is used. Maybe we should change it to "Always block" then. Sorry for
the abrupt change in direction.

https://codereview.chromium.org/1150343004/diff/40001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1150343004/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:2559: +      <message
name="IDS_ALLOWED_COOKIES_NO_ACTION" desc="Radio button to keep allowing the
cookies">
Can you add the additional context to the desc label like for
IDS_BLOCKED_COOKIES_NO_ACTION? This will help translators.

[Deepak, 2015-06-04 12:12:46]

Changes Done.

https://codereview.chromium.org/1150343004/diff/40001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1150343004/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:2559: +      <message
name="IDS_ALLOWED_COOKIES_NO_ACTION" desc="Radio button to keep allowing the
cookies">
On 2015/06/04 11:51:15, Bernhard Bauer wrote:
> Can you add the additional context to the desc label like for
> IDS_BLOCKED_COOKIES_NO_ACTION? This will help translators.

Done.

[Bernhard Bauer, 2015-06-04 14:21:34]

LGTM, thanks!

[Deepak, 2015-06-05 02:41:56]

The CQ bit was checked by deepak.m1@samsung.com

[Deepak, 2015-06-05 02:41:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
ankitkumar@chromium.org
Link to the patchset: https://codereview.chromium.org/1150343004/#ps60001
(title: "Changes as per review comments.")

[commit-bot: I haz the power, 2015-06-05 02:42:22]

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

[commit-bot: I haz the power, 2015-06-05 03:22:32]

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

[commit-bot: I haz the power, 2015-06-05 03:22:33]

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

[Deepak, 2015-06-05 09:26:05]

On 2015/06/05 03:22:33, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux
(JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

@Bernhard
PTAL content_setting_bubble_model_unittest.cc changes.

[Bernhard Bauer, 2015-06-05 09:46:28]

Not lgtm at the moment.

If you look at the test run before your changes to the unit test, there is this
failure:

[ RUN      ] ContentSettingBubbleModelTest.Cookies
../../chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc:119:
Failure
Value of: bubble_content_2.radio_group.radio_items[0]
  Actual: "Continue allowing cookies"
Expected: radio1
Which is: "Always allow  to set cookies"
../../chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc:120:
Failure
Value of: bubble_content_2.radio_group.radio_items[1]
  Actual: "Always block $1 to set cookies"
Expected: radio2
Which is: "Continue blocking cookies"
[  FAILED  ] ContentSettingBubbleModelTest.Cookies (20 ms)

"Always allow  to set cookies" is wrong, and so is "Always block $1 to set
cookies". The code that formats the string needs to be modified so it doesn't
special-case cookies anymore. That is probably also the reason why your tests
are failing.

https://codereview.chromium.org/1150343004/diff/80001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1150343004/diff/80001/chrome/app/generated_re...
chrome/app/generated_resources.grd:2563: +        Always block <ph
name="HOST">$1<ex>mail.google.com</ex></ph> to set cookies
This should probably be "Always block cookies on <host>".

[Deepak, 2015-06-05 10:33:49]

The CQ bit was checked by deepak.m1@samsung.com to run a CQ dry run

[Deepak, 2015-06-05 10:33:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
ankitkumar@chromium.org
Link to the patchset: https://codereview.chromium.org/1150343004/#ps100001
(title: " ")

[commit-bot: I haz the power, 2015-06-05 10:34:08]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1150343004/100001

[commit-bot: I haz the power, 2015-06-05 10:34:12]

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

[commit-bot: I haz the power, 2015-06-05 10:34:13]

Dry run: A disapproval has been posted by following reviewers:
bauerb@chromium.org.
Please make sure to get positive review before another CQ attempt.

[Deepak, 2015-06-05 10:34:18]

The CQ bit was unchecked by deepak.m1@samsung.com

[Deepak, 2015-06-05 10:45:10]

On 2015/06/05 09:46:28, Bernhard Bauer wrote:
> Not lgtm at the moment.
> 
> If you look at the test run before your changes to the unit test, there is
this
> failure:
> 
> [ RUN      ] ContentSettingBubbleModelTest.Cookies
>
../../chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc:119:
> Failure
> Value of: bubble_content_2.radio_group.radio_items[0]
>   Actual: "Continue allowing cookies"
> Expected: radio1
> Which is: "Always allow  to set cookies"
>
../../chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc:120:
> Failure
> Value of: bubble_content_2.radio_group.radio_items[1]
>   Actual: "Always block $1 to set cookies"
> Expected: radio2
> Which is: "Continue blocking cookies"
> [  FAILED  ] ContentSettingBubbleModelTest.Cookies (20 ms)
> 
> "Always allow  to set cookies" is wrong, and so is "Always block $1 to set
> cookies". The code that formats the string needs to be modified so it doesn't
> special-case cookies anymore. That is probably also the reason why your tests
> are failing.
> 
>
https://codereview.chromium.org/1150343004/diff/80001/chrome/app/generated_re...
> File chrome/app/generated_resources.grd (right):
> 
>
https://codereview.chromium.org/1150343004/diff/80001/chrome/app/generated_re...
> chrome/app/generated_resources.grd:2563: +        Always block <ph
> name="HOST">$1<ex>mail.google.com</ex></ph> to set cookies
> This should probably be "Always block cookies on <host>".

Actually this is not the problem in patch.
some issue in the build bot.
"linux_android_rel_ng" is  getting failed in #5 
I have run again failed build bot, It is passing all the test cases of
"linux_android_rel_ng" in #6 
that was failing earlier with the same changes.

[Bernhard Bauer, 2015-06-05 11:10:48]

On 2015/06/05 10:45:10, Deepak wrote:
> On 2015/06/05 09:46:28, Bernhard Bauer wrote:
> > Not lgtm at the moment.
> > 
> > If you look at the test run before your changes to the unit test, there is
> this
> > failure:
> > 
> > [ RUN      ] ContentSettingBubbleModelTest.Cookies
> >
>
../../chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc:119:
> > Failure
> > Value of: bubble_content_2.radio_group.radio_items[0]
> >   Actual: "Continue allowing cookies"
> > Expected: radio1
> > Which is: "Always allow  to set cookies"
> >
>
../../chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc:120:
> > Failure
> > Value of: bubble_content_2.radio_group.radio_items[1]
> >   Actual: "Always block $1 to set cookies"
> > Expected: radio2
> > Which is: "Continue blocking cookies"
> > [  FAILED  ] ContentSettingBubbleModelTest.Cookies (20 ms)
> > 
> > "Always allow  to set cookies" is wrong, and so is "Always block $1 to set
> > cookies". The code that formats the string needs to be modified so it
doesn't
> > special-case cookies anymore. That is probably also the reason why your
tests
> > are failing.
> > 
> >
>
https://codereview.chromium.org/1150343004/diff/80001/chrome/app/generated_re...
> > File chrome/app/generated_resources.grd (right):
> > 
> >
>
https://codereview.chromium.org/1150343004/diff/80001/chrome/app/generated_re...
> > chrome/app/generated_resources.grd:2563: +        Always block <ph
> > name="HOST">$1<ex>mail.google.com</ex></ph> to set cookies
> > This should probably be "Always block cookies on <host>".
> 
> Actually this is not the problem in patch.
> some issue in the build bot.
> "linux_android_rel_ng" is  getting failed in #5 
> I have run again failed build bot, It is passing all the test cases of
> "linux_android_rel_ng" in #6 
> that was failing earlier with the same changes.

I see... but the strings shown in the bubble will still be wrong. :) We just
don't notice this anymore now that you've modified the test.

[Deepak, 2015-06-05 11:20:57]

On 2015/06/05 11:10:48, Bernhard Bauer wrote:
> On 2015/06/05 10:45:10, Deepak wrote:
> > On 2015/06/05 09:46:28, Bernhard Bauer wrote:
> > > Not lgtm at the moment.
> > > 
> > > If you look at the test run before your changes to the unit test, there is
> > this
> > > failure:
> > > 
> > > [ RUN      ] ContentSettingBubbleModelTest.Cookies
> > >
> >
>
../../chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc:119:
> > > Failure
> > > Value of: bubble_content_2.radio_group.radio_items[0]
> > >   Actual: "Continue allowing cookies"
> > > Expected: radio1
> > > Which is: "Always allow  to set cookies"
> > >
> >
>
../../chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc:120:
> > > Failure
> > > Value of: bubble_content_2.radio_group.radio_items[1]
> > >   Actual: "Always block $1 to set cookies"
> > > Expected: radio2
> > > Which is: "Continue blocking cookies"
> > > [  FAILED  ] ContentSettingBubbleModelTest.Cookies (20 ms)
> > > 
> > > "Always allow  to set cookies" is wrong, and so is "Always block $1 to set
> > > cookies". The code that formats the string needs to be modified so it
> doesn't
> > > special-case cookies anymore. That is probably also the reason why your
> tests
> > > are failing.
> > > 
> > >
> >
>
https://codereview.chromium.org/1150343004/diff/80001/chrome/app/generated_re...
> > > File chrome/app/generated_resources.grd (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1150343004/diff/80001/chrome/app/generated_re...
> > > chrome/app/generated_resources.grd:2563: +        Always block <ph
> > > name="HOST">$1<ex>mail.google.com</ex></ph> to set cookies
> > > This should probably be "Always block cookies on <host>".
> > 
> > Actually this is not the problem in patch.
> > some issue in the build bot.
> > "linux_android_rel_ng" is  getting failed in #5 
> > I have run again failed build bot, It is passing all the test cases of
> > "linux_android_rel_ng" in #6 
> > that was failing earlier with the same changes.
> 
> I see... but the strings shown in the bubble will still be wrong. :) We just
> don't notice this anymore now that you've modified the test.

Actually in the test case, Your comment is "Update this once the strings have
been updated".
And as the check is EXPECT_NE(), as for TEST_F(ContentSettingBubbleModelTest,
PepperBroker).
so I feel we don't have to do change for format strings.
I am not able to run "git cl try' and CQ dry run enables CQ bits by default.
Please correct me if I misunderstood something.

[Bernhard Bauer, 2015-06-05 11:28:51]

On 2015/06/05 11:20:57, Deepak wrote:
> On 2015/06/05 11:10:48, Bernhard Bauer wrote:
> > On 2015/06/05 10:45:10, Deepak wrote:
> > > On 2015/06/05 09:46:28, Bernhard Bauer wrote:
> > > > Not lgtm at the moment.
> > > > 
> > > > If you look at the test run before your changes to the unit test, there
is
> > > this
> > > > failure:
> > > > 
> > > > [ RUN      ] ContentSettingBubbleModelTest.Cookies
> > > >
> > >
> >
>
../../chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc:119:
> > > > Failure
> > > > Value of: bubble_content_2.radio_group.radio_items[0]
> > > >   Actual: "Continue allowing cookies"
> > > > Expected: radio1
> > > > Which is: "Always allow  to set cookies"
> > > >
> > >
> >
>
../../chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc:120:
> > > > Failure
> > > > Value of: bubble_content_2.radio_group.radio_items[1]
> > > >   Actual: "Always block $1 to set cookies"
> > > > Expected: radio2
> > > > Which is: "Continue blocking cookies"
> > > > [  FAILED  ] ContentSettingBubbleModelTest.Cookies (20 ms)
> > > > 
> > > > "Always allow  to set cookies" is wrong, and so is "Always block $1 to
set
> > > > cookies". The code that formats the string needs to be modified so it
> > doesn't
> > > > special-case cookies anymore. That is probably also the reason why your
> > tests
> > > > are failing.
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/1150343004/diff/80001/chrome/app/generated_re...
> > > > File chrome/app/generated_resources.grd (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/1150343004/diff/80001/chrome/app/generated_re...
> > > > chrome/app/generated_resources.grd:2563: +        Always block <ph
> > > > name="HOST">$1<ex>mail.google.com</ex></ph> to set cookies
> > > > This should probably be "Always block cookies on <host>".
> > > 
> > > Actually this is not the problem in patch.
> > > some issue in the build bot.
> > > "linux_android_rel_ng" is  getting failed in #5 
> > > I have run again failed build bot, It is passing all the test cases of
> > > "linux_android_rel_ng" in #6 
> > > that was failing earlier with the same changes.
> > 
> > I see... but the strings shown in the bubble will still be wrong. :) We just
> > don't notice this anymore now that you've modified the test.
> 
> Actually in the test case, Your comment is "Update this once the strings have
> been updated".
> And as the check is EXPECT_NE(), as for TEST_F(ContentSettingBubbleModelTest,
> PepperBroker).
> so I feel we don't have to do change for format strings.

The test does need to be updated, but not in the way you do. The strings right
now are wrong. You need to remove the special-casing for cookies when formatting
the strings now, because IDS_ALLOWED_COOKIES_BLOCK has a placeholder that needs
to be replaced, and IDS_ALLOWED_COOKIES_NO_ACTION has no placeholder (but the
code currently tries to replace one).

> I am not able to run "git cl try' and CQ dry run enables CQ bits by default.
> Please correct me if I misunderstood something.

Why does `git cl try` not work? Also, you could run tests yourself locally.

[Deepak, 2015-06-05 12:51:51]

I understand,Thanks for clarification.
I have done the changes for string and run unit_tests cases locally. All are
getting passed now.
'git cl try' that run all the test case have some issue, for all due to some
one's checkin.
no one is able to run 'git cl try' as discussion is happening on chromium dev.

PTAL

[Deepak, 2015-06-05 13:33:24]

On 2015/06/05 12:51:51, Deepak wrote:
> I understand,Thanks for clarification.
> I have done the changes for string and run unit_tests cases locally. All are
> getting passed now.
> 'git cl try' that run all the test case have some issue, for all due to some
> one's checkin.
> no one is able to run 'git cl try' as discussion is happening on chromium dev.
> 
> PTAL

Luckily try bot is working now.
I have fired build on it.
Thanks

[Bernhard Bauer, 2015-06-05 14:23:47]

https://codereview.chromium.org/1150343004/diff/120001/chrome/app/generated_r...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1150343004/diff/120001/chrome/app/generated_r...
chrome/app/generated_resources.grd:2563: +        Always block <ph
name="HOST">$1<ex>mail.google.com</ex></ph> to set cookies
You probably missed my comment here: This isn't grammatically correct, it should
be "Always block cookies on <host>".

https://codereview.chromium.org/1150343004/diff/120001/chrome/browser/ui/cont...
File chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc
(right):

https://codereview.chromium.org/1150343004/diff/120001/chrome/browser/ui/cont...
chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc:118:
EXPECT_NE(radio1, bubble_content_2.radio_group.radio_items[0]);
Can we update this test so it tests something more meaningful than the messages
being different? At the risk of making it too brittle, I would suggest manually
constructing the expected message (via GetString(F)) and comparing that.

[Deepak, 2015-06-08 05:53:31]

Changes done as suggested.
PTAL

https://codereview.chromium.org/1150343004/diff/120001/chrome/app/generated_r...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1150343004/diff/120001/chrome/app/generated_r...
chrome/app/generated_resources.grd:2563: +        Always block <ph
name="HOST">$1<ex>mail.google.com</ex></ph> to set cookies
On 2015/06/05 14:23:46, Bernhard Bauer wrote:
> You probably missed my comment here: This isn't grammatically correct, it
should
> be "Always block cookies on <host>".

my apologies..
now changes done.

https://codereview.chromium.org/1150343004/diff/120001/chrome/browser/ui/cont...
File chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc
(right):

https://codereview.chromium.org/1150343004/diff/120001/chrome/browser/ui/cont...
chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc:118:
EXPECT_NE(radio1, bubble_content_2.radio_group.radio_items[0]);
On 2015/06/05 14:23:46, Bernhard Bauer wrote:
> Can we update this test so it tests something more meaningful than the
messages
> being different? At the risk of making it too brittle, I would suggest
manually
> constructing the expected message (via GetString(F)) and comparing that.

Done.

[Bernhard Bauer, 2015-06-08 07:59:39]

LGTM!

[Deepak, 2015-06-08 08:00:42]

On 2015/06/08 07:59:39, Bernhard Bauer wrote:
> LGTM!

Thank you.

[Deepak, 2015-06-08 08:00:44]

The CQ bit was checked by deepak.m1@samsung.com

[Deepak, 2015-06-08 08:00:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
ankitkumar@chromium.org
Link to the patchset: https://codereview.chromium.org/1150343004/#ps140001
(title: "Changes as per review comments.")

[commit-bot: I haz the power, 2015-06-08 08:01:01]

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

[commit-bot: I haz the power, 2015-06-08 08:43:05]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-06-08 08:43:50]

Patchset 8 (id:??) landed as
https://crrev.com/11d71f820fff62ad406cd5925c816e61403c70de
Cr-Commit-Position: refs/heads/master@{#333252}