<webview>: Fix navigation/attachment race

content/renderer/browser_plugin/browser_plugin.cc

 // Copyright (c) 2012 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/renderer/browser_plugin/browser_plugin.h"
 
 #include "base/command_line.h"
 #include "base/json/json_string_value_serializer.h"
 #include "base/message_loop.h"
 #include "base/string_number_conversions.h"
@@ skipping 119 matching lines @@
       current_nav_entry_index_(0),
       nav_entry_count_(0),
       compositing_enabled_(false),
       weak_ptr_factory_(this) {
 }
 
 BrowserPlugin::~BrowserPlugin() {
   // If the BrowserPlugin has never navigated then the browser process and
   // BrowserPluginManager don't know about it and so there is nothing to do
   // here.
-  if (!HasGuest())
+  if (!HasInstanceID())
     return;
   browser_plugin_manager()->RemoveBrowserPlugin(instance_id_);
   browser_plugin_manager()->Send(
       new BrowserPluginHostMsg_PluginDestroyed(render_view_routing_id_,
                                                instance_id_));
 }
 
 /*static*/
 BrowserPlugin* BrowserPlugin::FromContainer(
     WebKit::WebPluginContainer* container) {
@@ skipping 143 matching lines @@
   min_width = min_width ? min_width : width();
   // For autosize, minWidth should not be bigger than maxWidth.
   return std::min(min_width, GetAdjustedMaxWidth());
 }
 
 std::string BrowserPlugin::GetPartitionAttribute() const {
   return GetDOMAttributeValue(browser_plugin::kAttributePartition);
 }
 
 void BrowserPlugin::ParseNameAttribute() {
-  if (!HasGuest())
+  if (!HasInstanceID())
     return;
   browser_plugin_manager()->Send(
       new BrowserPluginHostMsg_SetName(render_view_routing_id_,
                                        instance_id_,
                                        GetNameAttribute()));
 }
 
 bool BrowserPlugin::ParseSrcAttribute(std::string* error_message) {
   if (!valid_partition_id_) {
     *error_message = browser_plugin::kErrorInvalidPartition;
     return false;
   }
   std::string src = GetSrcAttribute();
   if (src.empty())
     return true;
 
   // If we haven't created the guest yet, do so now. We will navigate it right
   // after creation. If |src| is empty, we can delay the creation until we
   // actually need it.
-  if (!HasGuest()) {
+  if (!HasInstanceID()) {
     // On initial navigation, we request an instance ID from the browser
     // process. We essentially ignore all subsequent calls to SetSrcAttribute
     // until we receive an instance ID. |before_first_navigation_|
     // prevents BrowserPlugin from allocating more than one instance ID.
     // Upon receiving an instance ID from the browser process, we continue
     // the process of navigation by populating the
     // BrowserPluginHostMsg_Attach_Params with the current state of
     // BrowserPlugin and sending a BrowserPluginHostMsg_CreateGuest to the
     // browser process in order to create a new guest.
     if (before_first_navigation_) {
@@ skipping 23 matching lines @@
   if (current_auto_size) {
     params->max_size = gfx::Size(GetAdjustedMaxWidth(), GetAdjustedMaxHeight());
     params->min_size = gfx::Size(GetAdjustedMinWidth(), GetAdjustedMinHeight());
   }
 }
 
 void BrowserPlugin::UpdateGuestAutoSizeState(bool current_auto_size) {
   // If we haven't yet heard back from the guest about the last resize request,
   // then we don't issue another request until we do in
   // BrowserPlugin::UpdateRect.
-  if (!HasGuest() || !resize_ack_received_)
+  if (!HasInstanceID() || !resize_ack_received_)
     return;
   BrowserPluginHostMsg_AutoSize_Params auto_size_params;
   BrowserPluginHostMsg_ResizeGuest_Params resize_guest_params;
   if (current_auto_size) {
     GetDamageBufferWithSizeParams(&auto_size_params, &resize_guest_params);
   } else {
     GetDamageBufferWithSizeParams(NULL, &resize_guest_params);
   }
   resize_ack_received_ = false;
   browser_plugin_manager()->Send(
@@ skipping 454 matching lines @@
   WebKit::WebPluginContainer* plugin_container =
       plugin_element.pluginContainer();
   if (!plugin_container)
     return false;
 
   BrowserPlugin* browser_plugin =
       BrowserPlugin::FromContainer(plugin_container);
   if (!browser_plugin)
     return false;
 
-  // If the BrowserPlugin already has a guest attached to it then we probably
-  // shouldn't allow attaching a different guest.
+  // If the BrowserPlugin has already begun to navigate then we shouldn't allow
+  // attaching a different guest.

[lazyboy, 2013/05/29 00:38:25]

Can you add a comment: in which case HasNavigated() == true while attachTo is
called? I'm not sure I totally follow.

   // TODO(fsamuel): We may wish to support reattaching guests in the future:
   // http://crbug.com/156219.
-  if (browser_plugin->HasGuest())
+  if (browser_plugin->HasNavigated())
     return false;
 
   browser_plugin->Attach(window_id);
   return true;
 }
 
-bool BrowserPlugin::HasGuest() const {
+bool BrowserPlugin::HasNavigated() const {
+  return !before_first_navigation_;
+}
+
+bool BrowserPlugin::HasInstanceID() const {
   return instance_id_ != browser_plugin::kInstanceIDNone;
 }
 
 bool BrowserPlugin::CanGoBack() const {
   return nav_entry_count_ > 1 && current_nav_entry_index_ > 0;
 }
 
 bool BrowserPlugin::CanGoForward() const {
   return current_nav_entry_index_ >= 0 &&
       current_nav_entry_index_ < (nav_entry_count_ - 1);
@@ skipping 25 matching lines @@
   } else {
     persist_storage_ = false;
   }
 
   valid_partition_id_ = true;
   storage_partition_id_ = input;
   return true;
 }
 
 bool BrowserPlugin::CanRemovePartitionAttribute(std::string* error_message) {
-  if (HasGuest())
+  if (HasInstanceID())
     *error_message = browser_plugin::kErrorCannotRemovePartition;
-  return !HasGuest();
+  return !HasInstanceID();
 }
 
 void BrowserPlugin::ParseAttributes() {
   // TODO(mthiesse): Handle errors here?
   std::string error;
   ParsePartitionAttribute(&error);
 
   // Parse the 'src' attribute last, as it will set the has_navigated_ flag to
   // true, which prevents changing the 'partition' attribute.
   ParseSrcAttribute(&error);
@@ skipping 108 matching lines @@
     // don't need to worry about BrowserPlugin going away.
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
         base::Bind(&BrowserPlugin::OnRequestObjectGarbageCollected,
                    plugin,
                    request_id));
   }
 }
 
 void BrowserPlugin::Back() {
-  if (!HasGuest())
+  if (!HasInstanceID())
     return;
   browser_plugin_manager()->Send(
       new BrowserPluginHostMsg_Go(render_view_routing_id_,
                                   instance_id_, -1));
 }
 
 void BrowserPlugin::Forward() {
-  if (!HasGuest())
+  if (!HasInstanceID())
     return;
   browser_plugin_manager()->Send(
       new BrowserPluginHostMsg_Go(render_view_routing_id_,
                                   instance_id_, 1));
 }
 
 void BrowserPlugin::Go(int relative_index) {
-  if (!HasGuest())
+  if (!HasInstanceID())
     return;
   browser_plugin_manager()->Send(
       new BrowserPluginHostMsg_Go(render_view_routing_id_,
                                   instance_id_,
                                   relative_index));
 }
 
 void BrowserPlugin::TerminateGuest() {
-  if (!HasGuest() || guest_crashed_)
+  if (!HasInstanceID() || guest_crashed_)
     return;
   browser_plugin_manager()->Send(
       new BrowserPluginHostMsg_TerminateGuest(render_view_routing_id_,
                                               instance_id_));
 }
 
 void BrowserPlugin::Stop() {
-  if (!HasGuest())
+  if (!HasInstanceID())
     return;
   browser_plugin_manager()->Send(
       new BrowserPluginHostMsg_Stop(render_view_routing_id_,
                                     instance_id_));
 }
 
 void BrowserPlugin::Reload() {
-  if (!HasGuest())
+  if (!HasInstanceID())
     return;
   browser_plugin_manager()->Send(
       new BrowserPluginHostMsg_Reload(render_view_routing_id_,
                                       instance_id_));
 }
 
 void BrowserPlugin::UpdateGuestFocusState() {
-  if (!HasGuest())
+  if (!HasInstanceID())
     return;
   bool should_be_focused = ShouldGuestBeFocused();
   browser_plugin_manager()->Send(new BrowserPluginHostMsg_SetFocus(
       render_view_routing_id_,
       instance_id_,
       should_be_focused));
 }
 
 bool BrowserPlugin::ShouldGuestBeFocused() const {
   bool embedder_focused = false;
@@ skipping 155 matching lines @@
       SkIntToScalar(plugin_rect_.height()));
   canvas->clipRect(image_data_rect);
   // Paint black or white in case we have nothing in our backing store or we
   // need to show a gutter.
   SkPaint paint;
   paint.setStyle(SkPaint::kFill_Style);
   paint.setColor(guest_crashed_ ? SK_ColorBLACK : SK_ColorWHITE);
   canvas->drawRect(image_data_rect, paint);
   // Stay a solid color if we have never set a non-empty src, or we don't have a
   // backing store.
-  if (!backing_store_.get() || !HasGuest())
+  if (!backing_store_.get() || !HasInstanceID())
     return;
   float inverse_scale_factor =  1.0f / backing_store_->GetScaleFactor();
   canvas->scale(inverse_scale_factor, inverse_scale_factor);
   canvas->drawBitmap(backing_store_->GetBitmap(), 0, 0);
 }
 
 bool BrowserPlugin::InBounds(const gfx::Point& position) const {
   // Note that even for plugins that are rotated using rotate transformations,
   // we use the the |plugin_rect_| provided by updateGeometry, which means we
   // will be off if |position| is within the plugin rect but does not fall
@@ skipping 48 matching lines @@
 }
 
 void BrowserPlugin::updateGeometry(
     const WebRect& window_rect,
     const WebRect& clip_rect,
     const WebVector<WebRect>& cut_outs_rects,
     bool is_visible) {
   int old_width = width();
   int old_height = height();
   plugin_rect_ = window_rect;
-  if (!HasGuest())
+  if (!HasInstanceID())
     return;
 
   // In AutoSize mode, guests don't care when the BrowserPlugin container is
   // resized. If |!resize_ack_received_|, then we are still waiting on a
   // previous resize to be ACK'ed and so we don't issue additional resizes
   // until the previous one is ACK'ed.
   // TODO(mthiesse): Assess the performance of calling GetAutoSizeAttribute() on
   // resize.
   if (!resize_ack_received_ ||
       (old_width == window_rect.width && old_height == window_rect.height) ||
@@ skipping 114 matching lines @@
 
   if (ShouldGuestBeFocused() != old_guest_focus_state)
     UpdateGuestFocusState();
 }
 
 void BrowserPlugin::updateVisibility(bool visible) {
   if (visible_ == visible)
     return;
 
   visible_ = visible;
-  if (!HasGuest())
+  if (!HasInstanceID())
     return;
 
   if (compositing_helper_)
     compositing_helper_->UpdateVisibility(visible);
 
   browser_plugin_manager()->Send(new BrowserPluginHostMsg_SetVisibility(
       render_view_routing_id_,
       instance_id_,
       visible));
 }
 
 bool BrowserPlugin::acceptsInputEvents() {
   return true;
 }
 
 bool BrowserPlugin::handleInputEvent(const WebKit::WebInputEvent& event,
                                      WebKit::WebCursorInfo& cursor_info) {
-  if (guest_crashed_ || !HasGuest() ||
+  if (guest_crashed_ || !HasInstanceID() ||
       event.type == WebKit::WebInputEvent::ContextMenu)
     return false;
 
   const WebKit::WebInputEvent* modified_event = &event;
   scoped_ptr<WebKit::WebTouchEvent> touch_event;
   // WebKit gives BrowserPlugin a list of touches that are down, but the browser
   // process expects a list of all touches. We modify the TouchEnd event here to
   // match these expectations.
   if (event.type == WebKit::WebInputEvent::TouchEnd) {
     const WebKit::WebTouchEvent* orig_touch_event =
@@ skipping 15 matching lines @@
                                                 modified_event));
   webkit_glue::GetWebKitCursorInfo(cursor_, &cursor_info);
   return true;
 }
 
 bool BrowserPlugin::handleDragStatusUpdate(WebKit::WebDragStatus drag_status,
                                            const WebKit::WebDragData& drag_data,
                                            WebKit::WebDragOperationsMask mask,
                                            const WebKit::WebPoint& position,
                                            const WebKit::WebPoint& screen) {
-  if (guest_crashed_ || !HasGuest())
+  if (guest_crashed_ || !HasInstanceID())
     return false;
   browser_plugin_manager()->Send(
       new BrowserPluginHostMsg_DragStatusUpdate(
         render_view_routing_id_,
         instance_id_,
         drag_status,
         WebDropData(drag_data),
         mask,
         position));
   return true;
@@ skipping 49 matching lines @@
     const WebKit::WebMouseEvent& event) {
   browser_plugin_manager()->Send(
       new BrowserPluginHostMsg_HandleInputEvent(render_view_routing_id_,
                                                 instance_id_,
                                                 plugin_rect_,
                                                 &event));
   return true;
 }
 
 }  // namespace content