Disable Print in the context menu when a modal dialog is shown

chrome/browser/renderer_context_menu/render_view_context_menu.cc

Index: chrome/browser/renderer_context_menu/render_view_context_menu.cc
diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu.cc b/chrome/browser/renderer_context_menu/render_view_context_menu.cc
index 210adc6d75d706936ca9186a699bc107e60d67e6..7b69204f111a89cd2b1e45441fee0f89533ac86e 100644
--- a/chrome/browser/renderer_context_menu/render_view_context_menu.cc
+++ b/chrome/browser/renderer_context_menu/render_view_context_menu.cc
@@ -1427,8 +1427,6 @@ bool RenderViewContextMenu::IsCommandIdEnabled(int id) const {
   int content_restrictions = 0;
   if (core_tab_helper)
     content_restrictions = core_tab_helper->content_restrictions();
-  if (id == IDC_PRINT && (content_restrictions & CONTENT_RESTRICTION_PRINT))

[Lei Zhang, 2016/08/04 23:54:08]

Maybe just leave this check in since it's relatively cheap to do?

[takumif, 2016/08/05 00:40:08]

Done.

-    return false;
 
   if (id == IDC_SAVE_PAGE &&
       (content_restrictions & CONTENT_RESTRICTION_SAVE)) {
@@ -1583,9 +1581,7 @@ bool RenderViewContextMenu::IsCommandIdEnabled(int id) const {
              incognito_avail != IncognitoModePrefs::DISABLED;
 
     case IDC_PRINT:
-      return prefs->GetBoolean(prefs::kPrintingEnabled) &&
-             (params_.media_type == WebContextMenuData::MediaTypeNone ||
-              params_.media_flags & WebContextMenuData::MediaCanPrint);
+      return IsPrintPreviewEnabled();
 
     case IDC_CONTENT_CONTEXT_SEARCHWEBFOR:
     case IDC_CONTENT_CONTEXT_GOTOURL:
@@ -2082,6 +2078,14 @@ bool RenderViewContextMenu::IsPasteAndMatchStyleEnabled() const {
       ui::CLIPBOARD_TYPE_COPY_PASTE);
 }
 
+bool RenderViewContextMenu::IsPrintPreviewEnabled() const {
+  Browser* browser = chrome::FindLastActive();

[Lei Zhang, 2016/08/04 23:54:09]

Seems everyone else here is calling
chrome::FindBrowserWithWebContents(source_web_contents_);

Does that seem more appropriate?

[takumif, 2016/08/05 00:40:08]

chrome::FindBrowserWithWebContents returns null in some cases (in particular
when a modal dialog is open), so just for disabling Print for modal dialogs,
that'd work as well. We wouldn't even have to call chrome::CanPrint in that
case. However, that feels a bit less straightforward to me, and I thought it
might be better to be consistent with the hotdog menu option, which relies on
chrome::CanPrint. What do you think?

[Lei Zhang, 2016/08/05 01:48:47]

How is checking chrome::FindBrowserWithWebContents() returning a nullptr any
different from checking if chrome::FindLastActive() returning a nullptr?

[takumif, 2016/08/05 18:12:29]

It was unexpected to me that chrome::FindBrowserWithWebContents() would return a
nullptr when a modal dialog is open, but it might be better to just take note of
that in a comment. Changing.

+  return browser &&
+         chrome::CanPrint(browser) &&
+         (params_.media_type == WebContextMenuData::MediaTypeNone ||

[Lei Zhang, 2016/08/04 23:54:09]

Check these separately first because it is the cheapest check?

[takumif, 2016/08/05 00:40:08]

Done.

+          params_.media_flags & WebContextMenuData::MediaCanPrint);
+}
+
 bool RenderViewContextMenu::IsRouteMediaEnabled() const {
   if (!media_router::MediaRouterEnabled(browser_context_))
     return false;