WebContentsImpl: IPC_BEGIN_MESSAGE_MAP_WITH_PARAM everywhere

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 4a1d2fb83c227dde49a3513ebb6c714b2a3067d5..3c9461c90ff51b161ffbc60afb5c91b794e2c86a 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -352,6 +352,14 @@ WebContentsImpl::ColorChooserInfo::ColorChooserInfo(int render_process_id,
 WebContentsImpl::ColorChooserInfo::~ColorChooserInfo() {
 }
 
+bool WebContentsImpl::ColorChooserInfo::Matches(
+    RenderFrameHostImpl* render_frame_host,
+    int color_chooser_id) {
+  return this->render_process_id == render_frame_host->GetProcess()->GetID() &&
+         this->render_frame_id == render_frame_host->GetRoutingID() &&
+         this->identifier == color_chooser_id;
+}
+
 // WebContentsImpl::WebContentsTreeNode ----------------------------------------
 WebContentsImpl::WebContentsTreeNode::WebContentsTreeNode()
     : outer_web_contents_(nullptr),
@@ -438,8 +446,6 @@ WebContentsImpl::WebContentsImpl(BrowserContext* browser_context)
       minimum_zoom_percent_(static_cast<int>(kMinimumZoomFactor * 100)),
       maximum_zoom_percent_(static_cast<int>(kMaximumZoomFactor * 100)),
       zoom_scroll_remainder_(0),
-      render_view_message_source_(NULL),
-      render_frame_message_source_(NULL),
       fullscreen_widget_process_id_(ChildProcessHost::kInvalidUniqueID),
       fullscreen_widget_routing_id_(MSG_ROUTING_NONE),
       fullscreen_widget_had_focus_at_shutdown_(false),
@@ -665,48 +671,60 @@ RenderFrameHostManager* WebContentsImpl::GetRenderManagerForTesting() {
   return GetRenderManager();
 }
 
-bool WebContentsImpl::OnMessageReceived(RenderViewHost* render_view_host,
-                                        const IPC::Message& message) {
-  return OnMessageReceived(render_view_host, NULL, message);
-}
-
-bool WebContentsImpl::OnMessageReceived(RenderViewHost* render_view_host,
-                                        RenderFrameHost* render_frame_host,
+bool WebContentsImpl::OnMessageReceived(RenderViewHostImpl* render_view_host,
                                         const IPC::Message& message) {
-  DCHECK(render_view_host || render_frame_host);
-
-  if (render_view_host) {
-    RenderFrameHost* main_frame = render_view_host->GetMainFrame();
-    if (main_frame) {
-      WebUIImpl* web_ui =
-          static_cast<RenderFrameHostImpl*>(main_frame)->web_ui();
-      if (web_ui && web_ui->OnMessageReceived(message))
-        return true;
-    }
+  RenderFrameHost* main_frame = render_view_host->GetMainFrame();
+  if (main_frame) {
+    WebUIImpl* web_ui = static_cast<RenderFrameHostImpl*>(main_frame)->web_ui();
+    if (web_ui && web_ui->OnMessageReceived(message))
+      return true;
   }
 
-  if (render_frame_host) {
-    for (auto& observer : observers_) {
-      if (observer.OnMessageReceived(message, render_frame_host))
-        return true;
-    }
-  } else {
-    for (auto& observer : observers_) {
-      if (observer.OnMessageReceived(message))
-        return true;
-    }
+  for (auto& observer : observers_) {
+    if (observer.OnMessageReceived(message))

[Charlie Reis, 2016/12/13 19:03:56]

Presumably this should be changed or removed someday?  Seems like we might have
more of these types of bugs in here, where people assume a message came from the
current RVH.  Maybe add a TODO?  (Certainly not something to tackle in this CL.)

[ncarter (slow), 2016/12/14 18:36:47]

Added a TODO

+      return true;
   }
 
-  // Message handlers should be aware of which
-  // RenderViewHost/RenderFrameHost sent the message, which is temporarily
-  // stored in render_(view|frame)_message_source_.
-  if (render_frame_host)
-    render_frame_message_source_ = render_frame_host;
-  else
-    render_view_message_source_ = render_view_host;
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(WebContentsImpl, message, render_view_host)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_DidFirstVisuallyNonEmptyPaint,
+                        OnFirstVisuallyNonEmptyPaint)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_GoToEntryAtOffset, OnGoToEntryAtOffset)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_UpdateZoomLimits, OnUpdateZoomLimits)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_PageScaleFactorChanged,
+                        OnPageScaleFactorChanged)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_EnumerateDirectory, OnEnumerateDirectory)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_AppCacheAccessed, OnAppCacheAccessed)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_WebUISend, OnWebUISend)
+#if BUILDFLAG(ENABLE_PLUGINS)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_RequestPpapiBrokerPermission,
+                        OnRequestPpapiBrokerPermission)
+#endif
+    IPC_MESSAGE_HANDLER(ViewHostMsg_UpdateFaviconURL, OnUpdateFaviconURL)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_ShowValidationMessage,
+                        OnShowValidationMessage)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_HideValidationMessage,
+                        OnHideValidationMessage)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_MoveValidationMessage,
+                        OnMoveValidationMessage)
+#if defined(OS_ANDROID)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_OpenDateTimeDialog, OnOpenDateTimeDialog)
+#endif
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+
+  return handled;
+}
+
+bool WebContentsImpl::OnMessageReceived(RenderFrameHostImpl* render_frame_host,
+                                        const IPC::Message& message) {
+  for (auto& observer : observers_) {
+    if (observer.OnMessageReceived(message, render_frame_host))
+      return true;
+  }
 
   bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(WebContentsImpl, message)
+  IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(WebContentsImpl, message, render_frame_host)
     IPC_MESSAGE_HANDLER(FrameHostMsg_DomOperationResponse,
                         OnDomOperationResponse)
     IPC_MESSAGE_HANDLER(FrameHostMsg_DidChangeThemeColor,
@@ -718,8 +736,6 @@ bool WebContentsImpl::OnMessageReceived(RenderViewHost* render_view_host,
     IPC_MESSAGE_HANDLER(FrameHostMsg_EndColorChooser, OnEndColorChooser)
     IPC_MESSAGE_HANDLER(FrameHostMsg_SetSelectedColorInColorChooser,
                         OnSetSelectedColorInColorChooser)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_DidFirstVisuallyNonEmptyPaint,
-                        OnFirstVisuallyNonEmptyPaint)
     IPC_MESSAGE_HANDLER(FrameHostMsg_DidLoadResourceFromMemoryCache,
                         OnDidLoadResourceFromMemoryCache)
     IPC_MESSAGE_HANDLER(FrameHostMsg_DidDisplayInsecureContent,
@@ -730,11 +746,6 @@ bool WebContentsImpl::OnMessageReceived(RenderViewHost* render_view_host,
                         OnDidDisplayContentWithCertificateErrors)
     IPC_MESSAGE_HANDLER(FrameHostMsg_DidRunContentWithCertificateErrors,
                         OnDidRunContentWithCertificateErrors)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_GoToEntryAtOffset, OnGoToEntryAtOffset)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_UpdateZoomLimits, OnUpdateZoomLimits)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_PageScaleFactorChanged,
-                        OnPageScaleFactorChanged)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_EnumerateDirectory, OnEnumerateDirectory)
     IPC_MESSAGE_HANDLER(FrameHostMsg_RegisterProtocolHandler,
                         OnRegisterProtocolHandler)
     IPC_MESSAGE_HANDLER(FrameHostMsg_UnregisterProtocolHandler,
@@ -742,8 +753,6 @@ bool WebContentsImpl::OnMessageReceived(RenderViewHost* render_view_host,
     IPC_MESSAGE_HANDLER(FrameHostMsg_UpdatePageImportanceSignals,
                         OnUpdatePageImportanceSignals)
     IPC_MESSAGE_HANDLER(FrameHostMsg_Find_Reply, OnFindReply)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_AppCacheAccessed, OnAppCacheAccessed)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_WebUISend, OnWebUISend)
 #if BUILDFLAG(ENABLE_PLUGINS)
     IPC_MESSAGE_HANDLER(FrameHostMsg_PepperInstanceCreated,
                         OnPepperInstanceCreated)
@@ -755,46 +764,22 @@ bool WebContentsImpl::OnMessageReceived(RenderViewHost* render_view_host,
     IPC_MESSAGE_HANDLER(FrameHostMsg_PepperStopsPlayback,
                         OnPepperStopsPlayback)
     IPC_MESSAGE_HANDLER(FrameHostMsg_PluginCrashed, OnPluginCrashed)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_RequestPpapiBrokerPermission,
-                        OnRequestPpapiBrokerPermission)
     IPC_MESSAGE_HANDLER_GENERIC(BrowserPluginHostMsg_Attach,
                                 OnBrowserPluginMessage(render_frame_host,
                                                        message))
 #endif
-    IPC_MESSAGE_HANDLER(ViewHostMsg_UpdateFaviconURL, OnUpdateFaviconURL)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_ShowValidationMessage,
-                        OnShowValidationMessage)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_HideValidationMessage,
-                        OnHideValidationMessage)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_MoveValidationMessage,
-                        OnMoveValidationMessage)
 #if defined(OS_ANDROID)
     IPC_MESSAGE_HANDLER(FrameHostMsg_FindMatchRects_Reply,
                         OnFindMatchRectsReply)
     IPC_MESSAGE_HANDLER(FrameHostMsg_GetNearestFindResult_Reply,
                         OnGetNearestFindResultReply)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_OpenDateTimeDialog,
-                        OnOpenDateTimeDialog)
 #endif
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
-  render_view_message_source_ = NULL;
-  render_frame_message_source_ = NULL;
 
   return handled;
 }
 
-bool WebContentsImpl::HasValidFrameSource() {
-  if (!render_frame_message_source_) {
-    DCHECK(render_view_message_source_);
-    bad_message::ReceivedBadMessage(GetRenderProcessHost(),
-                                    bad_message::WC_INVALID_FRAME_SOURCE);
-    return false;
-  }
-
-  return true;
-}
-
 NavigationControllerImpl& WebContentsImpl::GetController() {
   return controller_;
 }
@@ -2447,21 +2432,25 @@ ScreenOrientationProvider* WebContentsImpl::GetScreenOrientationProvider() {
 }
 
 void WebContentsImpl::OnShowValidationMessage(
+    RenderViewHostImpl* source,
     const gfx::Rect& anchor_in_root_view,
     const base::string16& main_text,
     const base::string16& sub_text) {
+  // TODO(nick): |source|?

[Charlie Reis, 2016/12/13 19:03:56]

I'm with you, but maybe this TODO should be a little clearer.  Something like:
"Should this being using |source|?"

(Or did you specifically mean using |source| instead of |this|?)

[ncarter (slow), 2016/12/14 18:36:47]

Done.

   if (delegate_)
     delegate_->ShowValidationMessage(
         this, anchor_in_root_view, main_text, sub_text);
 }
 
-void WebContentsImpl::OnHideValidationMessage() {
+void WebContentsImpl::OnHideValidationMessage(RenderViewHostImpl* source) {
   if (delegate_)
     delegate_->HideValidationMessage(this);
 }
 
 void WebContentsImpl::OnMoveValidationMessage(
+    RenderViewHostImpl* source,
     const gfx::Rect& anchor_in_root_view) {
+  // TODO(nick): |source|?
   if (delegate_)
     delegate_->MoveValidationMessage(this, anchor_in_root_view);
 }
@@ -3487,7 +3476,8 @@ bool WebContentsImpl::CanOverscrollContent() const {
   return false;
 }
 
-void WebContentsImpl::OnThemeColorChanged(SkColor theme_color) {
+void WebContentsImpl::OnThemeColorChanged(RenderFrameHostImpl* source,

[Charlie Reis, 2016/12/13 19:03:56]

A bunch of these methods don't check |source| either.  Did you determine that
they're safe (as opposed to the ones above)?

Or should we have a more general bug to audit all the message handlers to make
sure they're checking |source| and not worry about the TODOs?

[ncarter (slow), 2016/12/14 18:36:47]

I was adding TODOs as I saw obvious problems, but in a lot of cases I just
wasn't deeply thinking through these cases, since the original goal of this CL
was really just a mechanical rearrangement of IPC dispatch :(

Anyhow, I went through, did more thinking, and triaged them all. Now there are
two dozen TODOs. Please let me know if you see anything I missed.

+                                          SkColor theme_color) {
   // Update the theme color. This is to be published to observers after the
   // first visually non-empty paint.
   theme_color_ = theme_color;
@@ -3501,6 +3491,7 @@ void WebContentsImpl::OnThemeColorChanged(SkColor theme_color) {
 }
 
 void WebContentsImpl::OnDidLoadResourceFromMemoryCache(
+    RenderFrameHostImpl* source,
     const GURL& url,
     const std::string& http_method,
     const std::string& mime_type,
@@ -3522,12 +3513,13 @@ void WebContentsImpl::OnDidLoadResourceFromMemoryCache(
   }
 }
 
-void WebContentsImpl::OnDidDisplayInsecureContent() {
+void WebContentsImpl::OnDidDisplayInsecureContent(RenderFrameHostImpl* source) {
   RecordAction(base::UserMetricsAction("SSL.DisplayedInsecureContent"));
   controller_.ssl_manager()->DidDisplayMixedContent();
 }
 
-void WebContentsImpl::OnDidRunInsecureContent(const GURL& security_origin,
+void WebContentsImpl::OnDidRunInsecureContent(RenderFrameHostImpl* source,
+                                              const GURL& security_origin,
                                               const GURL& target_url) {
   LOG(WARNING) << security_origin << " ran insecure content from "
                << target_url.possibly_invalid_spec();
@@ -3539,11 +3531,13 @@ void WebContentsImpl::OnDidRunInsecureContent(const GURL& security_origin,
 }
 
 void WebContentsImpl::OnDidDisplayContentWithCertificateErrors(
+    RenderFrameHostImpl* source,
     const GURL& url) {
   controller_.ssl_manager()->DidDisplayContentWithCertErrors();
 }
 
 void WebContentsImpl::OnDidRunContentWithCertificateErrors(
+    RenderFrameHostImpl* source,
     const GURL& url) {
   NavigationEntry* entry = controller_.GetVisibleEntry();
   if (!entry)
@@ -3555,43 +3549,36 @@ void WebContentsImpl::OnDidRunContentWithCertificateErrors(
       entry->GetURL().GetOrigin());
 }
 
-void WebContentsImpl::OnDocumentLoadedInFrame() {
-  if (!HasValidFrameSource())
-    return;
-
-  RenderFrameHostImpl* rfh =
-      static_cast<RenderFrameHostImpl*>(render_frame_message_source_);
+void WebContentsImpl::OnDocumentLoadedInFrame(RenderFrameHostImpl* source) {
   for (auto& observer : observers_)
-    observer.DocumentLoadedInFrame(rfh);
+    observer.DocumentLoadedInFrame(source);
 }
 
-void WebContentsImpl::OnDidFinishLoad(const GURL& url) {
-  if (!HasValidFrameSource())
-    return;
-
+void WebContentsImpl::OnDidFinishLoad(RenderFrameHostImpl* source,
+                                      const GURL& url) {
   GURL validated_url(url);
-  RenderProcessHost* render_process_host =
-      render_frame_message_source_->GetProcess();
-  render_process_host->FilterURL(false, &validated_url);
+  source->GetProcess()->FilterURL(false, &validated_url);
 
-  RenderFrameHostImpl* rfh =
-      static_cast<RenderFrameHostImpl*>(render_frame_message_source_);
   for (auto& observer : observers_)
-    observer.DidFinishLoad(rfh, validated_url);
+    observer.DidFinishLoad(source, validated_url);
 }
 
-void WebContentsImpl::OnGoToEntryAtOffset(int offset) {
+void WebContentsImpl::OnGoToEntryAtOffset(RenderViewHostImpl* source,
+                                          int offset) {
+  // All frames are allowed to navigate the global history.
   if (!delegate_ || delegate_->OnGoToEntryOffset(offset))
     controller_.GoToOffset(offset);
 }
 
-void WebContentsImpl::OnUpdateZoomLimits(int minimum_percent,
+void WebContentsImpl::OnUpdateZoomLimits(RenderViewHostImpl* source,
+                                         int minimum_percent,
                                          int maximum_percent) {
   minimum_zoom_percent_ = minimum_percent;
   maximum_zoom_percent_ = maximum_percent;
 }
 
-void WebContentsImpl::OnPageScaleFactorChanged(float page_scale_factor) {
+void WebContentsImpl::OnPageScaleFactorChanged(RenderViewHostImpl* source,
+                                               float page_scale_factor) {
   bool is_one = page_scale_factor == 1.f;
   if (is_one != page_scale_factor_is_one_) {
     page_scale_factor_is_one_ = is_one;
@@ -3599,6 +3586,8 @@ void WebContentsImpl::OnPageScaleFactorChanged(float page_scale_factor) {
     HostZoomMapImpl* host_zoom_map =
         static_cast<HostZoomMapImpl*>(HostZoomMap::GetForWebContents(this));
 
+    // TODO(nick): Explain why we use GetRenderViewHost() here instead of
+    // |source|.

[Charlie Reis, 2016/12/13 19:03:56]

This seems like something we should resolve before landing.  Did you get a sense
that this was intentional?  Maybe wjmaclean@ can help if it's unclear.

[ncarter (slow), 2016/12/14 18:36:47]

I fixed up this one, but will add wjmaclean to the review.

I was trying to avoid having to think through all the consequences here.

     if (host_zoom_map && GetRenderProcessHost()) {
       host_zoom_map->SetPageScaleFactorIsOneForView(
           GetRenderProcessHost()->GetID(), GetRenderViewHost()->GetRoutingID(),
@@ -3610,18 +3599,22 @@ void WebContentsImpl::OnPageScaleFactorChanged(float page_scale_factor) {
     observer.OnPageScaleFactorChanged(page_scale_factor);
 }
 
-void WebContentsImpl::OnEnumerateDirectory(int request_id,
+void WebContentsImpl::OnEnumerateDirectory(RenderViewHostImpl* source,
+                                           int request_id,
                                            const base::FilePath& path) {
   if (!delegate_)
     return;
 
   ChildProcessSecurityPolicyImpl* policy =
       ChildProcessSecurityPolicyImpl::GetInstance();
-  if (policy->CanReadFile(GetRenderProcessHost()->GetID(), path))
+  if (policy->CanReadFile(source->GetProcess()->GetID(), path)) {
+    // TODO(nick): |this| param in the call below ought to be a RenderFrameHost.
     delegate_->EnumerateDirectory(this, request_id, path);
+  }
 }
 
-void WebContentsImpl::OnRegisterProtocolHandler(const std::string& protocol,
+void WebContentsImpl::OnRegisterProtocolHandler(RenderFrameHostImpl* source,
+                                                const std::string& protocol,
                                                 const GURL& url,
                                                 const base::string16& title,
                                                 bool user_gesture) {
@@ -3633,10 +3626,12 @@ void WebContentsImpl::OnRegisterProtocolHandler(const std::string& protocol,
   if (policy->IsPseudoScheme(protocol))
     return;
 
+  // TODO(nick): |source|? FilterURL?
   delegate_->RegisterProtocolHandler(this, protocol, url, user_gesture);
 }
 
-void WebContentsImpl::OnUnregisterProtocolHandler(const std::string& protocol,
+void WebContentsImpl::OnUnregisterProtocolHandler(RenderFrameHostImpl* source,
+                                                  const std::string& protocol,
                                                   const GURL& url,
                                                   bool user_gesture) {
   if (!delegate_)
@@ -3647,78 +3642,77 @@ void WebContentsImpl::OnUnregisterProtocolHandler(const std::string& protocol,
   if (policy->IsPseudoScheme(protocol))
     return;
 
+  // TODO(nick): |source|? FilterURL?
   delegate_->UnregisterProtocolHandler(this, protocol, url, user_gesture);
 }
 
 void WebContentsImpl::OnUpdatePageImportanceSignals(
+    RenderFrameHostImpl* source,
     const PageImportanceSignals& signals) {
   page_importance_signals_ = signals;
 }
 
-void WebContentsImpl::OnFindReply(int request_id,
+void WebContentsImpl::OnFindReply(RenderFrameHostImpl* source,
+                                  int request_id,
                                   int number_of_matches,
                                   const gfx::Rect& selection_rect,
                                   int active_match_ordinal,
                                   bool final_update) {
   // Forward the find reply to the FindRequestManager, along with the
   // RenderFrameHost associated with the frame that the reply came from.
-  GetOrCreateFindRequestManager()->OnFindReply(render_frame_message_source_,
-                                               request_id,
-                                               number_of_matches,
-                                               selection_rect,
-                                               active_match_ordinal,
-                                               final_update);
+  GetOrCreateFindRequestManager()->OnFindReply(
+      source, request_id, number_of_matches, selection_rect,
+      active_match_ordinal, final_update);
 }
 
 #if defined(OS_ANDROID)
 void WebContentsImpl::OnFindMatchRectsReply(
+    RenderFrameHostImpl* source,
     int version,
     const std::vector<gfx::RectF>& rects,
     const gfx::RectF& active_rect) {
-  GetOrCreateFindRequestManager()->OnFindMatchRectsReply(
-      render_frame_message_source_, version, rects, active_rect);
+  GetOrCreateFindRequestManager()->OnFindMatchRectsReply(source, version, rects,
+                                                         active_rect);
 }
 
-void WebContentsImpl::OnGetNearestFindResultReply(int request_id,
+void WebContentsImpl::OnGetNearestFindResultReply(RenderFrameHostImpl* source,
+                                                  int request_id,
                                                   float distance) {
   GetOrCreateFindRequestManager()->OnGetNearestFindResultReply(
-      render_frame_message_source_, request_id, distance);
+      source, request_id, distance);
 }
 
 void WebContentsImpl::OnOpenDateTimeDialog(
+    RenderViewHostImpl* source,
     const ViewHostMsg_DateTimeDialogValue_Params& value) {
-  date_time_chooser_->ShowDialog(GetTopLevelNativeWindow(),
-                                 GetRenderViewHost(),
-                                 value.dialog_type,
-                                 value.dialog_value,
-                                 value.minimum,
-                                 value.maximum,
-                                 value.step,
-                                 value.suggestions);
+  date_time_chooser_->ShowDialog(
+      GetTopLevelNativeWindow(), source, value.dialog_type, value.dialog_value,
+      value.minimum, value.maximum, value.step, value.suggestions);
 }
 #endif
 
-void WebContentsImpl::OnDomOperationResponse(const std::string& json_string) {
+void WebContentsImpl::OnDomOperationResponse(RenderFrameHostImpl* source,
+                                             const std::string& json_string) {
   std::string json = json_string;
   NotificationService::current()->Notify(NOTIFICATION_DOM_OPERATION_RESPONSE,
                                          Source<WebContents>(this),
                                          Details<std::string>(&json));
 }
 
-void WebContentsImpl::OnAppCacheAccessed(const GURL& manifest_url,
+void WebContentsImpl::OnAppCacheAccessed(RenderViewHostImpl* source,
+                                         const GURL& manifest_url,
                                          bool blocked_by_policy) {
+  // TODO(nick): |source|? FilterURL?
   // Notify observers about navigation.
   for (auto& observer : observers_)
     observer.AppCacheAccessed(manifest_url, blocked_by_policy);
 }
 
 void WebContentsImpl::OnOpenColorChooser(
+    RenderFrameHostImpl* source,
     int color_chooser_id,
     SkColor color,
     const std::vector<ColorSuggestion>& suggestions) {
-  if (!HasValidFrameSource())
-    return;
-
   ColorChooser* new_color_chooser = delegate_ ?
       delegate_->OpenColorChooser(this, color, suggestions) :
       NULL;
@@ -3728,50 +3722,54 @@ void WebContentsImpl::OnOpenColorChooser(
     color_chooser_info_->chooser->End();
 
   color_chooser_info_.reset(new ColorChooserInfo(
-      render_frame_message_source_->GetProcess()->GetID(),
-      render_frame_message_source_->GetRoutingID(),
-      new_color_chooser,
+      source->GetProcess()->GetID(), source->GetRoutingID(), new_color_chooser,
       color_chooser_id));
 }
 
-void WebContentsImpl::OnEndColorChooser(int color_chooser_id) {
+void WebContentsImpl::OnEndColorChooser(RenderFrameHostImpl* source,
+                                        int color_chooser_id) {
   if (color_chooser_info_ &&
-      color_chooser_id == color_chooser_info_->identifier)
+      color_chooser_info_->Matches(source, color_chooser_id))
     color_chooser_info_->chooser->End();
 }
 
-void WebContentsImpl::OnSetSelectedColorInColorChooser(int color_chooser_id,
-                                                       SkColor color) {
+void WebContentsImpl::OnSetSelectedColorInColorChooser(
+    RenderFrameHostImpl* source,
+    int color_chooser_id,
+    SkColor color) {
   if (color_chooser_info_ &&
-      color_chooser_id == color_chooser_info_->identifier)
+      color_chooser_info_->Matches(source, color_chooser_id))
     color_chooser_info_->chooser->SetSelectedColor(color);
 }
 
 // This exists for render views that don't have a WebUI, but do have WebUI
 // bindings enabled.
-void WebContentsImpl::OnWebUISend(const GURL& source_url,
+void WebContentsImpl::OnWebUISend(RenderViewHostImpl* source,
+                                  const GURL& source_url,
                                   const std::string& name,
                                   const base::ListValue& args) {
+  // TODO(nick): |source|? FilterURL?
   if (delegate_)
     delegate_->WebUISend(this, source_url, name, args);
 }
 
 #if BUILDFLAG(ENABLE_PLUGINS)
-void WebContentsImpl::OnPepperInstanceCreated(int32_t pp_instance) {
+void WebContentsImpl::OnPepperInstanceCreated(RenderFrameHostImpl* source,
+                                              int32_t pp_instance) {
   for (auto& observer : observers_)
     observer.PepperInstanceCreated();
-  pepper_playback_observer_->PepperInstanceCreated(
-      render_frame_message_source_, pp_instance);
+  pepper_playback_observer_->PepperInstanceCreated(source, pp_instance);
 }
 
-void WebContentsImpl::OnPepperInstanceDeleted(int32_t pp_instance) {
+void WebContentsImpl::OnPepperInstanceDeleted(RenderFrameHostImpl* source,
+                                              int32_t pp_instance) {
   for (auto& observer : observers_)
     observer.PepperInstanceDeleted();
-  pepper_playback_observer_->PepperInstanceDeleted(
-      render_frame_message_source_, pp_instance);
+  pepper_playback_observer_->PepperInstanceDeleted(source, pp_instance);
 }
 
-void WebContentsImpl::OnPepperPluginHung(int plugin_child_id,
+void WebContentsImpl::OnPepperPluginHung(RenderFrameHostImpl* source,
+                                         int plugin_child_id,
                                          const base::FilePath& path,
                                          bool is_hung) {
   UMA_HISTOGRAM_COUNTS("Pepper.PluginHung", 1);
@@ -3780,43 +3778,52 @@ void WebContentsImpl::OnPepperPluginHung(int plugin_child_id,
     observer.PluginHungStatusChanged(plugin_child_id, path, is_hung);
 }
 
-void WebContentsImpl::OnPepperStartsPlayback(int32_t pp_instance) {
-  pepper_playback_observer_->PepperStartsPlayback(
-      render_frame_message_source_, pp_instance);
+void WebContentsImpl::OnPepperStartsPlayback(RenderFrameHostImpl* source,
+                                             int32_t pp_instance) {
+  pepper_playback_observer_->PepperStartsPlayback(source, pp_instance);
 }
 
-void WebContentsImpl::OnPepperStopsPlayback(int32_t pp_instance) {
-  pepper_playback_observer_->PepperStopsPlayback(
-      render_frame_message_source_, pp_instance);
+void WebContentsImpl::OnPepperStopsPlayback(RenderFrameHostImpl* source,
+                                            int32_t pp_instance) {
+  pepper_playback_observer_->PepperStopsPlayback(source, pp_instance);
 }
 
-void WebContentsImpl::OnPluginCrashed(const base::FilePath& plugin_path,
+void WebContentsImpl::OnPluginCrashed(RenderFrameHostImpl* source,
+                                      const base::FilePath& plugin_path,
                                       base::ProcessId plugin_pid) {
   for (auto& observer : observers_)
     observer.PluginCrashed(plugin_path, plugin_pid);
 }
 
 void WebContentsImpl::OnRequestPpapiBrokerPermission(
-    int routing_id,
+    RenderViewHostImpl* source,
+    int ppb_broker_route_id,
     const GURL& url,
     const base::FilePath& plugin_path) {
+  base::Callback<void(bool)> permission_result_callback = base::Bind(
+      &WebContentsImpl::SendPpapiBrokerPermissionResult, base::Unretained(this),
+      source->GetProcess()->GetID(), ppb_broker_route_id);
   if (!delegate_) {
-    OnPpapiBrokerPermissionResult(routing_id, false);
+    permission_result_callback.Run(false);
     return;
   }
 
-  if (!delegate_->RequestPpapiBrokerPermission(
-      this, url, plugin_path,
-      base::Bind(&WebContentsImpl::OnPpapiBrokerPermissionResult,
-                 base::Unretained(this), routing_id))) {
+  if (!delegate_->RequestPpapiBrokerPermission(this, url, plugin_path,
+                                               permission_result_callback)) {
     NOTIMPLEMENTED();
-    OnPpapiBrokerPermissionResult(routing_id, false);
+    permission_result_callback.Run(false);
   }
 }
 
-void WebContentsImpl::OnPpapiBrokerPermissionResult(int routing_id,
-                                                    bool result) {
-  Send(new ViewMsg_PpapiBrokerPermissionResult(routing_id, result));
+void WebContentsImpl::SendPpapiBrokerPermissionResult(int process_id,
+                                                      int ppb_broker_route_id,
+                                                      bool result) {
+  RenderProcessHost* rph = RenderProcessHost::FromID(process_id);
+  if (rph) {
+    // Note that this is ViewMsg_, but it's actually sent to a PPB_Broker_Impl.
+    rph->Send(
+        new ViewMsg_PpapiBrokerPermissionResult(ppb_broker_route_id, result));

[Charlie Reis, 2016/12/13 19:03:56]

Nice.  Let's include a TODO for your idea to move these away from being view
messages, given the swapped out filter concern for OOPIFs.

[ncarter (slow), 2016/12/14 18:36:47]

The CL to fix this is already complete; I'll send it to jam@ as soon as this one
lands.

+  }
 }
 
 void WebContentsImpl::OnBrowserPluginMessage(RenderFrameHost* render_frame_host,
@@ -3828,14 +3835,13 @@ void WebContentsImpl::OnBrowserPluginMessage(RenderFrameHost* render_frame_host,
 #endif  // BUILDFLAG(ENABLE_PLUGINS)
 
 void WebContentsImpl::OnUpdateFaviconURL(
+    RenderViewHostImpl* source,
     const std::vector<FaviconURL>& candidates) {
   // We get updated favicon URLs after the page stops loading. If a cross-site
   // navigation occurs while a page is still loading, the initial page
   // may stop loading and send us updated favicon URLs after the navigation
   // for the new page has committed.
-  RenderViewHostImpl* rvhi =
-      static_cast<RenderViewHostImpl*>(render_view_message_source_);
-  if (!rvhi->is_active())
+  if (!source->is_active())
     return;
 
   for (auto& observer : observers_)
@@ -3858,7 +3864,7 @@ void WebContentsImpl::SetIsOverlayContent(bool is_overlay_content) {
   is_overlay_content_ = is_overlay_content;
 }
 
-void WebContentsImpl::OnFirstVisuallyNonEmptyPaint() {
+void WebContentsImpl::OnFirstVisuallyNonEmptyPaint(RenderViewHostImpl* source) {
   for (auto& observer : observers_)
     observer.DidFirstVisuallyNonEmptyPaint();
 
@@ -4047,11 +4053,6 @@ void WebContentsImpl::NotifyNavigationEntryCommitted(
     observer.NavigationEntryCommitted(load_details);
 }
 
-bool WebContentsImpl::OnMessageReceived(RenderFrameHost* render_frame_host,
-                                        const IPC::Message& message) {
-  return OnMessageReceived(NULL, render_frame_host, message);
-}
-
 void WebContentsImpl::OnAssociatedInterfaceRequest(
     RenderFrameHost* render_frame_host,
     const std::string& interface_name,