Show local storage usage on the Website Settings options page.

chrome/browser/ui/webui/options/website_settings_handler.cc

Index: chrome/browser/ui/webui/options/website_settings_handler.cc
diff --git a/chrome/browser/ui/webui/options/website_settings_handler.cc b/chrome/browser/ui/webui/options/website_settings_handler.cc
index a1b267de2230679d0f74ec09428c0d7b6aec063a..41aa1f6347d3c4e63c9747293d370cb1037cce9a 100644
--- a/chrome/browser/ui/webui/options/website_settings_handler.cc
+++ b/chrome/browser/ui/webui/options/website_settings_handler.cc
@@ -9,6 +9,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/web_ui.h"
 #include "grit/generated_resources.h"
+#include "ui/base/text/bytes_formatting.h"
 
 namespace {
 
@@ -35,6 +36,7 @@ void WebsiteSettingsHandler::GetLocalizedValues(
       {"websitesLabelGeolocation", IDS_WEBSITE_SETTINGS_TYPE_LOCATION},
       {"websitesLabelMediaStream", IDS_WEBSITE_SETTINGS_TYPE_MEDIASTREAM},
       {"websitesLabelNotifications", IDS_WEBSITE_SETTINGS_TYPE_NOTIFICATIONS},
+      {"websitesLabelStorage", IDS_WEBSITE_SETTINGS_TYPE_STORAGE},
   };
 
   RegisterStrings(localized_strings, resources, arraysize(resources));
@@ -52,6 +54,11 @@ void WebsiteSettingsHandler::RegisterMessages() {
       "updateOriginsSearchResults",
       base::Bind(&WebsiteSettingsHandler::HandleUpdateSearchResults,
                  base::Unretained(this)));
+
+  web_ui()->RegisterMessageCallback(
+      "updateLocalStorage",
+      base::Bind(&WebsiteSettingsHandler::HandleUpdateLocalStorage,
+                 base::Unretained(this)));
 }
 
 void WebsiteSettingsHandler::HandleUpdateOrigins(const base::ListValue* args) {
@@ -61,7 +68,7 @@ void WebsiteSettingsHandler::HandleUpdateOrigins(const base::ListValue* args) {
   ContentSettingsType content_type;
   content_settings::GetTypeFromName(content_setting_name, &content_type);

[Bernhard Bauer, 2014/07/21 11:29:30]

Can we add some more checks in here? I would like to avoid that a nonsensical
content type sneaks into |last_setting_| and then causes an error much later on.

[Daniel Nishi, 2014/07/21 17:53:37]

I added a DCHECK that GetTypeFromName got a valid type back and a DCHECK to
ensure that it was a ContentType we can handle.

 
-  last_setting_ = content_type;
+  last_setting_ = content_setting_name;
 
   UpdateOrigins(std::string());
 }
@@ -72,7 +79,48 @@ void WebsiteSettingsHandler::HandleUpdateSearchResults(
   bool rv = args->GetString(0, &filter);
   DCHECK(rv);
 
-  UpdateOrigins(filter);
+  if (last_setting_ == "storage")

[Bernhard Bauer, 2014/07/21 11:29:30]

Can you extract "storage" to a constant?

[Daniel Nishi, 2014/07/21 17:53:36]

Done.

+    UpdateLocalStorage(filter);

[Bernhard Bauer, 2014/07/21 11:29:29]

So, this means that every time the filter string changes (and it changes for
every typed character), we will fetch all local storage items again? That seems
like a recipe for jank. We will probably need to cache the results somewhere
(see also my comment below about a container for browsing data).

[Daniel Nishi, 2014/07/21 17:53:37]

I've changed the implementation to cache the local storage data when you select
the category and to use the cached list after that.

+  else
+    UpdateOrigins(filter);
+}
+
+void WebsiteSettingsHandler::HandleUpdateLocalStorage(
+    const base::ListValue* args) {
+  if (!local_storage_.get()) {

[Bernhard Bauer, 2014/07/21 11:29:30]

.get() is not necessary; scoped_refptr (and scoped_ptr) has an implicit coercion
to bool that will Do The Right Thing.

[Daniel Nishi, 2014/07/21 17:53:37]

Done.

+    Profile* profile = Profile::FromWebUI(web_ui());
+    local_storage_ = (new BrowsingDataLocalStorageHelper(profile));

[Bernhard Bauer, 2014/07/21 11:29:30]

Parentheses are not necessary.

[Daniel Nishi, 2014/07/21 17:53:37]

Done.

+  }
+
+  last_setting_ = "storage";
+
+  UpdateLocalStorage(std::string());

[Bernhard Bauer, 2014/07/21 11:29:29]

What if I had previously typed something into the filter box?

[Daniel Nishi, 2014/07/21 17:53:37]

Last query now stored and reused.

+}
+
+void WebsiteSettingsHandler::OnLocalStorageFetch(
+    const std::string& filter,
+    const std::list<BrowsingDataLocalStorageHelper::LocalStorageInfo>&
+        storage) {
+  base::DictionaryValue local_storage_map;
+  for (std::list<
+           BrowsingDataLocalStorageHelper::LocalStorageInfo>::const_iterator
+           it = storage.begin();
+       it != storage.end();
+       it++) {
+    std::string origin = (*it).origin_url.spec();

[Bernhard Bauer, 2014/07/21 11:29:29]

This can be `it->...`.

[Daniel Nishi, 2014/07/21 17:53:37]

Done and replaced in other places where I used (*it) instead.

+
+    if (origin.find(filter) == base::string16::npos)
+      continue;
+
+    base::DictionaryValue* origin_entry = new base::DictionaryValue();
+    origin_entry->SetWithoutPathExpansion(
+        "usage", new base::FundamentalValue(static_cast<double>((*it).size)));
+    origin_entry->SetWithoutPathExpansion(
+        "usage_string", new base::StringValue(ui::FormatBytes((*it).size)));

[Bernhard Bauer, 2014/07/21 11:29:30]

Nit: Javascript properties use lowercasedCamelCase, not unix_hacker_style. 

[Daniel Nishi, 2014/07/21 17:53:37]

Done.

+    local_storage_map.SetWithoutPathExpansion(origin, origin_entry);
+  }
+  web_ui()->CallJavascriptFunction("WebsiteSettingsManager.updateLocalStorage",
+                                   local_storage_map);
 }
 
 void WebsiteSettingsHandler::UpdateOrigins(const std::string& filter) {
@@ -80,7 +128,9 @@ void WebsiteSettingsHandler::UpdateOrigins(const std::string& filter) {
   HostContentSettingsMap* settings = profile->GetHostContentSettingsMap();
 
   ContentSettingsForOneType all_settings;
-  settings->GetSettingsForOneType(last_setting_, std::string(), &all_settings);
+  ContentSettingsType last_setting;
+  content_settings::GetTypeFromName(last_setting_, &last_setting);
+  settings->GetSettingsForOneType(last_setting, std::string(), &all_settings);
 
   base::DictionaryValue origins;
   for (ContentSettingsForOneType::const_iterator it = all_settings.begin();
@@ -106,4 +156,11 @@ void WebsiteSettingsHandler::UpdateOrigins(const std::string& filter) {
                                    origins);
 }
 
+void WebsiteSettingsHandler::UpdateLocalStorage(const std::string& filter) {
+  local_storage_->StartFetching(
+      base::Bind(&WebsiteSettingsHandler::OnLocalStorageFetch,
+                 base::Unretained(this),

[Bernhard Bauer, 2014/07/21 11:29:29]

Uh-oh... |local_storage_| might call this callback at any point, including when
the WebUI has already been torn down. It's a bit unfortunate that all the
browsing data helpers are refcounted with no way to cancel them, but as long as
that's the case, you'll probably need to use a weak pointer here.

When you're adding more browsing data helpers here, it might make sense to add
some container that will hold their scoped_refptrs and be the callback target;
then you can use weak pointers to that instead of the whole handler, and have
the handler simply own it. Basically, something like LocalDataContainer, except
without a CookiesTreeModel[*]. 

[*] You could probably even use CookiesTreeModel, but it's a bit awkward, as it
dates from the pre-WebUI version of the cookies dialog, which actually was,
y'know, a tree view, and its recursive structure doesn't really fit in well with
an origin list.

[Daniel Nishi, 2014/07/21 17:53:37]

I have it using a weak_ptr now. If another browsing data helper gets used, I'll
pull out the LocalDataContainer's dependency on CookiesTreeModel and work from
there, I think.

+                 filter));
+}
+
 }  // namespace options