Support the removal of only session cookies or persistent cookies

extensions/browser/guest_view/web_view/web_view_guest.cc

 // Copyright 2014 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 "extensions/browser/guest_view/web_view/web_view_guest.h"
 
 #include <stddef.h>
 
 #include <utility>
 
@@ skipping 44 matching lines @@
 #include "extensions/browser/guest_view/web_view/web_view_permission_helper.h"
 #include "extensions/browser/guest_view/web_view/web_view_permission_types.h"
 #include "extensions/browser/guest_view/web_view/web_view_renderer_state.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension_messages.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/strings/grit/extensions_strings.h"
 #include "ipc/ipc_message_macros.h"
 #include "net/base/escape.h"
 #include "net/base/net_errors.h"
+#include "net/cookies/canonical_cookie.h"
 #include "ui/base/models/simple_menu_model.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "url/url_constants.h"
 
 using base::UserMetricsAction;
 using content::GlobalRequestID;
 using content::RenderFrameHost;
 using content::RenderProcessHost;
 using content::StoragePartition;
 using content::WebContents;
 using guest_view::GuestViewBase;
 using guest_view::GuestViewEvent;
 using guest_view::GuestViewManager;
 using zoom::ZoomController;
 
 namespace extensions {
 
 namespace {
 
 // Returns storage partition removal mask from web_view clearData mask. Note
 // that storage partition mask is a subset of webview's data removal mask.
 uint32_t GetStoragePartitionRemovalMask(uint32_t web_view_removal_mask) {
   uint32_t mask = 0;
   if (web_view_removal_mask & webview::WEB_VIEW_REMOVE_DATA_MASK_APPCACHE)
     mask |= StoragePartition::REMOVE_DATA_MASK_APPCACHE;
-  if (web_view_removal_mask & webview::WEB_VIEW_REMOVE_DATA_MASK_COOKIES)
+  if (web_view_removal_mask & webview::WEB_VIEW_REMOVE_DATA_MASK_COOKIES ||
+      web_view_removal_mask &
+          webview::WEB_VIEW_REMOVE_DATA_MASK_SESSION_COOKIES ||
+      web_view_removal_mask &
+          webview::WEB_VIEW_REMOVE_DATA_MASK_PERSISTENT_COOKIES)
     mask |= StoragePartition::REMOVE_DATA_MASK_COOKIES;

[lazyboy, 2017/02/15 21:45:27]

nit: {}

[lfg, 2017/02/15 22:44:00]

Done.

   if (web_view_removal_mask & webview::WEB_VIEW_REMOVE_DATA_MASK_FILE_SYSTEMS)
     mask |= StoragePartition::REMOVE_DATA_MASK_FILE_SYSTEMS;
   if (web_view_removal_mask & webview::WEB_VIEW_REMOVE_DATA_MASK_INDEXEDDB)
     mask |= StoragePartition::REMOVE_DATA_MASK_INDEXEDDB;
   if (web_view_removal_mask & webview::WEB_VIEW_REMOVE_DATA_MASK_LOCAL_STORAGE)
     mask |= StoragePartition::REMOVE_DATA_MASK_LOCAL_STORAGE;
   if (web_view_removal_mask & webview::WEB_VIEW_REMOVE_DATA_MASK_WEBSQL)
     mask |= StoragePartition::REMOVE_DATA_MASK_WEBSQL;
 
   return mask;
@@ skipping 316 matching lines @@
 
 void WebViewGuest::ClearDataInternal(base::Time remove_since,
                                      uint32_t removal_mask,
                                      const base::Closure& callback) {
   uint32_t storage_partition_removal_mask =
       GetStoragePartitionRemovalMask(removal_mask);
   if (!storage_partition_removal_mask) {
     callback.Run();
     return;
   }
+
+  content::StoragePartition::CookieMatcherFunction cookie_matcher;
+
+  if ((removal_mask & webview::WEB_VIEW_REMOVE_DATA_MASK_COOKIES) ||

[lazyboy, 2017/02/15 21:45:27]

It's a bit more code, but for readability, consider:
bool remove_session_cookies = !!(mask&SESSION_COOKIES);
bool remove_persistent_cookies !!(mask&..PERSISTENT_COOKIES);
bool remove_all_cookies = !!(mask&MASK_COOKIES) || (remove_session_cookies &&
remove_persistent_cookies);


if (remove_all_cookies) ...
else if (remove_persistent) ..
else if (remove_session) ...
else NOTREACHED

[lfg, 2017/02/15 22:44:00]

Done, but the NOTREACHED part isn't correct, since the API may be used to clear
data that doesn't involve cookies.

[lazyboy, 2017/02/16 02:15:34]

Acknowledged.

+      ((removal_mask & webview::WEB_VIEW_REMOVE_DATA_MASK_SESSION_COOKIES) &&
+       (removal_mask &
+        webview::WEB_VIEW_REMOVE_DATA_MASK_PERSISTENT_COOKIES))) {
+    cookie_matcher = base::Bind(
+        [](const net::CanonicalCookie& cookie) -> bool { return true; });

[lazyboy, 2017/02/15 21:45:27]

I also wonder if we should continue to use the non CookieMatcherFunction variant
of ClearData below when we know we want to remove all. As removing all should be
the most common case and we shouldn't require calling callback for each cookie.
WDYT?

[lfg, 2017/02/15 22:44:00]

Good point. I switched so that the CookieMatcherFunction is left unset when
removing all cookies. ClearData will do the right thing and skip it, see
https://cs.chromium.org/chromium/src/content/browser/storage_partition_impl.c...
.

[lazyboy, 2017/02/16 02:15:34]

Great, I'd also comment this before line 434 saying empty cookie_matcher will
cause all cookies to be purged.

[lfg, 2017/02/16 17:44:25]

Done.

+  } else if (removal_mask &
+             webview::WEB_VIEW_REMOVE_DATA_MASK_SESSION_COOKIES) {
+    cookie_matcher = base::Bind([](const net::CanonicalCookie& cookie) -> bool {
+      return !cookie.IsPersistent();
+    });
+  } else if (removal_mask &
+             webview::WEB_VIEW_REMOVE_DATA_MASK_PERSISTENT_COOKIES) {
+    cookie_matcher = base::Bind([](const net::CanonicalCookie& cookie) -> bool {
+      return cookie.IsPersistent();
+    });
+  }
+
   content::StoragePartition* partition =
       content::BrowserContext::GetStoragePartition(
           web_contents()->GetBrowserContext(),
           web_contents()->GetSiteInstance());
   partition->ClearData(
       storage_partition_removal_mask,
-      content::StoragePartition::QUOTA_MANAGED_STORAGE_MASK_ALL, GURL(),
-      content::StoragePartition::OriginMatcherFunction(), remove_since,
-      base::Time::Now(), callback);
+      content::StoragePartition::QUOTA_MANAGED_STORAGE_MASK_ALL,
+      content::StoragePartition::OriginMatcherFunction(), cookie_matcher,
+      remove_since, base::Time::Now(), callback);
 }
 
 void WebViewGuest::GuestViewDidStopLoading() {
   std::unique_ptr<base::DictionaryValue> args(new base::DictionaryValue());
   DispatchEventToView(base::MakeUnique<GuestViewEvent>(webview::kEventLoadStop,
                                                        std::move(args)));
 }
 
 void WebViewGuest::EmbedderFullscreenToggled(bool entered_fullscreen) {
   is_embedder_fullscreen_ = entered_fullscreen;
@@ skipping 1081 matching lines @@
     std::unique_ptr<base::DictionaryValue> args(new base::DictionaryValue());
     DispatchEventToView(base::MakeUnique<GuestViewEvent>(
         webview::kEventExitFullscreen, std::move(args)));
   }
   // Since we changed fullscreen state, sending a Resize message ensures that
   // renderer/ sees the change.
   web_contents()->GetRenderViewHost()->GetWidget()->WasResized();
 }
 
 }  // namespace extensions