Cross-process postMessage

content/browser/content_frame.cc

+// Copyright (c) 2011 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_frame.h"

[jam, 2011/12/22 20:06:53]

nit: full path

+
+#include "content/browser/frame_map.h"
+#include "content/browser/renderer_host/render_view_host.h"
+#include "content/public/browser/browser_context.h"
+
+namespace content {
+
+ContentFrame::WebKitFrameIdTuple::WebKitFrameIdTuple(int proc_host_id,
+                                                     int route_id,
+                                                     int64 frame_id)
+  : process_host_id(proc_host_id),
+    route_id(route_id),
+    frame_id(frame_id) {
+}
+
+bool ContentFrame::WebKitFrameIdTuple::operator==(
+    const WebKitFrameIdTuple& o) const {

[awong, 2011/12/21 01:06:38]

single letter variable names are frowned upon.

+  return o.process_host_id == process_host_id &&
+         o.frame_id == frame_id;
+}
+
+ContentFrame::ContentFrame(int64 id,
+                           bool is_top_level,
+                           TabContents& tab_contents,
+                           ContentFrame* opener)
+  : id_(id),
+    current_webkit_frame_(WebKitFrameIdTuple(-1, -1, -1)),
+    is_top_level_(is_top_level),
+    tab_contents_(tab_contents),
+    opener_(opener) {
+  if (is_top_level)
+    new Observer(this);

[awong, 2011/12/21 01:06:38]

Doesn't this leak?

I think you need to keep the Observer in a scoped_ptr<>.  Then you can also just
memberwise initalize this too.

+}
+
+ContentFrame::~ContentFrame() {
+}
+
+void ContentFrame::UpdateFrame(int new_process_host_id,
+                               int new_route_id,
+                               int64 new_frame_id) {
+  WebKitFrameIdTuple newWebKitTuple(new_process_host_id, new_route_id,

[awong, 2011/12/21 01:06:38]

local variables are lowercase_with_underscores.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Explic...

+                                    new_frame_id);
+  current_webkit_frame_ = newWebKitTuple;
+
+  all_webkit_frames_.push_back(newWebKitTuple);
+  // TODO(supersat): Clear out subframes?

[awong, 2011/12/21 01:06:38]

Explain why?

+}
+
+ContentFrame::Observer::Observer(

[awong, 2011/12/21 01:06:38]

Can be on one line.

+    ContentFrame* frame)
+  : TabContentsObserver(&frame->tab_contents()),
+    frame_(frame) {
+}
+
+void ContentFrame::Observer::DidCommitProvisionalLoadForFrame(
+    int64 frame_id,
+    bool is_main_frame,
+    const GURL& url,
+    content::PageTransition transition_type) {
+  content::FrameMap& mapper =
+      frame_->tab_contents_.browser_context()->frame_mapper();
+
+  DLOG(WARNING) << "Navigating frame, Process = "
+      << frame_->tab_contents_.render_view_host()->process()->GetID()
+      << ", Renderer = "
+      << frame_->tab_contents_.render_view_host()->routing_id()
+      << ", frame = " << frame_id;
+
+  if (is_main_frame) {
+    mapper.UpdateFrame(
+        frame_, frame_->tab_contents_.render_view_host()->process()->GetID(),
+        frame_->tab_contents_.render_view_host()->routing_id(), frame_id);
+    DLOG(WARNING) << "Content Frame ID = " << frame_->id();
+  } else {
+    // If we're navigating a subframe, we may need to create a ContentFrame
+    // TODO(supersat): This is hack until we get proper subframe support
+    ContentFrame* frame = mapper.FindRendererFrame(
+        frame_->tab_contents_.render_view_host()->process()->GetID(),
+        frame_id);
+
+    if (!frame) {
+      frame = mapper.InitializeFrame(mapper.AllocateFrameId(), false,
+                                     frame_->tab_contents_, frame_);
+      mapper.UpdateFrame(frame,
+          frame_->tab_contents_.render_view_host()->process()->GetID(),
+          frame_->tab_contents_.render_view_host()->routing_id(),
+          frame_id);
+    }
+
+    DLOG(WARNING) << "Content Frame ID = " << frame->id();
+  }
+}
+
+void ContentFrame::Observer::TabContentsDestroyed(TabContents* tab) {
+  delete this;

[awong, 2011/12/21 01:56:07]

Ah, I see why there is no leak.

Would it make more sense to use a scoped_ptr and to unregister in the destructor
of your Observer class?  Or is this the "right" way to do things?  I don't use
observers much...

+}
+
+}  // namespace content