Add suborigin logic to url::Origin

content/browser/dom_storage/dom_storage_context_impl.cc

Index: content/browser/dom_storage/dom_storage_context_impl.cc
diff --git a/content/browser/dom_storage/dom_storage_context_impl.cc b/content/browser/dom_storage/dom_storage_context_impl.cc
index cd2cdbd49181b9fce9b888f912c708c41fc2096e..450e2c458e0ee4271102b23f00f6c62488750db9 100644
--- a/content/browser/dom_storage/dom_storage_context_impl.cc
+++ b/content/browser/dom_storage/dom_storage_context_impl.cc
@@ -30,8 +30,9 @@
 #include "content/public/browser/dom_storage_context.h"
 #include "content/public/browser/local_storage_usage_info.h"
 #include "content/public/browser/session_storage_usage_info.h"
-#include "content/public/common/origin_util.h"
 #include "storage/browser/quota/special_storage_policy.h"
+#include "url/gurl.h"
+#include "url/origin.h"
 
 namespace content {
 namespace {
@@ -201,27 +202,30 @@ void DOMStorageContextImpl::GetSessionStorageUsage(
   }
 }
 
-void DOMStorageContextImpl::DeleteLocalStorage(const GURL& origin) {
+void DOMStorageContextImpl::DeleteLocalStorage(const GURL& origin_url) {
   DCHECK(!is_shutdown_);
+  url::Origin origin(origin_url);
   DOMStorageNamespace* local = GetStorageNamespace(kLocalStorageNamespaceId);
   std::vector<GURL> origins;
   local->GetOriginsWithAreas(&origins);
   // Suborigin storage should be deleted in tandem with the physical origin's
   // storage. https://w3c.github.io/webappsec-suborigins/
-  for (auto origin_candidate : origins) {
+  for (auto origin_candidate_url : origins) {
+    url::Origin origin_candidate(origin_candidate_url);
     // |origin| is guaranteed to be deleted below, so don't delete it until
-    // then.
-    if (origin_candidate == origin ||
-        StripSuboriginFromUrl(origin_candidate) != origin) {
+    // then. That is, only suborigins at the same physical origin as |origin|
+    // should be deleted at this point.
+    if (origin.IsSameOriginWith(origin) ||

[michaeln, 2016/10/17 20:06:59]

What is the desired behavior if an "origin_url" explicitly expresses a
suborigin?

a. only the data for the specific suborigin of physorigin is deleted?
b. all data for the physorigin is deleted (all suborigins)?

Looks like this method does (b), is that right? And is that the desired outcome?

I see this comment...
// Suborigin storage should be deleted in tandem with the physical origin's
// storage. https://w3c.github.io/webappsec-suborigins/

My question is a little different, should all physorigin storage be deleted
given only a suborigin as input?

Is there a way to JUST delete a suborigin and leave the others alone? Does there
need to be?

If the inputs to the method can only contain PhysOrigins, can we have a DCHECK
to indicate that?

[jww, 2016/10/18 06:44:19]

Great point, Michael! In short, yes, you can yet an origin_url which is a
serialized suborigin. Frankly, I hadn't thought about this direction (origin_url
contains a suborigin) explicitly before. Now that I'm considering it, it should
delete the entire physical origin since, from the user's perspective, they're
the same. So I'll update the comments to reflect this, and add a test as well.

On 2016/10/17 20:06:59, michaeln wrote:

+        !origin_candidate.IsSamePhysicalOriginWith(origin)) {
       continue;
     }
-    DeleteAndClearStorageNamespaceForOrigin(origin_candidate, local);
+    DeleteAndClearStorageNamespaceForOrigin(origin_candidate_url, local);
   }
   // It is important to always explicitly delete |origin|. If it does not appear
   // it |origins| above, there still may be a directory open in the namespace in
   // preparation for this storage, and this call will make sure that the
   // directory is deleted.
-  DeleteAndClearStorageNamespaceForOrigin(origin, local);
+  DeleteAndClearStorageNamespaceForOrigin(origin_url, local);
 }
 
 void DOMStorageContextImpl::DeleteSessionStorage(
@@ -624,14 +628,14 @@ void DOMStorageContextImpl::DeleteNextUnusedNamespaceInCommitSequence() {
 }
 
 void DOMStorageContextImpl::DeleteAndClearStorageNamespaceForOrigin(
-    const GURL& origin,
+    const GURL& origin_url,
     DOMStorageNamespace* local) {
-  local->DeleteLocalStorageOrigin(origin);
+  local->DeleteLocalStorageOrigin(origin_url);
   // Synthesize a 'cleared' event if the area is open so CachedAreas in
   // renderers get emptied out too.
-  DOMStorageArea* area = local->GetOpenStorageArea(origin);
+  DOMStorageArea* area = local->GetOpenStorageArea(origin_url);
   if (area)
-    NotifyAreaCleared(area, origin);
+    NotifyAreaCleared(area, origin_url);
 }
 
 }  // namespace content