Move ContextMenu show/hide state tracking to WebContents

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index cef65a924bd6e3cc1036361d9f338427a92ec0de..76c56a705ed1b041986e626cf21cf3c4eb247a8e 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -516,6 +516,7 @@ WebContentsImpl::WebContentsImpl(BrowserContext* browser_context)
 #endif  // !defined(OS_ANDROID)
       mouse_lock_widget_(nullptr),
       is_overlay_content_(false),
+      showing_context_menu_(false),
       loading_weak_factory_(this),
       weak_factory_(this) {
   frame_tree_.SetFrameRemoveListener(
@@ -4400,7 +4401,7 @@ void WebContentsImpl::ShowContextMenu(RenderFrameHost* render_frame_host,
                                       const ContextMenuParams& params) {
   // If a renderer fires off a second command to show a context menu before the
   // first context menu is closed, just ignore it. https://crbug.com/707534
-  if (GetRenderWidgetHostView()->IsShowingContextMenu())
+  if (showing_context_menu_)
     return;
 
   ContextMenuParams context_menu_params(params);
@@ -4514,6 +4515,21 @@ bool WebContentsImpl::IsFocusedElementEditable() {
   return frame && frame->has_focused_editable_element();
 }
 
+bool WebContentsImpl::IsShowingContextMenu() const {
+  return showing_context_menu_;
+}
+
+void WebContentsImpl::SetShowingContextMenu(bool showing) {
+  DCHECK_NE(showing_context_menu_, showing);
+  showing_context_menu_ = showing;
+#if defined(OS_MACOSX)
+  if (auto* view = GetRenderWidgetHostView()) {
+    static_cast<RenderWidgetHostViewBase*>(view)->SetShowingContextMenu(
+        showing);

[Avi (use Gerrit), 2017/06/06 14:18:30]

This concerns me. The function |SetShowingContextMenu| on
|RenderWidgetHostViewBase| is marked as "Called by WebContentsImpl..." with no
indication that it's called only on certain platforms. If we really only want to
call it on the Mac, then #if MACOSX it, otherwise remove the #if here. Let's
have those two match.

[EhsanK, 2017/06/06 16:17:38]

Thanks! I agree the mismatch does not look nice. But here is the problem I have:
inside RWHVBase we do not have any platform specific code (I guess we should
not). We also do not refer to any RWHVX's here in WebContentsImpl. So the only
option seems to be just to call this on all RWHVBase's which is a NOOP except
for Mac.

+  }
+#endif
+}
+
 void WebContentsImpl::ClearFocusedElement() {
   if (auto* frame = GetFocusedFrame())
     frame->ClearFocusedElement();
@@ -5666,6 +5682,10 @@ void WebContentsImpl::ShowInsecureLocalhostWarningIfNeeded() {
                          " releasing your website to the public."));
 }
 
+bool WebContentsImpl::IsShowingContextMenuOnPage() const {
+  return showing_context_menu_;
+}
+
 void WebContentsImpl::NotifyPreferencesChanged() {
   std::set<RenderViewHost*> render_view_host_set;
   for (FrameTreeNode* node : frame_tree_.Nodes()) {