Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(146)

Issue 2476753002: [MD settings] get cookie details from site without transfering cookie tree (Closed)

Created:
4 years, 1 month ago by dschuyler
Modified:
4 years, 1 month ago
Reviewers:
Dan Beam
CC:
chromium-reviews, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, arv+watch_chromium.org, vitalyp+closure_chromium.org, dbeam+watch-settings_chromium.org, dbeam+watch-closure_chromium.org, stevenjb+watch-md-settings_chromium.org, jlklein+watch-closure_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[MD settings] get cookie details from site without transfering cookie tree This CL accelerates the site cookie details page. To fully address the bug, the main list of cookies (non-details) needs improvement as well. This CL is a separable step in toward that goal. BUG=659865 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/68cd6a7fb8998e99a7832617363f3a7f43200fd0 Cr-Commit-Position: refs/heads/master@{#430021}

Patch Set 1 : touchups #

Total comments: 10

Patch Set 2 : comment changes #

Patch Set 3 : CHECK for params #

Total comments: 4

Patch Set 4 : nit and merge with master #

Patch Set 5 : closure fix #

Messages

Total messages: 38 (28 generated)
dschuyler
4 years, 1 month ago (2016-11-03 21:51:31 UTC) #11
Dan Beam
https://codereview.chromium.org/2476753002/diff/40001/chrome/browser/ui/webui/cookies_tree_model_util.h File chrome/browser/ui/webui/cookies_tree_model_util.h (right): https://codereview.chromium.org/2476753002/diff/40001/chrome/browser/ui/webui/cookies_tree_model_util.h#newcode54 chrome/browser/ui/webui/cookies_tree_model_util.h:54: // is not valid. ^ https://codereview.chromium.org/2476753002/diff/40001/chrome/browser/ui/webui/settings/settings_cookies_view_handler.cc File chrome/browser/ui/webui/settings/settings_cookies_view_handler.cc (right): ...
4 years, 1 month ago (2016-11-04 00:35:32 UTC) #16
dschuyler
I changed the if() tests to CHECK for the params. https://codereview.chromium.org/2476753002/diff/40001/chrome/browser/ui/webui/cookies_tree_model_util.h File chrome/browser/ui/webui/cookies_tree_model_util.h (right): https://codereview.chromium.org/2476753002/diff/40001/chrome/browser/ui/webui/cookies_tree_model_util.h#newcode54 ...
4 years, 1 month ago (2016-11-04 00:50:45 UTC) #19
Dan Beam
i'm fairly unfamiliar with cookies code, so take my review there with a grain of ...
4 years, 1 month ago (2016-11-04 02:03:35 UTC) #20
dschuyler
https://codereview.chromium.org/2476753002/diff/80001/chrome/browser/resources/settings/site_settings/site_data_details_subpage.js File chrome/browser/resources/settings/site_settings/site_data_details_subpage.js (right): https://codereview.chromium.org/2476753002/diff/80001/chrome/browser/resources/settings/site_settings/site_data_details_subpage.js#newcode61 chrome/browser/resources/settings/site_settings/site_data_details_subpage.js:61: if (site == undefined || site == this.site_) On ...
4 years, 1 month ago (2016-11-04 18:17:42 UTC) #25
dschuyler
https://codereview.chromium.org/2476753002/diff/80001/chrome/browser/resources/settings/site_settings/site_data_details_subpage.js File chrome/browser/resources/settings/site_settings/site_data_details_subpage.js (right): https://codereview.chromium.org/2476753002/diff/80001/chrome/browser/resources/settings/site_settings/site_data_details_subpage.js#newcode61 chrome/browser/resources/settings/site_settings/site_data_details_subpage.js:61: if (site == undefined || site == this.site_) On ...
4 years, 1 month ago (2016-11-04 18:48:21 UTC) #30
Dan Beam
https://codereview.chromium.org/2476753002/diff/80001/chrome/browser/resources/settings/site_settings/site_data_details_subpage.js File chrome/browser/resources/settings/site_settings/site_data_details_subpage.js (right): https://codereview.chromium.org/2476753002/diff/80001/chrome/browser/resources/settings/site_settings/site_data_details_subpage.js#newcode61 chrome/browser/resources/settings/site_settings/site_data_details_subpage.js:61: if (site == undefined || site == this.site_) On ...
4 years, 1 month ago (2016-11-04 18:48:54 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2476753002/120001
4 years, 1 month ago (2016-11-04 21:31:03 UTC) #35
commit-bot: I haz the power
Committed patchset #5 (id:120001)
4 years, 1 month ago (2016-11-04 21:37:54 UTC) #36
commit-bot: I haz the power
4 years, 1 month ago (2016-11-04 21:40:15 UTC) #38
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/68cd6a7fb8998e99a7832617363f3a7f43200fd0
Cr-Commit-Position: refs/heads/master@{#430021}

Powered by Google App Engine
This is Rietveld 408576698