[MD setting] separate RawSiteException and SiteException

[MD setting] separate RawSiteException and SiteException

This CL creates a new RawSiteSeception data type to separate the type
that is passed from C++ vs the version used in the UI.

(related to bug 695578)
BUG=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2760183002
Cr-Commit-Position: refs/heads/master@{#458808}
Committed: https://chromium.googlesource.com/chromium/src/+/d864bc1dd103a8905734988823dee69e52e04940

[dschuyler, 2017-03-20 23:47:59]

Description was changed from

==========
[MD setting] separate RawSiteException and SiteException

This CL creates a new RawSiteSeception data type to separate the type
that is passed from C++ vs the version used in the UI.

(related to bug 695578)
BUG=None
==========

to

==========
[MD setting] separate RawSiteException and SiteException

This CL creates a new RawSiteSeception data type to separate the type
that is passed from C++ vs the version used in the UI.

(related to bug 695578)
BUG=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dschuyler, 2017-03-20 23:48:01]

The CQ bit was checked by dschuyler@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-20 23:48:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dschuyler, 2017-03-21 00:12:17]

dschuyler@chromium.org changed reviewers:
+ dpapad@chromium.org

[dschuyler, 2017-03-21 00:13:33]

On 2017/03/21 00:12:18, dschuyler wrote:

This is in response to a comment on CL
https://codereview.chromium.org/2749023008/

[dpapad, 2017-03-21 01:04:22]

Thanks for attempting to make the types in site_settings more accurate. Few more
questions.

https://codereview.chromium.org/2760183002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/site_settings/site_settings_behavior.js
(right):

https://codereview.chromium.org/2760183002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_settings_behavior.js:14: *
See site_settings/constants.js for possible values.
Could this be changed to better reflect the possible values?

@type {!settings.ContentSettingsTypes}

https://codereview.chromium.org/2760183002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js
(right):

https://codereview.chromium.org/2760183002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js:37:
* @typedef {{category: string,
Follow up question from previous comment.

1) Should this be
category: settings.ContentSettingsTypes
2) Should this type be moved to site_settings/constants.js? It is not used
anywhere in the proxy anymore (see related following comment too).

https://codereview.chromium.org/2760183002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js:61:
* @typedef {{location: Array<SiteException>,
What about these two?

[commit-bot: I haz the power, 2017-03-21 02:59:43]

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

[commit-bot: I haz the power, 2017-03-21 02:59:47]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[dschuyler, 2017-03-22 01:34:39]

The CQ bit was checked by dschuyler@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-22 01:35:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dschuyler, 2017-03-22 01:35:41]

https://codereview.chromium.org/2760183002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/site_settings/site_settings_behavior.js
(right):

https://codereview.chromium.org/2760183002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_settings_behavior.js:14: *
See site_settings/constants.js for possible values.
On 2017/03/21 01:04:22, dpapad wrote:
> Could this be changed to better reflect the possible values?
> 
> @type {!settings.ContentSettingsTypes}

Done.

https://codereview.chromium.org/2760183002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js
(right):

https://codereview.chromium.org/2760183002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js:37:
* @typedef {{category: string,
On 2017/03/21 01:04:22, dpapad wrote:
> Follow up question from previous comment.
> 
> 1) Should this be
> category: settings.ContentSettingsTypes

It can be. Done.

> 2) Should this type be moved to site_settings/constants.js? It is not used
> anywhere in the proxy anymore (see related following comment too).

That's a reasonable idea. I'd like to keep this Cl relatively small since it's
just a cleanup/enhancement.

https://codereview.chromium.org/2760183002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js:61:
* @typedef {{location: Array<SiteException>,
On 2017/03/21 01:04:22, dpapad wrote:
> What about these two?

Are ya asking if these should be RawSiteExceptions? I believe these are
correctly SiteExceptions. And, they could probably be moved elsewhere as well.
It's used in SiteSettingsPref, which in turn is used (maybe only) in unit tests.

[commit-bot: I haz the power, 2017-03-22 03:37:26]

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

[commit-bot: I haz the power, 2017-03-22 03:37:27]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't
started yet, builder probably lacks capacity)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT,
build hasn't started yet, builder probably lacks capacity)

[dpapad, 2017-03-22 17:28:53]

LGTM

https://codereview.chromium.org/2760183002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js
(right):

https://codereview.chromium.org/2760183002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js:61:
* @typedef {{location: Array<SiteException>,
On 2017/03/22 at 01:35:41, dschuyler wrote:
> On 2017/03/21 01:04:22, dpapad wrote:
> > What about these two?
> 
> Are ya asking if these should be RawSiteExceptions? I believe these are
correctly SiteExceptions. And, they could probably be moved elsewhere as well.
It's used in SiteSettingsPref, which in turn is used (maybe only) in unit tests.

Ok. Yes, that is what I was asking.

[dschuyler, 2017-03-22 18:14:39]

The CQ bit was checked by dschuyler@chromium.org

[commit-bot: I haz the power, 2017-03-22 18:15:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 18:23:33]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490206479050280,
"parent_rev": "55844bc1348ddf628a1559a2ed4532fe6f614478", "commit_rev":
"d864bc1dd103a8905734988823dee69e52e04940"}

[commit-bot: I haz the power, 2017-03-22 18:24:20]

Description was changed from

==========
[MD setting] separate RawSiteException and SiteException

This CL creates a new RawSiteSeception data type to separate the type
that is passed from C++ vs the version used in the UI.

(related to bug 695578)
BUG=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[MD setting] separate RawSiteException and SiteException

This CL creates a new RawSiteSeception data type to separate the type
that is passed from C++ vs the version used in the UI.

(related to bug 695578)
BUG=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2760183002
Cr-Commit-Position: refs/heads/master@{#458808}
Committed:
https://chromium.googlesource.com/chromium/src/+/d864bc1dd103a8905734988823de...
==========

[commit-bot: I haz the power, 2017-03-22 18:24:21]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/d864bc1dd103a8905734988823de...