"Load image" context menu item to reload a LoFi image.

chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc

Index: chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc b/chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
index ae93e95b2fda6023f6dea2b59707d92d493beb41..8c1c7bf9b284619b18c8dddb65c7df47f251aaa8 100644
--- a/chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
+++ b/chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
@@ -4,6 +4,7 @@
 
 #include <string>
 
+#include "base/bind.h"
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
@@ -34,6 +35,10 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_utils.h"
+#include "net/base/load_flags.h"
+#include "net/url_request/url_request.h"
+#include "net/url_request/url_request_filter.h"
+#include "net/url_request/url_request_interceptor.h"
 #include "third_party/WebKit/public/web/WebContextMenuData.h"
 #include "third_party/WebKit/public/web/WebInputEvent.h"
 
@@ -45,7 +50,7 @@ class ContextMenuBrowserTest : public InProcessBrowserTest {
  public:
   ContextMenuBrowserTest() {}
 
-  TestRenderViewContextMenu* CreateContextMenu(
+  content::ContextMenuParams CreateContextMenuParams(
       const GURL& unfiltered_url,
       const GURL& url,
       blink::WebContextMenuData::MediaType media_type) {
@@ -62,6 +67,11 @@ class ContextMenuBrowserTest : public InProcessBrowserTest {
     params.writing_direction_left_to_right = 0;
     params.writing_direction_right_to_left = 0;
 #endif  // OS_MACOSX
+    return params;
+  }
+
+  TestRenderViewContextMenu* CreateContextMenu(
+      content::ContextMenuParams params) {
     TestRenderViewContextMenu* menu = new TestRenderViewContextMenu(
         browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame(),
         params);
@@ -72,8 +82,8 @@ class ContextMenuBrowserTest : public InProcessBrowserTest {
   TestRenderViewContextMenu* CreateContextMenuMediaTypeNone(
       const GURL& unfiltered_url,
       const GURL& url) {
-    return CreateContextMenu(unfiltered_url, url,
-                             blink::WebContextMenuData::MediaTypeNone);
+    return CreateContextMenu(CreateContextMenuParams(
+        unfiltered_url, url, blink::WebContextMenuData::MediaTypeNone));
   }
 };
 
@@ -308,14 +318,30 @@ IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest, ViewPageInfoWithNoEntry) {
   menu.ExecuteCommand(IDC_CONTENT_CONTEXT_VIEWPAGEINFO, 0);
 }
 
+IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest, DataSaverLoadImage) {

[Lei Zhang, 2015/05/11 20:16:53]

Can this be a unit test instead?

[megjablon, 2015/05/11 22:58:40]

I looked into writing it as a unit test, but the
IDC_CONTENT_CONTEXT_OPENIMAGENEWTAB option in AppendImageItems makes this
complicated because the params in DataReductionProxyChromeSettings do not get
initialized and settings->CanUseDataReductionProxy(params_.src_url) crashes.

[Lei Zhang, 2015/05/12 21:16:39]

It's probably possible, just a pain to set up all the services that are needed
to exercise the code.

[megjablon, 2015/05/13 23:36:38]

Just found out someone is already doing this work in the context menu unit tests
:) once that is landed i'll sync and change this to a unit test

[Lei Zhang, 2015/05/13 23:50:19]

Yay! Can you add a TODO to convert ContextMenuBrowserTest.DataSaverLoadImage to
a unit test?

[megjablon, 2015/05/14 19:26:24]

Done.

+  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+  command_line->AppendSwitch(
+      data_reduction_proxy::switches::kEnableDataReductionProxyLoFi);
+
+  content::ContextMenuParams params = CreateContextMenuParams(
+      GURL(), GURL("http://url.com/image.png"),
+      blink::WebContextMenuData::MediaTypeImage);
+  params.image_was_fetched_lo_fi = true;
+
+  scoped_ptr<TestRenderViewContextMenu> menu(CreateContextMenu(params));
+
+  ASSERT_TRUE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_RELOAD_ORIGINAL_IMAGE));
+}
+
 IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest, DataSaverOpenOrigImageInNewTab) {
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   command_line->AppendSwitch(
       data_reduction_proxy::switches::kEnableDataReductionProxy);
 
   scoped_ptr<TestRenderViewContextMenu> menu(
-      CreateContextMenu(GURL(), GURL("http://url.com/image.png"),
-                        blink::WebContextMenuData::MediaTypeImage));
+      CreateContextMenu(CreateContextMenuParams(
+          GURL(), GURL("http://url.com/image.png"),
+          blink::WebContextMenuData::MediaTypeImage)));
 
   ASSERT_FALSE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_OPENIMAGENEWTAB));
   ASSERT_TRUE(menu->IsItemPresent(
@@ -329,8 +355,9 @@ IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest,
       data_reduction_proxy::switches::kEnableDataReductionProxy);
 
   scoped_ptr<TestRenderViewContextMenu> menu(
-      CreateContextMenu(GURL(), GURL("https://url.com/image.png"),
-                        blink::WebContextMenuData::MediaTypeImage));
+      CreateContextMenu(CreateContextMenuParams(
+          GURL(), GURL("https://url.com/image.png"),
+          blink::WebContextMenuData::MediaTypeImage)));
 
   ASSERT_FALSE(
       menu->IsItemPresent(IDC_CONTENT_CONTEXT_OPEN_ORIGINAL_IMAGE_NEW_TAB));
@@ -339,8 +366,9 @@ IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest, OpenImageInNewTab) {
   scoped_ptr<TestRenderViewContextMenu> menu(
-      CreateContextMenu(GURL(), GURL("http://url.com/image.png"),
-                        blink::WebContextMenuData::MediaTypeImage));
+      CreateContextMenu(CreateContextMenuParams(
+          GURL(), GURL("http://url.com/image.png"),
+          blink::WebContextMenuData::MediaTypeImage)));
   ASSERT_FALSE(
       menu->IsItemPresent(IDC_CONTENT_CONTEXT_OPEN_ORIGINAL_IMAGE_NEW_TAB));
   ASSERT_TRUE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_OPENIMAGENEWTAB));
@@ -525,4 +553,128 @@ IN_PROC_BROWSER_TEST_F(SearchByImageBrowserTest, ImageSearchWithCorruptImage) {
   EXPECT_EQ(ThumbnailResponseWatcher::THUMBNAIL_RECEIVED, watcher.Wait());
 }
 
+class LoadImageRequestInterceptor : public net::URLRequestInterceptor {
+ public:
+  LoadImageRequestInterceptor() : num_requests_(0),
+                                  requests_to_wait_for_(-1),
+                                  weak_factory_(this) {
+  }
+
+  ~LoadImageRequestInterceptor() override {}
+
+  // net::URLRequestInterceptor implementation
+  net::URLRequestJob* MaybeInterceptRequest(
+      net::URLRequest* request,
+      net::NetworkDelegate* network_delegate) const override {
+    DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));

[Lei Zhang, 2015/05/11 20:16:53]

Use DCHECK_CURRENTLY_ON()

[megjablon, 2015/05/11 22:58:40]

Done.

+    EXPECT_TRUE(request->load_flags() & net::LOAD_BYPASS_CACHE);
+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI, FROM_HERE,
+        base::Bind(&LoadImageRequestInterceptor::RequestCreated,
+                   weak_factory_.GetWeakPtr()));
+    return nullptr;
+  }
+
+  void WaitForRequests(int requests_to_wait_for) {
+    DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+    DCHECK_EQ(-1, requests_to_wait_for_);
+    DCHECK(!run_loop_);
+
+    if (num_requests_ >= requests_to_wait_for)
+      return;
+
+    requests_to_wait_for_ = requests_to_wait_for;
+    run_loop_.reset(new base::RunLoop());
+    run_loop_->Run();
+    run_loop_.reset();
+    requests_to_wait_for_ = -1;
+    EXPECT_EQ(num_requests_, requests_to_wait_for);
+  }
+
+  // It is up to the caller to wait until all relevant requests has been
+  // created, either through calling WaitForRequests or some other manner,
+  // before calling this method.
+  int num_requests() const {
+    DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+    return num_requests_;
+  }
+
+ private:
+  void RequestCreated() {
+    DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+    num_requests_++;
+    if (num_requests_ == requests_to_wait_for_)
+      run_loop_->Quit();
+  }
+
+  // These are only used on the UI thread.
+  int num_requests_;
+  int requests_to_wait_for_;
+  scoped_ptr<base::RunLoop> run_loop_;
+
+  // This prevents any risk of flake if any test doesn't wait for a request
+  // it sent.  Mutable so it can be accessed from a const function.
+  mutable base::WeakPtrFactory<LoadImageRequestInterceptor> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(LoadImageRequestInterceptor);
+};
+
+class LoadImageBrowserTest : public InProcessBrowserTest {
+ protected:
+  void SetupAndLoadImagePage(const std::string& image_path) {
+    // Go to a page with an image in it. The test server doesn't serve the image
+    // with the right MIME type, so use a data URL to make a page containing it.
+    GURL image_url(test_server()->GetURL(image_path));
+    GURL page("data:text/html,<img src='" + image_url.spec() + "'>");
+    ui_test_utils::NavigateToURL(browser(), page);
+  }
+
+  void AddLoadImageInterceptor(const std::string& image_path) {
+    interceptor_ = new LoadImageRequestInterceptor();
+    scoped_ptr<net::URLRequestInterceptor> owned_interceptor(interceptor_);
+    // Ownership of the |interceptor_| is passed to an object the IO thread, but
+    // a pointer is kept in the test fixture. As soon as anything calls
+    // URLRequestFilter::ClearHandlers(), |interceptor_| can become invalid.

[Lei Zhang, 2015/05/11 20:16:53]

I realized this is copied over from another test, but how does
LoadImageBrowserTest know when |interceptor_| is invalid, and when to stop using
it?

[megjablon, 2015/05/11 22:58:40]

Looks like ClearHandlers is only called by tests and was on tear down from where
I followed this implementation, so I don't think this comment is necessary.

+    content::BrowserThread::PostTask(
+        content::BrowserThread::IO, FROM_HERE,
+        base::Bind(&LoadImageBrowserTest::AddInterceptorForURL,
+                   base::Unretained(this),
+                   GURL(test_server()->GetURL(image_path).spec()),
+                   base::Passed(&owned_interceptor)));
+  }
+
+  void AttemptImageLoad() {
+    // Right-click where the image should be.
+    // |menu_observer_| will cause the load-image menu item to be clicked.
+    menu_observer_.reset(new ContextMenuNotificationObserver(
+        IDC_CONTENT_CONTEXT_RELOAD_ORIGINAL_IMAGE));
+    content::WebContents* tab =
+        browser()->tab_strip_model()->GetActiveWebContents();
+    content::SimulateMouseClickAt(tab, 0, blink::WebMouseEvent::ButtonRight,
+                                  gfx::Point(15, 15));
+  }
+
+  void AddInterceptorForURL(
+      const GURL& url, scoped_ptr<net::URLRequestInterceptor> handler) {
+    DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+    net::URLRequestFilter::GetInstance()->AddUrlInterceptor(
+        url, handler.Pass());
+  }
+
+  LoadImageRequestInterceptor* interceptor_;
+
+ private:
+  scoped_ptr<ContextMenuNotificationObserver> menu_observer_;
+};
+
+IN_PROC_BROWSER_TEST_F(LoadImageBrowserTest, LoadImage) {
+  static const char kValidImage[] = "files/load_image/image.png";
+  SetupAndLoadImagePage(kValidImage);
+  AddLoadImageInterceptor(kValidImage);
+  AttemptImageLoad();
+  interceptor_->WaitForRequests(1);
+  EXPECT_EQ(1, interceptor_->num_requests());
+}
+
 }  // namespace