Add suborigin logic to url::Origin

content/browser/dom_storage/dom_storage_context_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/dom_storage/dom_storage_context_impl.h"
 
 #include <inttypes.h>
 #include <stddef.h>
 #include <stdlib.h>
 
@@ skipping 12 matching lines @@
 #include "base/trace_event/process_memory_dump.h"
 #include "content/browser/dom_storage/dom_storage_area.h"
 #include "content/browser/dom_storage/dom_storage_database.h"
 #include "content/browser/dom_storage/dom_storage_namespace.h"
 #include "content/browser/dom_storage/dom_storage_task_runner.h"
 #include "content/browser/dom_storage/session_storage_database.h"
 #include "content/common/dom_storage/dom_storage_types.h"
 #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 {
 
 // Limits on the cache size and number of areas in memory, over which the areas
 // are purged.
 #if defined(OS_ANDROID)
 const unsigned kMaxStorageAreaCount = 20;
 const size_t kMaxCacheSize = 2 * 1024 * 1024;
 #else
@@ skipping 149 matching lines @@
     for (std::vector<GURL>::const_iterator origin_it = it->second.begin();
          origin_it != it->second.end(); ++origin_it) {
       SessionStorageUsageInfo info;
       info.persistent_namespace_id = it->first;
       info.origin = *origin_it;
       infos->push_back(info);
     }
   }
 }
 
-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(
     const SessionStorageUsageInfo& usage_info) {
   DCHECK(!is_shutdown_);
   DOMStorageNamespace* dom_storage_namespace = NULL;
   std::map<std::string, int64_t>::const_iterator it =
       persistent_namespace_id_to_namespace_id_.find(
           usage_info.persistent_namespace_id);
   if (it != persistent_namespace_id_to_namespace_id_.end()) {
@@ skipping 382 matching lines @@
   if (!deletable_persistent_namespace_ids_.empty()) {
     task_runner_->PostDelayedTask(
         FROM_HERE, base::Bind(
             &DOMStorageContextImpl::DeleteNextUnusedNamespace,
             this),
         base::TimeDelta::FromSeconds(kSessionStoraceScavengingSeconds));
   }
 }
 
 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