[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

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 2a06e4d955d28ee3f4ca856f0dd2f14683390f10..8517b07a38e5cad9196c3e120ec937f1ed487594 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
@@ -34,6 +34,7 @@
 #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"
@@ -41,6 +42,7 @@
 #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"
@@ -671,6 +673,67 @@ IN_PROC_BROWSER_TEST_F(SearchByImageBrowserTest, ImageSearchWithCorruptImage) {
   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),