Reload favicon from HTTP cache on Ctrl+Refresh

chrome/browser/favicon/favicon_tab_helper_browsertest.cc

Index: chrome/browser/favicon/favicon_tab_helper_browsertest.cc
diff --git a/chrome/browser/favicon/favicon_tab_helper_browsertest.cc b/chrome/browser/favicon/favicon_tab_helper_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..73bb7836eca1841a95d763dc539fa3b5062b0373
--- /dev/null
+++ b/chrome/browser/favicon/favicon_tab_helper_browsertest.cc
@@ -0,0 +1,203 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.

[sky, 2015/02/19 18:08:10]

2015

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/favicon/favicon_tab_helper.h"
+
+#include "base/run_loop.h"
+#include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/favicon/favicon_handler.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/notification_types.h"
+#include "content/public/browser/resource_dispatcher_host.h"
+#include "content/public/browser/resource_dispatcher_host_delegate.h"
+#include "net/base/load_flags.h"
+#include "net/url_request/url_request.h"
+#include "net/test/spawned_test_server/spawned_test_server.h"
+#include "url/url_constants.h"
+
+namespace {
+
+// Tracks whether the URL passed to the constructor is requested and whether
+// the request bypasses the cache.

[pkotwicz, 2015/02/19 16:47:02]

This file violates chrome/browser/favicon/DEPS
I suspect an exception is perfectly fine?

I did not find a way of writing this test without making a browser test. In
particular, I need to navigate to a real webpage.

[sky, 2015/02/19 18:08:11]

What files doesn't deps like?

[pkotwicz, 2015/02/19 21:29:57]

DEPS does not like chrome/browser/ui

[sky, 2015/02/20 17:57:50]

Updating DEPS for the test is fine.

+class TestResourceDispatcherHostDelegate
+    : public content::ResourceDispatcherHostDelegate {
+ public:
+  explicit TestResourceDispatcherHostDelegate(GURL url)

[sky, 2015/02/19 18:08:10]

const GURL&

+      : url_(url), was_requested_(false), bypassed_cache_(false) {}
+  ~TestResourceDispatcherHostDelegate() override {}
+
+  void Reset() {
+    was_requested_ = false;
+    bypassed_cache_ = false;
+  }
+
+  // Resturns whether |url_| was requested.
+  bool was_requested() const { return was_requested_; }
+
+  // Returns whether any of the requests bypassed the HTTP cache.
+  bool bypassed_cache() const { return bypassed_cache_; }
+
+ private:
+  // content::ResourceDispatcherHostDelegate:
+  bool ShouldBeginRequest(const std::string& method,
+                          const GURL& url,
+                          content::ResourceType resource_type,
+                          content::ResourceContext* resource_context) override {
+    return true;
+  }
+
+  void RequestBeginning(
+      net::URLRequest* request,
+      content::ResourceContext* resource_context,
+      content::AppCacheService* appcache_service,
+      content::ResourceType resource_type,
+      ScopedVector<content::ResourceThrottle>* throttles) override {
+    if (request->url() == url_) {
+      was_requested_ = true;
+      if (request->load_flags() & net::LOAD_BYPASS_CACHE)
+        bypassed_cache_ = true;
+    }
+  }
+
+  void DownloadStarting(net::URLRequest* request,
+                        content::ResourceContext* resource_context,
+                        int child_id,
+                        int route_id,
+                        int request_id,
+                        bool is_content_initiated,
+                        bool must_download,
+                        ScopedVector<content::ResourceThrottle>* throttles) override {
+  }
+
+ private:
+  GURL url_;
+  bool was_requested_;
+  bool bypassed_cache_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestResourceDispatcherHostDelegate);
+};
+
+// Waits for the following the finish:
+// - The pending navigation.
+// - FaviconHandler's pending favicon database requests.
+// - FaviconHandler's pending downloads.
+class PendingTaskWaiter : public content::NotificationObserver {
+ public:
+  PendingTaskWaiter(content::WebContents* web_contents,
+                    FaviconHandler* favicon_handler)
+      : favicon_handler_(favicon_handler), load_stopped_(false) {
+    registrar_.Add(this, chrome::NOTIFICATION_FAVICON_UPDATED,
+                   content::Source<content::WebContents>(web_contents));
+    registrar_.Add(this, content::NOTIFICATION_LOAD_STOP,
+                   content::Source<content::NavigationController>(
+                       &web_contents->GetController()));
+  }
+  ~PendingTaskWaiter() override {}
+
+  void Wait() {
+    if (CanStopWaiting())
+      return;
+
+    base::RunLoop run_loop;
+    quit_closure_ = run_loop.QuitClosure();
+    run_loop.Run();
+  }
+
+ private:
+  // content::NotificationObserver:
+  void Observe(int type,
+               const content::NotificationSource& source,
+               const content::NotificationDetails& details) override {
+    if (type == content::NOTIFICATION_LOAD_STOP)
+      load_stopped_ = true;
+
+    if (!quit_closure_.is_null() && CanStopWaiting())
+      quit_closure_.Run();
+  }
+
+  bool CanStopWaiting() {
+    return load_stopped_ &&
+           !favicon_handler_->HasPendingFaviconServiceRequestsForTest() &&
+           !favicon_handler_->HasPendingDownloadsForTest();
+  }
+
+  FaviconHandler* favicon_handler_;
+  bool load_stopped_;
+  base::Closure quit_closure_;
+  content::NotificationRegistrar registrar_;
+
+  DISALLOW_COPY_AND_ASSIGN(PendingTaskWaiter);
+};
+
+}  // namespace
+
+class FaviconTabHelperTest : public InProcessBrowserTest {
+ public:
+  FaviconTabHelperTest() {}
+  ~FaviconTabHelperTest() override {}
+
+  content::WebContents* web_contents() {
+    return browser()->tab_strip_model()->GetActiveWebContents();
+  }
+
+  FaviconHandler* favicon_handler() {
+    return FaviconTabHelper::FromWebContents(web_contents())->favicon_handler_.get();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FaviconTabHelperTest);
+};
+
+// Test that when a user reloads a page ignoring the cache that the favicon is
+// is redownloaded and (not returned from either the favicon cache or the HTTP
+// cache).
+IN_PROC_BROWSER_TEST_F(FaviconTabHelperTest, ReloadIgnoringCache) {
+  ASSERT_TRUE(test_server()->Start());
+  GURL url = test_server()->GetURL("files/favicon/page_with_favicon.html");
+  GURL icon_url = test_server()->GetURL("files/favicon/icon.ico");
+
+  scoped_ptr<TestResourceDispatcherHostDelegate> delegate(
+      new TestResourceDispatcherHostDelegate(icon_url));
+  content::ResourceDispatcherHost::Get()->SetDelegate(delegate.get());
+
+  // Initial visit in order to populate the cache.
+  {
+    PendingTaskWaiter waiter(web_contents(), favicon_handler());
+    ui_test_utils::NavigateToURLWithDisposition(
+        browser(), url, CURRENT_TAB, ui_test_utils::BROWSER_TEST_NONE);
+    waiter.Wait();
+  }
+  ASSERT_TRUE(delegate->was_requested());
+  EXPECT_FALSE(delegate->bypassed_cache());
+  delegate->Reset();
+
+  ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
+
+  // A normal visit should fetch the favicon from either the favicon database or
+  // the HTTP cache.
+  {
+    PendingTaskWaiter waiter(web_contents(), favicon_handler());
+    ui_test_utils::NavigateToURLWithDisposition(
+        browser(), url, CURRENT_TAB, ui_test_utils::BROWSER_TEST_NONE);
+    waiter.Wait();
+  }
+  EXPECT_FALSE(delegate->bypassed_cache());
+  delegate->Reset();
+
+  // A reload ingoring the cache should refetch the favicon from the website.
+  {
+    PendingTaskWaiter waiter(web_contents(), favicon_handler());
+    chrome::ExecuteCommand(browser(), IDC_RELOAD_IGNORING_CACHE);
+    waiter.Wait();
+  }
+  ASSERT_TRUE(delegate->was_requested());
+  EXPECT_TRUE(delegate->bypassed_cache());
+}