Reset profile: reset cookies and site data.

chrome/browser/profile_resetter/profile_resetter_browsertest.cc

+// Copyright 2013 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 "chrome/browser/profile_resetter/profile_resetter.h"
+
+#include "base/bind.h"
+#include "chrome/browser/profile_resetter/profile_resetter_test_base.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/test/test_utils.h"
+#include "net/cookies/cookie_store.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_context_getter.h"
+
+namespace {
+
+using content::BrowserThread;
+
+class RemoveCookieTester {

[battre, 2013/06/20 06:15:57]

highlevel class comment?

[vasilii, 2013/06/20 13:08:22]

Done.

+ public:
+  explicit RemoveCookieTester(Profile* profile);
+  ~RemoveCookieTester();
+
+  bool ContainsCookie();
+  void AddCookie();
+
+ private:
+  void GetCookieOnIOThread(net::URLRequestContextGetter* rq_context);
+  void SetCookieOnIOThread(net::URLRequestContextGetter* rq_context);
+  void GetCookieCallback(const std::string& cookies);
+  void SetCookieCallback(bool result);
+
+  void BlockUntilNotified();
+  void Notify();
+
+  bool get_cookie_success_;
+  bool waiting_callback_;
+  Profile* profile_;
+  scoped_refptr<content::MessageLoopRunner> runner_;
+
+  DISALLOW_COPY_AND_ASSIGN(RemoveCookieTester);
+};
+
+RemoveCookieTester::RemoveCookieTester(Profile* profile)
+    : get_cookie_success_(false),
+      waiting_callback_(false),
+      profile_(profile) {
+}
+
+RemoveCookieTester::~RemoveCookieTester() {}
+
+// Returns true, if the given cookie exists in the cookie store.
+bool RemoveCookieTester::ContainsCookie() {
+  get_cookie_success_ = false;
+  waiting_callback_ = true;
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&RemoveCookieTester::GetCookieOnIOThread,
+                 base::Unretained(this),
+                 base::Unretained(profile_->GetRequestContext())));

[battre, 2013/06/20 06:15:57]

with this suggestions below, this function would become something like

...
       base::Bind(&RemoveCookieTester::GetCookieOnIOThread,
                  base::Unretained(this),
                  base::Unretained(profile_->GetRequestContext())
                  cookie_hostname)));
  BlockUntilNotified();
  return last_cookies_;

[vasilii, 2013/06/20 13:08:22]

Done.

+  BlockUntilNotified();
+  return get_cookie_success_;
+}
+
+void RemoveCookieTester::AddCookie() {
+  waiting_callback_ = true;
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&RemoveCookieTester::SetCookieOnIOThread,
+                 base::Unretained(this),
+                 base::Unretained(profile_->GetRequestContext())));
+  BlockUntilNotified();
+}
+
+void RemoveCookieTester::GetCookieOnIOThread(
+    net::URLRequestContextGetter* rq_context) {

[battre, 2013/06/20 06:15:57]

nit: We try not to use abbreviations in variable names. I would call it
context_getter.

[vasilii, 2013/06/20 13:08:22]

Done.

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  net::CookieStore* cookie_store = rq_context->
+      GetURLRequestContext()->cookie_store();
+  cookie_store->GetCookiesWithOptionsAsync(
+        GURL("http://host1/"), net::CookieOptions(),

[battre, 2013/06/20 06:15:57]

nit: -2 indentation

[battre, 2013/06/20 06:15:57]

What do you think of passing the host name as a parameter?

[battre, 2013/06/20 06:15:57]

can you move this string literal and also the A=1 out of the functions? -->
first they become global literals
const char* kCookieHostname = "http://host1";
const char* kCookieDefinition = "A=1";
and second they become parameters to that are passed from the test into this
helper class.

[vasilii, 2013/06/20 13:08:22]

Done.

[vasilii, 2013/06/20 13:08:22]

Done.

[vasilii, 2013/06/20 13:08:22]

Done.

+        base::Bind(&RemoveCookieTester::GetCookieCallback,
+                   base::Unretained(this)));
+}
+
+void RemoveCookieTester::SetCookieOnIOThread(
+    net::URLRequestContextGetter* rq_context) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  net::CookieStore* cookie_store = rq_context->
+      GetURLRequestContext()->cookie_store();
+  cookie_store->SetCookieWithOptionsAsync(
+      GURL("http://host1/"), "A=1", net::CookieOptions(),

[battre, 2013/06/20 06:15:57]

what do you think of passing the hostname and cookie value as parameters?

[vasilii, 2013/06/20 13:08:22]

Done.

+      base::Bind(&RemoveCookieTester::SetCookieCallback,
+                 base::Unretained(this)));
+}
+
+void RemoveCookieTester::GetCookieCallback(const std::string& cookies) {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&RemoveCookieTester::GetCookieCallback,
+                   base::Unretained(this), cookies));
+    return;
+  }
+  if (cookies == "A=1") {
+    get_cookie_success_ = true;

[battre, 2013/06/20 06:15:57]

opt: What do you think of just storing cookies into an attribute |last_cookies_|
and then replacing return get_cookie_success_; in
RemoveCookieTester::ContainsCookie() with "return last_cookies_ ==
kCookieDefinition;"?

reasoning: I don't like the get_cookie_success_ variable name so much and that
the "A=1" string is in this function.

[vasilii, 2013/06/20 13:08:22]

Done.

+  } else {
+    EXPECT_EQ("", cookies);
+    get_cookie_success_ = false;
+  }
+  Notify();
+}
+
+void RemoveCookieTester::SetCookieCallback(bool result) {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&RemoveCookieTester::SetCookieCallback,
+                   base::Unretained(this), result));
+    return;
+  }
+  ASSERT_TRUE(result);
+  Notify();
+}
+
+void RemoveCookieTester::BlockUntilNotified() {
+  DCHECK(!runner_);
+  if (waiting_callback_) {
+    runner_ = new content::MessageLoopRunner;
+    runner_->Run();
+    runner_ = NULL;
+  }
+}
+
+void RemoveCookieTester::Notify() {
+  DCHECK(waiting_callback_);
+  waiting_callback_ = false;
+  if (runner_)
+    runner_->Quit();
+}
+
+class ProfileResetTest : public InProcessBrowserTest,
+                         public ProfileResetterTestBase {
+ protected:
+  virtual void SetUpOnMainThread() OVERRIDE {
+    resetter_.reset(new ProfileResetter(browser()->profile()));
+  }
+};
+
+
+IN_PROC_BROWSER_TEST_F(ProfileResetTest, ResetCookiesAndSiteData) {
+  RemoveCookieTester tester(browser()->profile());
+  tester.AddCookie();

[battre, 2013/06/20 06:15:57]

how about making this call
tester.AddCookie(kCookieHostname, kCookieDefinition);

[vasilii, 2013/06/20 13:08:22]

Done.

+  ASSERT_TRUE(tester.ContainsCookie());
+
+  resetter_->Reset(ProfileResetter::COOKIES_AND_SITE_DATA,
+                   base::Bind(&ProfileResetterMockObject::StopLoop,
+                              base::Unretained(&mock_object_)));
+  mock_object_.RunLoop();
+
+  EXPECT_FALSE(tester.ContainsCookie());

[battre, 2013/06/20 06:15:57]

and making this here:
EXPECT_EQ(kCookieDefinition, tester.GetCookies(kCookieHostname));

[vasilii, 2013/06/20 13:08:22]

Done.

+}
+
+}  // namespace