Use GURLs instead of patterns in SetContentSetting in notifications

Use GURLs instead of patterns in SetContentSetting in notification

DesktopNotificationProfileUtil::ClearSetting() is changed to take GURLs directly
which internally call HostContentSettingsMap::SetContentSettingDefaultScope().

For case in MessageCenterSettingsController, since the patterns are from user
input, SetContentSetting() is used instead of ClearSetting() to take patterns
directly.

BUG=551747
Committed: https://crrev.com/71deb5f7cacc66ee3c901ac406db75c5f1eb83a4
Cr-Commit-Position: refs/heads/master@{#381608}

[lshang, 2016-03-07 04:35:32]

Description was changed from

==========
Use GURL in SetContentSetting in notification, papi_broker and durable_storage


some more change


format


change setcontentsetting to use urls

BUG=
==========

to

==========
Use GURL in SetContentSetting in notification

DesktopNotificationProfileUtil::ClearSetting() is changed to take GURLs directly
which internally call HostContentSettingsMap::SetContentSettingDefaultScope().

patterns_ is also removed from MessageCenterSettingsController. It is a vector 
of pairs from urls->patterns, which is introduced in the CL:
https://chromiumcodereview.appspot.com/12879016
to keep the original patterns from pattern->url->pattern inconsistency.
Since now urls are directly used in ClearSetting(), we removed patterns_.


BUG=551747
==========

[lshang, 2016-03-07 04:37:09]

Description was changed from

==========
Use GURL in SetContentSetting in notification

DesktopNotificationProfileUtil::ClearSetting() is changed to take GURLs directly
which internally call HostContentSettingsMap::SetContentSettingDefaultScope().

patterns_ is also removed from MessageCenterSettingsController. It is a vector 
of pairs from urls->patterns, which is introduced in the CL:
https://chromiumcodereview.appspot.com/12879016
to keep the original patterns from pattern->url->pattern inconsistency.
Since now urls are directly used in ClearSetting(), we removed patterns_.


BUG=551747
==========

to

==========
Use GURLs instead of patterns in SetContentSetting in notification

DesktopNotificationProfileUtil::ClearSetting() is changed to take GURLs directly
which internally call HostContentSettingsMap::SetContentSettingDefaultScope().

patterns_ is also removed from MessageCenterSettingsController. It is a vector 
of pairs from urls->patterns, which is introduced in the CL:
https://chromiumcodereview.appspot.com/12879016
to keep the original patterns from pattern->url->pattern inconsistency.
Since now urls are directly used in ClearSetting(), we removed patterns_.


BUG=551747
==========

[lshang, 2016-03-07 04:38:49]

Description was changed from

==========
Use GURLs instead of patterns in SetContentSetting in notification

DesktopNotificationProfileUtil::ClearSetting() is changed to take GURLs directly
which internally call HostContentSettingsMap::SetContentSettingDefaultScope().

patterns_ is also removed from MessageCenterSettingsController. It is a vector 
of pairs from urls->patterns, which is introduced in the CL:
https://chromiumcodereview.appspot.com/12879016
to keep the original patterns from pattern->url->pattern inconsistency.
Since now urls are directly used in ClearSetting(), we removed patterns_.


BUG=551747
==========

to

==========
Use GURLs instead of patterns in SetContentSetting in notification

DesktopNotificationProfileUtil::ClearSetting() is changed to take GURLs directly
which internally call HostContentSettingsMap::SetContentSettingDefaultScope().

patterns_ is also removed from MessageCenterSettingsController. It is a vector 
of pairs from urls->patterns, which is introduced in the CL:
https://chromiumcodereview.appspot.com/12879016
to keep the original patterns from pattern->url->pattern conversion 
inconsistency. Since now urls are directly used in ClearSetting(), we removed
patterns_.


BUG=551747
==========

[lshang, 2016-03-08 00:41:11]

lshang@chromium.org changed reviewers:
+ raymes@chromium.org

[lshang, 2016-03-08 00:41:12]

PTAL thanks!

[raymes, 2016-03-09 06:53:49]

https://codereview.chromium.org/1694923004/diff/120001/chrome/browser/notific...
File chrome/browser/notifications/message_center_settings_controller.cc (left):

https://codereview.chromium.org/1694923004/diff/120001/chrome/browser/notific...
chrome/browser/notifications/message_center_settings_controller.cc:355:
DesktopNotificationProfileUtil::ClearSetting(profile, pattern);
Hey Liu, I don't fully understand this code but it looks like we might have to
keep it in place. I'm not sure if all the settings here are guaranteed to be
scoped in the way we expect :(. We can chat more tomorrow.

[lshang, 2016-03-16 00:58:51]

Description was changed from

==========
Use GURLs instead of patterns in SetContentSetting in notification

DesktopNotificationProfileUtil::ClearSetting() is changed to take GURLs directly
which internally call HostContentSettingsMap::SetContentSettingDefaultScope().

patterns_ is also removed from MessageCenterSettingsController. It is a vector 
of pairs from urls->patterns, which is introduced in the CL:
https://chromiumcodereview.appspot.com/12879016
to keep the original patterns from pattern->url->pattern conversion 
inconsistency. Since now urls are directly used in ClearSetting(), we removed
patterns_.


BUG=551747
==========

to

==========
Use GURLs instead of patterns in SetContentSetting in notification

DesktopNotificationProfileUtil::ClearSetting() is changed to take GURLs directly
which internally call HostContentSettingsMap::SetContentSettingDefaultScope().

For case in MessageCenterSettingsController, since the patterns are from user 
input, SetContentSetting() is used instead of ClearSetting() to take patterns 
directly.


BUG=551747
==========

[lshang, 2016-03-16 02:27:30]

Hi Raymes, could you take a look at this CL again? 

I changed ClearSetting() for notifications to use urls. For the case we
discussed about user input patterns in MessageCenterSettings, I remained it to
use patterns through SetContentSetting().
 
Thanks!

[raymes, 2016-03-16 03:17:45]

lgtm

https://codereview.chromium.org/1694923004/diff/140001/chrome/browser/notific...
File chrome/browser/notifications/message_center_settings_controller.cc (right):

https://codereview.chromium.org/1694923004/diff/140001/chrome/browser/notific...
chrome/browser/notifications/message_center_settings_controller.cc:353: if
(pattern.IsValid())
nit: {}

Also, I think we should add a comment here about why we don't go through 
DesktopNotificationProfileUtil otherwise someone might come and change it!

[lshang, 2016-03-16 04:56:16]

Description was changed from

==========
Use GURLs instead of patterns in SetContentSetting in notification

DesktopNotificationProfileUtil::ClearSetting() is changed to take GURLs directly
which internally call HostContentSettingsMap::SetContentSettingDefaultScope().

For case in MessageCenterSettingsController, since the patterns are from user 
input, SetContentSetting() is used instead of ClearSetting() to take patterns 
directly.


BUG=551747
==========

to

==========
Use GURLs instead of patterns in SetContentSetting in notification

DesktopNotificationProfileUtil::ClearSetting() is changed to take GURLs directly
which internally call HostContentSettingsMap::SetContentSettingDefaultScope().

For case in MessageCenterSettingsController, since the patterns are from user 
input, SetContentSetting() is used instead of ClearSetting() to take patterns 
directly.

BUG=551747
==========

[lshang, 2016-03-16 04:59:14]

lshang@chromium.org changed reviewers:
+ bauerb@chromium.org, peter@chromium.org

[lshang, 2016-03-16 04:59:14]

+OWNER

peter@: please review changes in chrome/browser/notifications/*

bauerb@: please review changes in chrome/browser/android/

Thanks!

https://codereview.chromium.org/1694923004/diff/140001/chrome/browser/notific...
File chrome/browser/notifications/message_center_settings_controller.cc (right):

https://codereview.chromium.org/1694923004/diff/140001/chrome/browser/notific...
chrome/browser/notifications/message_center_settings_controller.cc:353: if
(pattern.IsValid())
On 2016/03/16 03:17:45, raymes wrote:
> nit: {}
> 
> Also, I think we should add a comment here about why we don't go through 
> DesktopNotificationProfileUtil otherwise someone might come and change it! 

Done.

[Bernhard Bauer, 2016-03-16 10:25:22]

lgtm

https://codereview.chromium.org/1694923004/diff/160001/chrome/browser/notific...
File chrome/browser/notifications/message_center_settings_controller.cc (right):

https://codereview.chromium.org/1694923004/diff/160001/chrome/browser/notific...
chrome/browser/notifications/message_center_settings_controller.cc:355: // here
because pattern may from user manual input.
Nit: this subordinate clause no verb 😃. "[...] because the pattern might not
match the default one used by ClearSetting()"?

[Peter Beverloo, 2016-03-16 18:02:36]

lgtm % Bernhard's phrasing comment :)

[lshang, 2016-03-16 23:56:58]

Thanks, Bernhard and Peter!:-)

https://codereview.chromium.org/1694923004/diff/160001/chrome/browser/notific...
File chrome/browser/notifications/message_center_settings_controller.cc (right):

https://codereview.chromium.org/1694923004/diff/160001/chrome/browser/notific...
chrome/browser/notifications/message_center_settings_controller.cc:355: // here
because pattern may from user manual input.
On 2016/03/16 10:25:22, Bernhard Bauer wrote:
> Nit: this subordinate clause no verb 😃. "[...] because the pattern might not
> match the default one used by ClearSetting()"?

Done.

[lshang, 2016-03-16 23:57:22]

The CQ bit was checked by lshang@chromium.org

[lshang, 2016-03-16 23:57:22]

The patchset sent to the CQ was uploaded after l-g-t-m from raymes@chromium.org,
peter@chromium.org, bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/1694923004/#ps180001
(title: "minor change")

[commit-bot: I haz the power, 2016-03-16 23:58:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1694923004/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1694923004/180001

[commit-bot: I haz the power, 2016-03-17 00:41:05]

Description was changed from

==========
Use GURLs instead of patterns in SetContentSetting in notification

DesktopNotificationProfileUtil::ClearSetting() is changed to take GURLs directly
which internally call HostContentSettingsMap::SetContentSettingDefaultScope().

For case in MessageCenterSettingsController, since the patterns are from user 
input, SetContentSetting() is used instead of ClearSetting() to take patterns 
directly.

BUG=551747
==========

to

==========
Use GURLs instead of patterns in SetContentSetting in notification

DesktopNotificationProfileUtil::ClearSetting() is changed to take GURLs directly
which internally call HostContentSettingsMap::SetContentSettingDefaultScope().

For case in MessageCenterSettingsController, since the patterns are from user 
input, SetContentSetting() is used instead of ClearSetting() to take patterns 
directly.

BUG=551747
==========

[commit-bot: I haz the power, 2016-03-17 00:41:06]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-03-17 00:42:20]

Description was changed from

==========
Use GURLs instead of patterns in SetContentSetting in notification

DesktopNotificationProfileUtil::ClearSetting() is changed to take GURLs directly
which internally call HostContentSettingsMap::SetContentSettingDefaultScope().

For case in MessageCenterSettingsController, since the patterns are from user 
input, SetContentSetting() is used instead of ClearSetting() to take patterns 
directly.

BUG=551747
==========

to

==========
Use GURLs instead of patterns in SetContentSetting in notification

DesktopNotificationProfileUtil::ClearSetting() is changed to take GURLs directly
which internally call HostContentSettingsMap::SetContentSettingDefaultScope().

For case in MessageCenterSettingsController, since the patterns are from user
input, SetContentSetting() is used instead of ClearSetting() to take patterns
directly.

BUG=551747

Committed: https://crrev.com/71deb5f7cacc66ee3c901ac406db75c5f1eb83a4
Cr-Commit-Position: refs/heads/master@{#381608}
==========

[commit-bot: I haz the power, 2016-03-17 00:42:21]

Patchset 10 (id:??) landed as
https://crrev.com/71deb5f7cacc66ee3c901ac406db75c5f1eb83a4
Cr-Commit-Position: refs/heads/master@{#381608}