Support the removal of only session cookies or persistent cookies

extensions/browser/guest_view/web_view/web_view_guest.cc

Index: extensions/browser/guest_view/web_view/web_view_guest.cc
diff --git a/extensions/browser/guest_view/web_view/web_view_guest.cc b/extensions/browser/guest_view/web_view/web_view_guest.cc
index 57150cd98eb993c553f36107e0fdd8aecd4541bb..a05a5064233a4422750f21f470b4f1f53c1fd115 100644
--- a/extensions/browser/guest_view/web_view/web_view_guest.cc
+++ b/extensions/browser/guest_view/web_view/web_view_guest.cc
@@ -62,6 +62,7 @@
 #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"
@@ -87,7 +88,11 @@ 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;
@@ -425,15 +430,36 @@ void WebViewGuest::ClearDataInternal(base::Time remove_since,
     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() {