MD Settings: make blowing away per-origin data (i.e. cookies) easier

MD Settings: make blowing away per-origin data (i.e. cookies) easier

R=dschuyler@chromium.org
BUG=680164
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2686063004
Cr-Commit-Position: refs/heads/master@{#450548}
Committed: https://chromium.googlesource.com/chromium/src/+/7bbeadb70f71ff8809edb8b7b4c3c5b34ea49db1

[Dan Beam, 2017-02-09 20:50:19]

Description was changed from

==========
MD Settings: make blowing away per-origin date (i.e. cookies) easier

R=dpapad@chromium.org
BUG=680164
==========

to

==========
MD Settings: make blowing away per-origin date (i.e. cookies) easier

R=dpapad@chromium.org
BUG=680164
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dpapad, 2017-02-10 02:14:51]

LGTM

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

https://codereview.chromium.org/2686063004/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_data.js:107: * @param
{Event} e
Nit: !Event

[Dan Beam, 2017-02-10 03:54:13]

Description was changed from

==========
MD Settings: make blowing away per-origin date (i.e. cookies) easier

R=dpapad@chromium.org
BUG=680164
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: make blowing away per-origin date (i.e. cookies) easier

R=dschuyler@chromium.org
BUG=680164
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2017-02-10 03:54:14]

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

[Dan Beam, 2017-02-10 03:54:35]

didn't publish yet, but OK ;)

+dschuyler@ because this is site_settings land

[Dan Beam, 2017-02-10 03:55:35]

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

[commit-bot: I haz the power, 2017-02-10 03:56:08]

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, 2017-02-10 04:27:08]

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

[commit-bot: I haz the power, 2017-02-10 04:27:09]

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...)

[Dan Beam, 2017-02-11 01:09:27]

https://codereview.chromium.org/2686063004/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/site_settings/site_data.js (left):

https://codereview.chromium.org/2686063004/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_data.js:29: idToDelete_:
String,
unused (afaict)

https://codereview.chromium.org/2686063004/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_data.js:95:
this.onDeleteSite_();
never hit

https://codereview.chromium.org/2686063004/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_data.js:105: onDeleteSite_:
function() {
never hit

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

https://codereview.chromium.org/2686063004/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_data.js:107: * @param
{Event} e
On 2017/02/10 02:14:51, dpapad wrote:
> Nit: !Event

Done.

[Dan Beam, 2017-02-11 01:13:47]

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

[commit-bot: I haz the power, 2017-02-11 01:14:21]

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, 2017-02-11 08:07:57]

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

[commit-bot: I haz the power, 2017-02-11 08:07:57]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_TIMED_OUT,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Dan Beam, 2017-02-11 22:55:33]

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

[commit-bot: I haz the power, 2017-02-11 22:55:41]

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, 2017-02-11 23:56:17]

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

[commit-bot: I haz the power, 2017-02-11 23:56:18]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2017-02-13 06:06:58]

Description was changed from

==========
MD Settings: make blowing away per-origin date (i.e. cookies) easier

R=dschuyler@chromium.org
BUG=680164
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: make blowing away per-origin data (i.e. cookies) easier

R=dschuyler@chromium.org
BUG=680164
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dschuyler, 2017-02-13 20:14:35]

https://codereview.chromium.org/2686063004/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/site_data.html (right):

https://codereview.chromium.org/2686063004/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/site_data.html:41:
on-tap="onRemoveAllTap_"
Can we call this removeAllShown, removeShownSites, or removeMultiple. The name
remove all seems misleading (it may remove all or a subset depending on the
filter).

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

https://codereview.chromium.org/2686063004/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/site_data.js:96: * Called when
deletion for a single/multiple sites has been confirmed.
It looks like the comment needs to be updated.

maybe
Confirm delete of site data from some or all sites.

https://codereview.chromium.org/2686063004/diff/40001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/test_site_settings_prefs_browser_proxy.js
(right):

https://codereview.chromium.org/2686063004/diff/40001/chrome/test/data/webui/...
chrome/test/data/webui/settings/test_site_settings_prefs_browser_proxy.js:234:
return Promise.resolve({id: null, children: []});
Would it make sense to define a private var around line 65 and return that var
here? (So that a setter around line 81 could set up the cookie list).

[Dan Beam, 2017-02-14 04:14:04]

https://codereview.chromium.org/2686063004/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/site_data.html (right):

https://codereview.chromium.org/2686063004/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/site_data.html:41:
on-tap="onRemoveAllTap_"
On 2017/02/13 20:14:35, dschuyler wrote:
> Can we call this removeAllShown, removeShownSites, or removeMultiple. The name
> remove all seems misleading (it may remove all or a subset depending on the
> filter).

Done.

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

https://codereview.chromium.org/2686063004/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/site_data.js:96: * Called when
deletion for a single/multiple sites has been confirmed.
On 2017/02/13 20:14:35, dschuyler wrote:
> It looks like the comment needs to be updated.
> 
> maybe
> Confirm delete of site data from some or all sites.

Done.

https://codereview.chromium.org/2686063004/diff/40001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/test_site_settings_prefs_browser_proxy.js
(right):

https://codereview.chromium.org/2686063004/diff/40001/chrome/test/data/webui/...
chrome/test/data/webui/settings/test_site_settings_prefs_browser_proxy.js:234:
return Promise.resolve({id: null, children: []});
On 2017/02/13 20:14:35, dschuyler wrote:
> Would it make sense to define a private var around line 65 and return that var
> here? (So that a setter around line 81 could set up the cookie list).

not until the setter would be used, no

[Dan Beam, 2017-02-14 04:14:24]

Patchset #4 (id:60001) has been deleted

[Dan Beam, 2017-02-14 04:16:35]

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

[commit-bot: I haz the power, 2017-02-14 04:17:20]

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, 2017-02-14 05:32:30]

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

[commit-bot: I haz the power, 2017-02-14 05:32:31]

Dry run: This issue passed the CQ dry run.

[dschuyler, 2017-02-15 00:09:19]

lgtm

[Dan Beam, 2017-02-15 00:21:15]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2017-02-15 00:21:15]

The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org
Link to the patchset: https://codereview.chromium.org/2686063004/#ps80001
(title: "dschuyler@ review")

[commit-bot: I haz the power, 2017-02-15 00:22:31]

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-02-15 01:38:28]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1487118075253880,
"parent_rev": "a95e0192e7bd1f896eed55bad41cdc4ba9f46cb2", "commit_rev":
"7bbeadb70f71ff8809edb8b7b4c3c5b34ea49db1"}

[commit-bot: I haz the power, 2017-02-15 01:39:27]

Description was changed from

==========
MD Settings: make blowing away per-origin data (i.e. cookies) easier

R=dschuyler@chromium.org
BUG=680164
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: make blowing away per-origin data (i.e. cookies) easier

R=dschuyler@chromium.org
BUG=680164
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2686063004
Cr-Commit-Position: refs/heads/master@{#450548}
Committed:
https://chromium.googlesource.com/chromium/src/+/7bbeadb70f71ff8809edb8b7b4c3...
==========

[commit-bot: I haz the power, 2017-02-15 01:39:28]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/7bbeadb70f71ff8809edb8b7b4c3...