Use suggested filename for "Save Link As"

chrome/browser/renderer_context_menu/render_view_context_menu_browsertest_util.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_browsertest_util.h"
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
+#include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/renderer_context_menu/render_view_context_menu.h"
 #include "content/public/browser/notification_service.h"
+#include "content/public/test/test_utils.h"
 
 ContextMenuNotificationObserver::ContextMenuNotificationObserver(
     int command_to_execute)
-    : command_to_execute_(command_to_execute) {
+    : command_to_execute_(command_to_execute), menu_visible_(false) {
   registrar_.Add(this,
                  chrome::NOTIFICATION_RENDER_VIEW_CONTEXT_MENU_SHOWN,
                  content::NotificationService::AllSources());
 }
 
 ContextMenuNotificationObserver::~ContextMenuNotificationObserver() {
 }
 
 void ContextMenuNotificationObserver::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   switch (type) {
     case chrome::NOTIFICATION_RENDER_VIEW_CONTEXT_MENU_SHOWN: {
-      RenderViewContextMenu* context_menu =
-          content::Source<RenderViewContextMenu>(source).ptr();
+      menu_visible_ = true;
+      context_menu_ = content::Source<RenderViewContextMenu>(source).ptr();

[lazyboy, 2014/06/24 01:15:04]

Instead of saving the raw pointer of RVContextMenu, just copy
context_menu_->params() to a member.
That way you don't have to worry whether context_menu_ is valid ptr anymore or
not (Once you show another context menu, this would become invalid iiuc).

[Nikhil, 2014/06/24 10:39:51]

Done.

       base::MessageLoop::current()->PostTask(
           FROM_HERE,
           base::Bind(&ContextMenuNotificationObserver::ExecuteCommand,
-                     base::Unretained(this), context_menu));
+                     base::Unretained(this),
+                     context_menu_));
       break;
     }
 
     default:
       NOTREACHED();
   }
 }
 
+void ContextMenuNotificationObserver::WaitForMenu() {
+  content::WindowedNotificationObserver menu_observer(
+      chrome::NOTIFICATION_RENDER_VIEW_CONTEXT_MENU_SHOWN,
+      content::NotificationService::AllSources());
+  if (!menu_visible_)
+    menu_observer.Wait();
+  menu_visible_ = false;
+}
+
+base::string16 ContextMenuNotificationObserver::GetSuggestedFilename() {
+  return context_menu_->params().suggested_filename;
+}
+
 void ContextMenuNotificationObserver::ExecuteCommand(
     RenderViewContextMenu* context_menu) {
-  context_menu->ExecuteCommand(command_to_execute_, 0);
-  context_menu->Cancel();
+  switch (command_to_execute_) {
+    case IDC_CONTENT_CONTEXT_OPENLINKNEWTAB: {

[lazyboy, 2014/06/24 01:15:04]

The class is named more generic as ContextMenUNotificationObserver but is doing
very special focused things for NEWTAB and SAVELINKAS.
I'd extract the SaveLinkAs to separate class, possibly extending from
ContextMenuNOtificationObserver, to sth like SaveLinkAsContextMenuObserver or
DownloadContextMenuObserver. Then we can have our own virtual ExecuteCommand
implementation there.
Otherwise this class does not remain generic enough to be used by any test that
requires observer context menu.

[Nikhil, 2014/06/24 10:39:51]

Done.

+      context_menu->ExecuteCommand(command_to_execute_, 0);
+      context_menu->Cancel();
+      break;
+    }
+    case IDC_CONTENT_CONTEXT_SAVELINKAS: {
+      // Don't start download.
+      context_menu->Cancel();
+      break;
+    }
+
+    default:
+      NOTREACHED();
+  }
 }