[MD settings] move cookie tree management to cookie tree behavior

[MD settings] move cookie tree management to cookie tree behavior

This CL moves code that will soon be shared between the site_data page
and the site_data_details_subpage page. This is separated away from
adding the subpage to ease the review process and support git bisect,
if needed in the future. The code to load and manage the cookie tree
has been moved into cookie_tree_behavior. The code to fill out the cookie
info has been moved to cookie_info. Support for closure compilation in
content settings has been improved (some types were incorrect). This
CL is related groundwork for a future CL and not a full fix for the bug.

BUG=654943
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/bc04c3ad2ead66463a7a296f77d3c9bd9f8794da
Cr-Commit-Position: refs/heads/master@{#428465}

[dschuyler, 2016-10-25 23:55:41]

Description was changed from

==========
[MD settings] move cookie tree management to cookie tree behavior

This CL moves code that will soon be shared between the site_data page
and the site_data_details_subpage page. This is separated away from
adding the subpage to ease the review process and support git bisect,
if needed in the future. The code to load and manage the cookie tree
has been moved into cookie_tree_behavior. The code to fill out the cookie
info has been moved to cookie_info. Support for closure compilation in
content settings has been improved (some types were incorrect). This
CL is related groundwork for a future CL and not a full fix for the bug.

BUG=654943
==========

to

==========
[MD settings] move cookie tree management to cookie tree behavior

This CL moves code that will soon be shared between the site_data page
and the site_data_details_subpage page. This is separated away from
adding the subpage to ease the review process and support git bisect,
if needed in the future. The code to load and manage the cookie tree
has been moved into cookie_tree_behavior. The code to fill out the cookie
info has been moved to cookie_info. Support for closure compilation in
content settings has been improved (some types were incorrect). This
CL is related groundwork for a future CL and not a full fix for the bug.

BUG=654943
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dschuyler, 2016-10-25 23:55:44]

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

[commit-bot: I haz the power, 2016-10-25 23:56:14]

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

[dschuyler, 2016-10-26 00:08:05]

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

[dschuyler, 2016-10-26 00:08:25]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2016-10-26 00:08:28]

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

[dschuyler, 2016-10-26 00:18:37]

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

[dschuyler, 2016-10-26 00:18:38]

https://codereview.chromium.org/2451553008/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/cookie_info.js (right):

https://codereview.chromium.org/2451553008/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_info.js:18: var
CookieDataForDisplay;
These were moved (unchanged).

https://codereview.chromium.org/2451553008/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_info.js:73: var
getCookieData = function(data) {
This was moved and slightly changed.
|data| was a cookie rather than CookieDetails
but only the CookieDetails were needed - so
the type of the param changed.

https://codereview.chromium.org/2451553008/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/cookie_tree_behavior.js
(right):

https://codereview.chromium.org/2451553008/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_tree_behavior.js:16:
sites: Array,
Moved from another file.

https://codereview.chromium.org/2451553008/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_tree_behavior.js:24:
rootCookieNode_: Object,
|rootCookieNode_| had the name |treeNodes_| which I felt
was confusing. I find it easier to read as the root
cookie node of the cookie tree. (That sentence is kinda funny).

https://codereview.chromium.org/2451553008/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_tree_behavior.js:26: 
The contents of this file is ~98% moved from other
files.

https://codereview.chromium.org/2451553008/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/cookie_tree_node.js (left):

https://codereview.chromium.org/2451553008/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_tree_node.js:18: var
CookieDataForDisplay;
Moved (not removed).

https://codereview.chromium.org/2451553008/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_tree_node.js:255: },
Moved to cookie_info.js

https://codereview.chromium.org/2451553008/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/site_data.js (left):

https://codereview.chromium.org/2451553008/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/site_data.js:9: 
Items in this file generally moved to cookie_tree_behavior.js.

https://codereview.chromium.org/2451553008/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js
(left):

https://codereview.chromium.org/2451553008/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js:160:
* @return {!Promise<Array<CookieDataSummaryItem>>} Returns the full cookie
These were incorrect (may have been accurate at one time). The C++ side
returns an object that has an array/list rather than returning the array
directly.

[commit-bot: I haz the power, 2016-10-26 02:48:21]

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

[commit-bot: I haz the power, 2016-10-26 02:48:22]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[dschuyler, 2016-10-26 22:32:05]

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

[commit-bot: I haz the power, 2016-10-26 22:33:10]

Dry run: 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, 2016-10-27 00:14:01]

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

[commit-bot: I haz the power, 2016-10-27 00:14:02]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2016-10-27 01:31:55]

https://codereview.chromium.org/2451553008/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/cookie_tree_behavior.js
(right):

https://codereview.chromium.org/2451553008/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_tree_behavior.js:14: *
@type {Array<CookieDataSummaryItem>}
nit: !Array<!CookieDataSummaryItem>

https://codereview.chromium.org/2451553008/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_tree_behavior.js:22: *
@protected
if this is protected, don't use _

https://codereview.chromium.org/2451553008/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/site_data_details_dialog.js
(right):

https://codereview.chromium.org/2451553008/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/site_data_details_dialog.js:130:
this.cookieNodes_ = getCookieData(node.data_);
why is this accessing a private member?

https://codereview.chromium.org/2451553008/diff/40001/chrome/browser/resource...
File
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js
(right):

https://codereview.chromium.org/2451553008/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js:160:
* @return {!Promise<CookieList>} Returns the full cookie
arguably these should be !CookieList (unless they can be null)

[dschuyler, 2016-10-27 20:44:36]

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

[commit-bot: I haz the power, 2016-10-27 20:45:33]

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

[dschuyler, 2016-10-27 20:47:36]

https://codereview.chromium.org/2451553008/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/cookie_tree_behavior.js
(right):

https://codereview.chromium.org/2451553008/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_tree_behavior.js:14: *
@type {Array<CookieDataSummaryItem>}
On 2016/10/27 01:31:55, Dan Beam wrote:
> nit: !Array<!CookieDataSummaryItem>

Done.

https://codereview.chromium.org/2451553008/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_tree_behavior.js:22: *
@protected
On 2016/10/27 01:31:55, Dan Beam wrote:
> if this is protected, don't use _

Done.

https://codereview.chromium.org/2451553008/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/site_data_details_dialog.js
(right):

https://codereview.chromium.org/2451553008/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/site_data_details_dialog.js:130:
this.cookieNodes_ = getCookieData(node.data_);
On 2016/10/27 01:31:55, Dan Beam wrote:
> why is this accessing a private member?

I'd like to refactor the way the node data is fetched
and organized, but for this CL I added an accessor.

Done.

https://codereview.chromium.org/2451553008/diff/40001/chrome/browser/resource...
File
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js
(right):

https://codereview.chromium.org/2451553008/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/site_settings_prefs_browser_proxy.js:160:
* @return {!Promise<CookieList>} Returns the full cookie
On 2016/10/27 01:31:55, Dan Beam wrote:
> arguably these should be !CookieList (unless they can be null)

Done.

[commit-bot: I haz the power, 2016-10-27 21:04:20]

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

[commit-bot: I haz the power, 2016-10-27 21:04:21]

Dry run: Try jobs failed on following builders:
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[Dan Beam, 2016-10-27 21:05:11]

https://codereview.chromium.org/2451553008/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/cookie_tree_behavior.js
(right):

https://codereview.chromium.org/2451553008/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_tree_behavior.js:71: *
@return {Promise}
nit: !Promise<...type...>

https://codereview.chromium.org/2451553008/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_tree_behavior.js:74:
return this.browserProxy.reloadCookies().then(
wait, where does .browserProxy come from?

https://codereview.chromium.org/2451553008/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/cookie_tree_node.js
(right):

https://codereview.chromium.org/2451553008/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_tree_node.js:96:
getCookieDetails: function() {
this works, but why not just ... make data_ public as data?

[dschuyler, 2016-10-27 22:52:35]

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

[commit-bot: I haz the power, 2016-10-27 22:53:18]

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

[dschuyler, 2016-10-27 22:54:25]

https://codereview.chromium.org/2451553008/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/cookie_tree_behavior.js
(right):

https://codereview.chromium.org/2451553008/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_tree_behavior.js:71: *
@return {Promise}
On 2016/10/27 21:05:10, Dan Beam wrote:
> nit: !Promise<...type...>

There isn't a type, the Promise is used for timing rather
than passing data.

https://codereview.chromium.org/2451553008/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_tree_behavior.js:74:
return this.browserProxy.reloadCookies().then(
On 2016/10/27 21:05:10, Dan Beam wrote:
> wait, where does .browserProxy come from?

It comes from SiteSettingsBehavior. I'd like to refactor
SiteSettingsBehavior separately from this CL.

https://codereview.chromium.org/2451553008/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/cookie_tree_node.js
(right):

https://codereview.chromium.org/2451553008/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/cookie_tree_node.js:96:
getCookieDetails: function() {
On 2016/10/27 21:05:10, Dan Beam wrote:
> this works, but why not just ... make data_ public as data?

Done.

[Dan Beam, 2016-10-27 22:56:14]

lgtm

[commit-bot: I haz the power, 2016-10-27 22:58:45]

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

[commit-bot: I haz the power, 2016-10-27 22:58:46]

Dry run: Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dschuyler, 2016-10-28 17:39:50]

The CQ bit was checked by dschuyler@chromium.org

[commit-bot: I haz the power, 2016-10-28 17:40:21]

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, 2016-10-28 19:47:49]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-10-28 19:56:09]

Description was changed from

==========
[MD settings] move cookie tree management to cookie tree behavior

This CL moves code that will soon be shared between the site_data page
and the site_data_details_subpage page. This is separated away from
adding the subpage to ease the review process and support git bisect,
if needed in the future. The code to load and manage the cookie tree
has been moved into cookie_tree_behavior. The code to fill out the cookie
info has been moved to cookie_info. Support for closure compilation in
content settings has been improved (some types were incorrect). This
CL is related groundwork for a future CL and not a full fix for the bug.

BUG=654943
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[MD settings] move cookie tree management to cookie tree behavior

This CL moves code that will soon be shared between the site_data page
and the site_data_details_subpage page. This is separated away from
adding the subpage to ease the review process and support git bisect,
if needed in the future. The code to load and manage the cookie tree
has been moved into cookie_tree_behavior. The code to fill out the cookie
info has been moved to cookie_info. Support for closure compilation in
content settings has been improved (some types were incorrect). This
CL is related groundwork for a future CL and not a full fix for the bug.

BUG=654943
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/bc04c3ad2ead66463a7a296f77d3c9bd9f8794da
Cr-Commit-Position: refs/heads/master@{#428465}
==========

[commit-bot: I haz the power, 2016-10-28 19:56:10]

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