Display third party cookies and site data counts in the WebsiteSettings UI.

chrome/browser/browsing_data_local_storage_helper.h

Index: chrome/browser/browsing_data_local_storage_helper.h
diff --git a/chrome/browser/browsing_data_local_storage_helper.h b/chrome/browser/browsing_data_local_storage_helper.h
index 6d32b0df79d5343f67b192d371c0d67daa9dd600..02a414c087842190bc3b65c5feb3d85f9c1f4e2f 100644
--- a/chrome/browser/browsing_data_local_storage_helper.h
+++ b/chrome/browser/browsing_data_local_storage_helper.h
@@ -35,7 +35,6 @@ class BrowsingDataLocalStorageHelper
  public:
   // Contains detailed information about local storage.
   struct LocalStorageInfo {
-    LocalStorageInfo();
     LocalStorageInfo(
         const std::string& protocol,
         const std::string& host,
@@ -86,7 +85,7 @@ class BrowsingDataLocalStorageHelper
   // This only mutates on the UI thread.
   bool is_fetching_;
 
-  // This only mutates in the WEBKIT thread.
+  // This only mutates in the FILE thread.

[Bernhard Bauer, 2012/05/10 17:18:59]

That's not really true. 

These BrowsingDataHelper classes use this weird mixture of protecting access via
locks, via apartment threading, and via guard variables (which are themselves
protected via apartment threading). I think before we land this CL, we're going
to need to take a good look at thread safety for all of them.

[markusheintz_, 2012/05/11 12:18:28]

Yeah I forgot to mention the UI thread.

I checked it carefully but this code is *ughh* in a complicated state currently.

This comment was wrong. |BrowsingDataLocalStorageHelper| postes a task to the
FILE thread (see chrome/browser/browsing_data_local_storage_helper.h:79).
FetchLocalStorageInfo fetchs the LocalStorageInfo objects and adds them to the
|local_storage_info_| on the FILE thread.

The |CannedBrowsingDataLocalStorageHelper| posts a new task on the UI thread to
convert the PendingLocalStorageInfo objects to LocalStorageInfo objects and adds
them to the |local_storage_info_|.

So local_storage_info_ may mutate on the UI and FILE thread, but never during
the same hm.. "object life cycle".

[Bernhard Bauer, 2012/05/11 13:05:00]

Yeah, that's what I meant. Invariants of the form "only mutates on the X thread"
don't really help in terms of thread safety, because we still need to guard
against read/write race conditions. Invariants of the form "only mutates on the
X or Y thread" help even less, as this on its own actually means that it could
be concurrently modified :-D

The "object life cycle" thing is what I meant by guard variables: in this case,
we only access |local_storage_info_| while |is_fetching_| is true, and
|is_fetching_| is only *accessed* on the UI thread. So, I think the code is
correct here (as long as it's used correctly, i.e. as long as no one calls
StartFetching while we are already fetching), but we should update the comments
to make it easier to verify the thread safety.

[markusheintz_, 2012/05/14 11:00:54]

I tried to make this comment a bit more useful.

   std::list<LocalStorageInfo> local_storage_info_;
 
  private:
@@ -124,6 +123,9 @@ class CannedBrowsingDataLocalStorageHelper
   // Returns the number of local storages currently stored.
   size_t GetLocalStorageCount() const;
 
+  // Returns the set of origins that use local storage.
+  const std::set<GURL>& GetLocalStorageInfo() const;
+
   // BrowsingDataLocalStorageHelper implementation.
   virtual void StartFetching(
       const base::Callback<void(const std::list<LocalStorageInfo>&)>& callback)