[Reland 1] Save pdf file instead of web page through context menu for embedded pdf file

chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc

 // Copyright 2014 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/renderer_context_menu/render_view_context_menu.h"
 
 #include <algorithm>
 #include <memory>
 #include <string>
 #include <utility>
@@ skipping 16 matching lines @@
 #include "chrome/browser/profiles/profile_attributes_entry.h"
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/profiles/profile_window.h"
 #include "chrome/browser/renderer_context_menu/render_view_context_menu_browsertest_util.h"
 #include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/thumbnail_capturer.mojom.h"
+#include "chrome/grit/generated_resources.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/search_test_utils.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_switches.h"
 #include "components/search_engines/template_url_data.h"
 #include "components/search_engines/template_url_service.h"
 #include "content/public/browser/browser_message_filter.h"
+#include "content/public/browser/browser_plugin_guest_manager.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
@@ skipping 610 matching lines @@
   thumbnail_capturer->RequestThumbnailForContextNode(
       0, gfx::Size(2048, 2048),
       base::Bind(callback, &response_received, run_loop.QuitClosure()));
   run_loop.Run();
 
   // The browser should receive a response from the renderer, because the
   // renderer should not crash.
   ASSERT_TRUE(response_received);
 }
 
+IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest, FullPagePdfHasPageItems) {
+  // Load a pdf page.
+  GURL page_url =
+      ui_test_utils::GetTestUrl(base::FilePath(FILE_PATH_LITERAL("pdf")),
+                                base::FilePath(FILE_PATH_LITERAL("test.pdf")));
+  ui_test_utils::NavigateToURL(browser(), page_url);
+
+  // Get the pdf plugin's WebContents and its main frame.
+  WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  content::BrowserPluginGuestManager* guest_manager =
+      web_contents->GetBrowserContext()->GetGuestManager();
+  WebContents* guest_contents = guest_manager->GetFullPageGuest(web_contents);
+  ASSERT_TRUE(guest_contents);
+  content::RenderFrameHost* frame = guest_contents->GetMainFrame();
+  ASSERT_TRUE(frame);
+  ASSERT_NE(frame, web_contents->GetMainFrame());
+
+  content::ContextMenuParams params;
+  params.page_url = page_url;
+  params.frame_url = frame->GetLastCommittedURL();
+  params.frame_page_state = content::PageState::CreateFromURL(params.frame_url);
+  params.media_type = blink::WebContextMenuData::kMediaTypePlugin;
+  TestRenderViewContextMenu menu(frame, params);
+  menu.Init();
+
+  // The full page related items such as 'back' should be there.
+  ASSERT_TRUE(menu.IsItemPresent(IDC_BACK));
+}
+
+IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest, EmbeddedPdfHasNoPageItems) {
+  // Load a page with an embedded pdf.
+  GURL page_url = ui_test_utils::GetTestUrl(
+      base::FilePath(FILE_PATH_LITERAL("pdf")),
+      base::FilePath(FILE_PATH_LITERAL("test-embedded-pdf.html")));
+  ui_test_utils::NavigateToURL(browser(), page_url);
+
+  WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  // Observe loading the pdf plugin.
+  content::WebContentsAddedObserver web_contents_added_observer;
+  ASSERT_TRUE(ExecuteScript(web_contents,
+                            "var l = document.getElementById('link1');"
+                            "l.click();"));
+  // Get the embedded pdf plugin's WebContents and its main frame.
+  WebContents* inner_contents = web_contents_added_observer.GetWebContents();

[lazyboy, 2017/05/11 18:15:17]

Do you need to
ASSERT_TRUE(content::WaitForLoadStop(innner_contents));?

Yesterday, lukasza@ was seeing test flakiness because of this
(https://codereview.chromium.org/2871063002/). 

[Wei Li, 2017/05/17 05:37:09]

Turned out that I do need some kind of wait() to make sure the loading is done.
Changed.

[lazyboy, 2017/05/17 16:34:44]

I can't find where you've added the wait in the new patch set. Can you explain?
Also, add a comment where we are waiting:
// Wait for .. to finish loading.
etc.

+  ASSERT_TRUE(inner_contents);
+  content::RenderFrameHost* frame = inner_contents->GetMainFrame();
+
+  content::ContextMenuParams params;
+  params.page_url = page_url;
+  params.frame_url = frame->GetLastCommittedURL();
+  params.frame_page_state = content::PageState::CreateFromURL(params.frame_url);
+  params.media_type = blink::WebContextMenuData::kMediaTypePlugin;
+  TestRenderViewContextMenu menu(frame, params);
+  menu.Init();
+
+  // The full page related items such as 'back' should not be displayed.
+  ASSERT_FALSE(menu.IsItemPresent(IDC_BACK));

[lazyboy, 2017/05/11 18:15:17]

Here and above, back could be unavailable for other reasons like we don't have a
navigation item to go back to... IDC_RELOAD is better I think because it will
always be there.
However, since it's looking at its presence (IsItemPresent) instead of its
enabled-ness (IsCommandIdEnabled) this isn't wrong either. So consider this
optional.

[Wei Li, 2017/05/17 05:37:09]

I agree that enabled or not doesn't matter for this test case. But using
'reload' might help understanding. Done.
 

+}
+
 class LoadImageRequestInterceptor : public net::URLRequestInterceptor {
  public:
   LoadImageRequestInterceptor() : num_requests_(0),
                                   requests_to_wait_for_(-1),
                                   weak_factory_(this) {
   }
 
   ~LoadImageRequestInterceptor() override {}
 
   // net::URLRequestInterceptor implementation
@@ skipping 107 matching lines @@
 IN_PROC_BROWSER_TEST_F(LoadImageBrowserTest, LoadImage) {
   static const char kValidImage[] = "/load_image/image.png";
   SetupAndLoadImagePage(kValidImage);
   AddLoadImageInterceptor(kValidImage);
   AttemptLoadImage();
   interceptor_->WaitForRequests(1);
   EXPECT_EQ(1, interceptor_->num_requests());
 }
 
 }  // namespace