Make CookieStore::SetCookieAccessDelegate non-virtual

Previously this method was virtual, and the default implementation just
did nothing. However this would cause the unique_ptr argument to just
be destroyed, and that was causing segfaults in tests which tried to
hold onto the pointer to the delegate (and use it to set expected
values, etc.), if the CookieStore they were using had the default
implementation.

This change moves the implementation of SetCookieAccessDelegate from
CookieMonster to its base class CookieStore, and stores the delegate
in the base class itself, to avoid this problem.

Bug: 978172
Change-Id: Iee2c2ee7a5edae9e98266a7c4d38774f5babb3ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1849114Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704242}

net/cookies/cookie_monster.cc

@@ -518,11 +518,6 @@ CookieChangeDispatcher& CookieMonster::GetChangeDispatcher() {
   return change_dispatcher_;
 }
 
-void CookieMonster::SetCookieAccessDelegate(
-    std::unique_ptr<CookieAccessDelegate> delegate) {
-  cookie_access_delegate_ = std::move(delegate);
-}
-
 void CookieMonster::DumpMemoryStats(
     base::trace_event::ProcessMemoryDump* pmd,
     const std::string& parent_absolute_name) const {
@@ -1676,8 +1671,8 @@ bool CookieMonster::HasCookieableScheme(const GURL& url) {
 
 CookieAccessSemantics CookieMonster::GetAccessSemanticsForCookie(
     const CanonicalCookie& cookie) const {
-  if (cookie_access_delegate_)
-    return cookie_access_delegate_->GetAccessSemantics(cookie);
+  if (cookie_access_delegate())
+    return cookie_access_delegate()->GetAccessSemantics(cookie);
   return CookieAccessSemantics::UNKNOWN;
 }
 

net/cookies/cookie_monster.h

@@ -192,10 +192,6 @@ class NET_EXPORT CookieMonster : public CookieStore {
   static const char* const kDefaultCookieableSchemes[];
   static const int kDefaultCookieableSchemesCount;
 
-  // Take ownership of a CookieAccessDelegate.
-  void SetCookieAccessDelegate(
-      std::unique_ptr<CookieAccessDelegate> delegate) override;
-
   void DumpMemoryStats(base::trace_event::ProcessMemoryDump* pmd,
                        const std::string& parent_absolute_name) const override;
 
@@ -628,10 +624,6 @@ class NET_EXPORT CookieMonster : public CookieStore {
 
   bool persist_session_cookies_;
 
-  // Used to determine whether a particular cookie should be subject to legacy
-  // or non-legacy access semantics.
-  std::unique_ptr<CookieAccessDelegate> cookie_access_delegate_;
-
   base::ThreadChecker thread_checker_;
 
   base::WeakPtrFactory<CookieMonster> weak_ptr_factory_{this};

net/cookies/cookie_store.cc

@@ -9,6 +9,8 @@
 
 namespace net {
 
+CookieStore::CookieStore() = default;
+
 CookieStore::~CookieStore() = default;
 
 void CookieStore::DeleteAllAsync(DeleteCallback callback) {
@@ -22,7 +24,7 @@ void CookieStore::SetForceKeepSessionState() {
 
 void CookieStore::SetCookieAccessDelegate(
     std::unique_ptr<CookieAccessDelegate> delegate) {
-  // By default, do nothing.
+  cookie_access_delegate_ = std::move(delegate);
 }
 
 void CookieStore::DumpMemoryStats(

net/cookies/cookie_store.h

@@ -54,6 +54,7 @@ class NET_EXPORT CookieStore {
   using DeleteCallback = base::OnceCallback<void(uint32_t num_deleted)>;
   using SetCookieableSchemesCallback = base::OnceCallback<void(bool success)>;
 
+  CookieStore();
   virtual ~CookieStore();
 
   // Set the cookie on the cookie store.  |cookie.IsCanonical()| must
@@ -129,12 +130,23 @@ class NET_EXPORT CookieStore {
                                     SetCookieableSchemesCallback callback) = 0;
 
   // Transfer ownership of a CookieAccessDelegate.
-  virtual void SetCookieAccessDelegate(
-      std::unique_ptr<CookieAccessDelegate> delegate);
+  void SetCookieAccessDelegate(std::unique_ptr<CookieAccessDelegate> delegate);
 
   // Reports the estimate of dynamically allocated memory in bytes.
   virtual void DumpMemoryStats(base::trace_event::ProcessMemoryDump* pmd,
                                const std::string& parent_absolute_name) const;
+
+ protected:
+  // This may be null if no delegate has been set yet, or the delegate has been
+  // reset to null.
+  const CookieAccessDelegate* cookie_access_delegate() const {
+    return cookie_access_delegate_.get();
+  }
+
+ private:
+  // Used to determine whether a particular cookie should be subject to legacy
+  // or non-legacy access semantics.
+  std::unique_ptr<CookieAccessDelegate> cookie_access_delegate_;
 };
 
 }  // namespace net