Arc app integration in shelf launcher context menu

chrome/browser/ui/ash/launcher/extension_launcher_context_menu.cc

+// Copyright 2016 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/ui/ash/launcher/extension_launcher_context_menu.h"
+
+#include "ash/shelf/shelf.h"
+#include "ash/shelf/shelf_item_delegate.h"
+#include "base/bind.h"
+#include "chrome/browser/extensions/context_menu_matcher.h"
+#include "chrome/browser/extensions/extension_util.h"
+#include "chrome/browser/prefs/incognito_mode_prefs.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
+#include "chrome/common/extensions/extension_constants.h"
+#include "chrome/grit/generated_resources.h"
+#include "content/public/common/context_menu_params.h"
+#include "grit/ash_strings.h"
+#include "ui/base/l10n/l10n_util.h"
+
+namespace {
+
+bool MenuItemHasLauncherContext(const extensions::MenuItem* item) {
+  return item->contexts().Contains(extensions::MenuItem::LAUNCHER);
+}
+
+}  // namespace
+
+ExtensionLauncherContextMenu::ExtensionLauncherContextMenu(
+    ChromeLauncherController* controller,
+    const ash::ShelfItem* item,
+    ash::Shelf* shelf)
+    : LauncherContextMenu(controller, item, shelf) {
+  Init();
+}
+
+ExtensionLauncherContextMenu::~ExtensionLauncherContextMenu() {}
+
+void ExtensionLauncherContextMenu::Init() {

[stevenjb, 2016/04/06 00:06:49]

Can you try using a lower --similarity value with git cl upload so that we can
maybe get a diff from launcher_context_menu.cc? I assume this logic hasn't
changed?

[lgcheng, 2016/04/06 17:02:42]

This logic is not changed, but I change the implementation a little:
In original implementation, it was:
if (item_.id != 0) {
  details chunk.......
}

Now there is no need to check item id in this function, so the details chunk is
moved 2 spaces forward, causing git sees the whole chunk as new implementation.
Direct access to private member item_ , controller_, shelf_ in original
implementation is also changed to protected method of base class item(),
controller(), shelf(). These make the similarity of
ExtensionLauncherContextMenu.cc and LauncherContextMenu.cc very low.

I need to set the similarity value really low(26%) to make git treat
launcher_context_menu.cc as the base file of extension_launcher_context_menu.cc.
But this would cause some head files treat unrelated SimpleMenuModel subclass's
head files(other contextmenu implementation, e.g applist contextmenu) in other
directory as base files, which would also annoy the review. Do you have any
suggestions?

[stevenjb, 2016/04/06 17:16:55]

I think the thing to do then is to split this CL up:
a) Everything except the addition of arc_launcher_context_menu.* (and the
changes integrating it)
b) Add arc_launcher_context_menu.*

Then you can use a low similarity value for a) and a normal one for b).

Otherwise reviewing the changes ot this file is going to be a lot more work.

[lgcheng, 2016/04/06 18:04:19]

Ignoring Arc related updates.
When I try to lower the similarity threshold (30%), the head file of extension
implementation itself first detects unrelated head file as base file. (See
detail below)

chrome/browser/ui/ash/launcher/extension_launcher_context_menu.cc               
                           | 220
++++++++++++++++++++++++++++++++++++++++++++++++++++++
chrome/browser/ui/{app_list/extension_app_context_menu.h =>
ash/launcher/extension_launcher_context_menu.h} |  55 ++++++--------

---------------------------------------------------------------------------------------------
Which means even if I split the Arc related code out of this cl, it still
doesn't work.
Is it possible that we have this workaround.
I create a "fake cl" only for review with only one file
extension_launcher_context_menu.cc. lower the threshold so that
launcher_context_menu.cc can be detected as base file of this for you to review
and leave this cl unchanged? 
Otherwise, I can't find a clean solution (I have asked everybody around for
ideas, but none has a clean solution either).

What do you suggest?

Thanks

+  extension_items_.reset(new extensions::ContextMenuMatcher(
+      controller()->profile(), this, this,
+      base::Bind(MenuItemHasLauncherContext)));
+  if (item().type == ash::TYPE_APP_SHORTCUT ||
+      item().type == ash::TYPE_WINDOWED_APP) {
+    // V1 apps can be started from the menu - but V2 apps should not.
+    if (!controller()->IsPlatformApp(item().id)) {
+      AddItem(MENU_OPEN_NEW, base::string16());
+      AddSeparator(ui::NORMAL_SEPARATOR);
+    }
+    AddPinMenu();
+    if (controller()->IsOpen(item().id))
+      AddItemWithStringId(MENU_CLOSE, IDS_LAUNCHER_CONTEXT_MENU_CLOSE);
+
+    if (!controller()->IsPlatformApp(item().id) &&
+        item().type != ash::TYPE_WINDOWED_APP) {
+      AddSeparator(ui::NORMAL_SEPARATOR);
+      if (extensions::util::IsNewBookmarkAppsEnabled()) {
+        // With bookmark apps enabled, hosted apps launch in a window by
+        // default. This menu item is re-interpreted as a single, toggle-able
+        // option to launch the hosted app as a tab.
+        AddCheckItemWithStringId(LAUNCH_TYPE_WINDOW,
+                                 IDS_APP_CONTEXT_MENU_OPEN_WINDOW);
+      } else {
+        AddCheckItemWithStringId(LAUNCH_TYPE_REGULAR_TAB,
+                                 IDS_APP_CONTEXT_MENU_OPEN_REGULAR);
+        AddCheckItemWithStringId(LAUNCH_TYPE_PINNED_TAB,
+                                 IDS_APP_CONTEXT_MENU_OPEN_PINNED);
+        AddCheckItemWithStringId(LAUNCH_TYPE_WINDOW,
+                                 IDS_APP_CONTEXT_MENU_OPEN_WINDOW);
+        // Even though the launch type is Full Screen it is more accurately
+        // described as Maximized in Ash.
+        AddCheckItemWithStringId(LAUNCH_TYPE_FULLSCREEN,
+                                 IDS_APP_CONTEXT_MENU_OPEN_MAXIMIZED);
+      }
+    }
+  } else if (item().type == ash::TYPE_BROWSER_SHORTCUT) {
+    AddItemWithStringId(MENU_NEW_WINDOW, IDS_APP_LIST_NEW_WINDOW);
+    if (!controller()->IsLoggedInAsGuest()) {
+      AddItemWithStringId(MENU_NEW_INCOGNITO_WINDOW,
+                          IDS_APP_LIST_NEW_INCOGNITO_WINDOW);
+    }
+  } else if (item().type == ash::TYPE_DIALOG) {
+    AddItemWithStringId(MENU_CLOSE, IDS_LAUNCHER_CONTEXT_MENU_CLOSE);
+  } else {
+    if (item().type == ash::TYPE_PLATFORM_APP)
+      AddItemWithStringId(MENU_PIN, IDS_LAUNCHER_CONTEXT_MENU_PIN);
+    bool show_close_button = controller()->IsOpen(item().id);
+#if defined(OS_CHROMEOS)
+    if (extension_misc::IsImeMenuExtensionId(
+            controller()->GetAppIDForShelfID(item().id)))
+      show_close_button = false;
+#endif
+    if (show_close_button)
+      AddItemWithStringId(MENU_CLOSE, IDS_LAUNCHER_CONTEXT_MENU_CLOSE);
+  }
+  AddSeparator(ui::NORMAL_SEPARATOR);
+  if (item().type == ash::TYPE_APP_SHORTCUT ||
+      item().type == ash::TYPE_WINDOWED_APP ||
+      item().type == ash::TYPE_PLATFORM_APP) {
+    const extensions::MenuItem::ExtensionKey app_key(
+        controller()->GetAppIDForShelfID(item().id));
+    if (!app_key.empty()) {
+      int index = 0;
+      extension_items_->AppendExtensionItems(app_key, base::string16(), &index,
+                                             false);  // is_action_menu
+      AddSeparator(ui::NORMAL_SEPARATOR);
+    }
+  }
+  AddAutohideAlignmentWallpaperMenu();
+}
+
+bool ExtensionLauncherContextMenu::IsItemForCommandIdDynamic(
+    int command_id) const {
+  return command_id == MENU_OPEN_NEW;
+}
+
+base::string16 ExtensionLauncherContextMenu::GetLabelForCommandId(
+    int command_id) const {
+  if (command_id == MENU_OPEN_NEW) {
+    if (item().type == ash::TYPE_PLATFORM_APP)
+      return l10n_util::GetStringUTF16(IDS_APP_LIST_CONTEXT_MENU_NEW_WINDOW);
+    switch (controller()->GetLaunchType(item().id)) {
+      case extensions::LAUNCH_TYPE_PINNED:
+      case extensions::LAUNCH_TYPE_REGULAR:
+        return l10n_util::GetStringUTF16(IDS_APP_LIST_CONTEXT_MENU_NEW_TAB);
+      case extensions::LAUNCH_TYPE_FULLSCREEN:
+      case extensions::LAUNCH_TYPE_WINDOW:
+        return l10n_util::GetStringUTF16(IDS_APP_LIST_CONTEXT_MENU_NEW_WINDOW);
+      default:
+        NOTREACHED();
+        return base::string16();
+    }
+  }
+  NOTREACHED();
+  return base::string16();
+}
+
+bool ExtensionLauncherContextMenu::IsCommandIdChecked(int command_id) const {
+  switch (command_id) {
+    case LAUNCH_TYPE_PINNED_TAB:
+      return controller()->GetLaunchType(item().id) ==
+             extensions::LAUNCH_TYPE_PINNED;
+    case LAUNCH_TYPE_REGULAR_TAB:
+      return controller()->GetLaunchType(item().id) ==
+             extensions::LAUNCH_TYPE_REGULAR;
+    case LAUNCH_TYPE_WINDOW:
+      return controller()->GetLaunchType(item().id) ==
+             extensions::LAUNCH_TYPE_WINDOW;
+    case LAUNCH_TYPE_FULLSCREEN:
+      return controller()->GetLaunchType(item().id) ==
+             extensions::LAUNCH_TYPE_FULLSCREEN;
+    default:
+      if (command_id < MENU_ITEM_COUNT)
+        return LauncherContextMenu::IsCommandIdChecked(command_id);
+      return (extension_items_ &&
+              extension_items_->IsCommandIdChecked(command_id));
+  }
+}
+
+bool ExtensionLauncherContextMenu::IsCommandIdEnabled(int command_id) const {
+  switch (command_id) {
+    case MENU_NEW_WINDOW:
+      // "Normal" windows are not allowed when incognito is enforced.
+      return IncognitoModePrefs::GetAvailability(
+                 controller()->profile()->GetPrefs()) !=
+             IncognitoModePrefs::FORCED;
+    case MENU_NEW_INCOGNITO_WINDOW:
+      // Incognito windows are not allowed when incognito is disabled.
+      return IncognitoModePrefs::GetAvailability(
+                 controller()->profile()->GetPrefs()) !=
+             IncognitoModePrefs::DISABLED;
+    default:
+      if (command_id < MENU_ITEM_COUNT)
+        return LauncherContextMenu::IsCommandIdEnabled(command_id);
+      return (extension_items_ &&
+              extension_items_->IsCommandIdEnabled(command_id));
+  }
+}
+
+void ExtensionLauncherContextMenu::ExecuteCommand(int command_id,
+                                                  int event_flags) {
+  if (IsCommonCommandId(command_id))
+    return LauncherContextMenu::ExecuteCommand(command_id, event_flags);
+  switch (static_cast<MenuItem>(command_id)) {
+    case LAUNCH_TYPE_PINNED_TAB:
+      controller()->SetLaunchType(item().id, extensions::LAUNCH_TYPE_PINNED);
+      break;
+    case LAUNCH_TYPE_REGULAR_TAB:
+      controller()->SetLaunchType(item().id, extensions::LAUNCH_TYPE_REGULAR);
+      break;
+    case LAUNCH_TYPE_WINDOW: {
+      extensions::LaunchType launch_type = extensions::LAUNCH_TYPE_WINDOW;
+      // With bookmark apps enabled, hosted apps can only toggle between
+      // LAUNCH_WINDOW and LAUNCH_REGULAR.
+      if (extensions::util::IsNewBookmarkAppsEnabled()) {
+        launch_type = controller()->GetLaunchType(item().id) ==
+                              extensions::LAUNCH_TYPE_WINDOW
+                          ? extensions::LAUNCH_TYPE_REGULAR
+                          : extensions::LAUNCH_TYPE_WINDOW;
+      }
+      controller()->SetLaunchType(item().id, launch_type);
+      break;
+    }
+    case LAUNCH_TYPE_FULLSCREEN:
+      controller()->SetLaunchType(item().id,
+                                  extensions::LAUNCH_TYPE_FULLSCREEN);
+      break;
+    case MENU_NEW_WINDOW:
+      controller()->CreateNewWindow();
+      break;
+    case MENU_NEW_INCOGNITO_WINDOW:
+      controller()->CreateNewIncognitoWindow();
+      break;
+    default:
+      if (extension_items_) {
+        extension_items_->ExecuteCommand(command_id, nullptr, nullptr,
+                                         content::ContextMenuParams());
+      }
+  }
+}