change FrameMsg_ViewChanged to take a struct

the viz::FrameSinkId is not used when mus is hosting viz (the
current code DCHECKs in mus as we end up supplying an invalid
FrameSinkId, which triggers a DCHECK). To address this address a
struct with param traits to ensure the FrameSinkId is set when
appropriate.

BUG=804508
TEST=covered by tests

Change-Id: I1949d546d7aa7db39dc0ad2116694b614b3b875f
Reviewed-on: https://chromium-review.googlesource.com/887619
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532245}

content/browser/frame_host/cross_process_frame_connector.cc

@@ -26,6 +26,7 @@
 #include "content/public/common/screen_info.h"
 #include "gpu/ipc/common/gpu_messages.h"
 #include "third_party/WebKit/public/platform/WebInputEvent.h"
+#include "ui/base/ui_base_switches_util.h"
 #include "ui/gfx/geometry/dip_util.h"
 
 namespace content {
@@ -93,9 +94,11 @@ void CrossProcessFrameConnector::SetView(RenderWidgetHostViewChildFrame* view) {
     view_->SetFrameConnectorDelegate(this);
     if (is_hidden_)
       OnVisibilityChanged(false);
+    FrameMsg_ViewChanged_Params params;
+    if (!switches::IsMusHostingViz())
+      params.frame_sink_id = view_->GetFrameSinkId();
     frame_proxy_in_parent_renderer_->Send(new FrameMsg_ViewChanged(
-        frame_proxy_in_parent_renderer_->GetRoutingID(),
-        view_->GetFrameSinkId()));
+        frame_proxy_in_parent_renderer_->GetRoutingID(), params));
   }
 }
 

content/common/BUILD.gn

@@ -130,6 +130,8 @@ source_set("common") {
     "font_list_mac.mm",
     "font_list_win.cc",
     "frame_message_enums.h",
+    "frame_message_structs.cc",
+    "frame_message_structs.h",
     "frame_messages.h",
     "frame_messages_forward.h",
     "frame_owner_properties.cc",

content/common/content_param_traits.cc

@@ -7,11 +7,13 @@
 #include <stddef.h>
 
 #include "base/strings/string_number_conversions.h"
+#include "content/common/frame_message_structs.h"
 #include "ipc/ipc_mojo_message_helper.h"
 #include "ipc/ipc_mojo_param_traits.h"
 #include "net/base/ip_endpoint.h"
 #include "third_party/WebKit/common/message_port/message_port_channel.h"
 #include "ui/accessibility/ax_modes.h"
+#include "ui/base/ui_base_switches_util.h"
 #include "ui/events/blink/web_input_event_traits.h"
 
 namespace IPC {
@@ -134,6 +136,37 @@ void ParamTraits<scoped_refptr<storage::BlobHandle>>::Log(const param_type& p,
   l->append("<storage::BlobHandle>");
 }
 
+// static
+void ParamTraits<content::FrameMsg_ViewChanged_Params>::Write(
+    base::Pickle* m,
+    const param_type& p) {
+  DCHECK(switches::IsMusHostingViz() ||
+         (p.frame_sink_id.has_value() && p.frame_sink_id->is_valid()));
+  WriteParam(m, p.frame_sink_id);
+}
+
+bool ParamTraits<content::FrameMsg_ViewChanged_Params>::Read(
+    const base::Pickle* m,
+    base::PickleIterator* iter,
+    param_type* r) {
+  if (!ReadParam(m, iter, &(r->frame_sink_id)))
+    return false;
+  if (!switches::IsMusHostingViz() &&
+      (!r->frame_sink_id || !r->frame_sink_id->is_valid())) {
+    NOTREACHED();
+    return false;
+  }
+  return true;
+}
+
+// static
+void ParamTraits<content::FrameMsg_ViewChanged_Params>::Log(const param_type& p,
+                                                            std::string* l) {
+  l->append("(");
+  LogParam(p.frame_sink_id, l);
+  l->append(")");
+}
+
 }  // namespace IPC
 
 // Generate param traits write methods.

content/common/content_param_traits.h

@@ -25,6 +25,10 @@ namespace blink {
 class MessagePortChannel;
 }
 
+namespace content {
+struct FrameMsg_ViewChanged_Params;
+}
+
 namespace IPC {
 
 template <>
@@ -82,6 +86,16 @@ struct CONTENT_EXPORT ParamTraits<scoped_refptr<storage::BlobHandle>> {
   static void Log(const param_type& p, std::string* l);
 };
 
+template <>
+struct CONTENT_EXPORT ParamTraits<content::FrameMsg_ViewChanged_Params> {
+  using param_type = content::FrameMsg_ViewChanged_Params;
+  static void Write(base::Pickle* m, const param_type& p);
+  static bool Read(const base::Pickle* m,
+                   base::PickleIterator* iter,
+                   param_type* p);
+  static void Log(const param_type& p, std::string* l);
+};
+
 }  // namespace IPC
 
 #endif  // CONTENT_COMMON_CONTENT_PARAM_TRAITS_H_

content/common/frame_message_structs.cc

+// Copyright 2018 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 "content/common/frame_message_structs.h"
+
+namespace content {
+
+FrameMsg_ViewChanged_Params::FrameMsg_ViewChanged_Params() = default;
+
+FrameMsg_ViewChanged_Params::~FrameMsg_ViewChanged_Params() = default;
+
+}  // namespace content

content/common/frame_message_structs.h

+// Copyright 2018 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.
+
+#ifndef CONTENT_COMMON_FRAME_MESSAGE_STRUCTS_H_
+#define CONTENT_COMMON_FRAME_MESSAGE_STRUCTS_H_
+
+#include "base/optional.h"
+#include "components/viz/common/surfaces/frame_sink_id.h"
+#include "content/common/content_export.h"
+
+namespace content {
+
+struct CONTENT_EXPORT FrameMsg_ViewChanged_Params {
+  FrameMsg_ViewChanged_Params();
+  ~FrameMsg_ViewChanged_Params();
+
+  // |frame_sink_id| is not used when mus is hosting viz.
+  base::Optional<viz::FrameSinkId> frame_sink_id;
+};
+
+}  // namespace content
+
+#endif  // CONTENT_COMMON_FRAME_MESSAGE_STRUCTS_H_

content/common/frame_messages.h

@@ -26,6 +26,7 @@
 #include "content/common/download/mhtml_save_status.h"
 #include "content/common/features.h"
 #include "content/common/frame_message_enums.h"
+#include "content/common/frame_message_structs.h"
 #include "content/common/frame_owner_properties.h"
 #include "content/common/frame_replication_state.h"
 #include "content/common/navigation_gesture.h"
@@ -951,7 +952,8 @@ IPC_MESSAGE_ROUTED2(FrameMsg_DidUpdateOrigin,
 
 // Notifies RenderFrameProxy that its associated RenderWidgetHostView has
 // changed.
-IPC_MESSAGE_ROUTED1(FrameMsg_ViewChanged, viz::FrameSinkId /* frame_sink_id */)
+IPC_MESSAGE_ROUTED1(FrameMsg_ViewChanged,
+                    content::FrameMsg_ViewChanged_Params /* params */)
 
 // Notifies this frame or proxy that it is now focused.  This is used to
 // support cross-process focused frame changes.

content/renderer/render_frame_proxy.cc

@@ -12,7 +12,7 @@
 #include "base/lazy_instance.h"
 #include "components/viz/common/features.h"
 #include "content/common/content_switches_internal.h"
-#include "content/common/frame_messages.h"
+#include "content/common/frame_message_structs.h"
 #include "content/common/frame_owner_properties.h"
 #include "content/common/frame_replication_state.h"
 #include "content/common/input_messages.h"
@@ -443,10 +443,11 @@ void RenderFrameProxy::OnDidStartLoading() {
   web_frame_->DidStartLoading();
 }
 
-void RenderFrameProxy::OnViewChanged(const viz::FrameSinkId& frame_sink_id) {
+void RenderFrameProxy::OnViewChanged(
+    const FrameMsg_ViewChanged_Params& params) {
   // In mash the FrameSinkId comes from RendererWindowTreeClient.
   if (!switches::IsMusHostingViz())
-    frame_sink_id_ = frame_sink_id;
+    frame_sink_id_ = *params.frame_sink_id;
 
   // Resend the FrameRects and allocate a new viz::LocalSurfaceId when the view
   // changes.

content/renderer/render_frame_proxy.h

@@ -9,6 +9,7 @@
 #include "base/memory/ref_counted.h"
 #include "components/viz/common/surfaces/parent_local_surface_id_allocator.h"
 #include "content/common/content_export.h"
+#include "content/common/frame_messages.h"
 #include "content/public/common/screen_info.h"
 #include "ipc/ipc_listener.h"
 #include "ipc/ipc_sender.h"
@@ -211,7 +212,7 @@ class CONTENT_EXPORT RenderFrameProxy : public IPC::Listener,
   void OnIntrinsicSizingInfoOfChildChanged(
       blink::WebIntrinsicSizingInfo sizing_info);
   void OnUpdateOpener(int opener_routing_id);
-  void OnViewChanged(const viz::FrameSinkId& frame_sink_id);
+  void OnViewChanged(const FrameMsg_ViewChanged_Params& params);
   void OnDidStopLoading();
   void OnDidUpdateFramePolicy(const blink::FramePolicy& frame_policy);
   void OnDidSetActiveSandboxFlags(blink::WebSandboxFlags active_sandbox_flags);