<webview>: resizing with display:none set should resize on attachment

extensions/renderer/guest_view/guest_view_container.cc

Index: extensions/renderer/guest_view/guest_view_container.cc
diff --git a/extensions/renderer/guest_view/guest_view_container.cc b/extensions/renderer/guest_view/guest_view_container.cc
index 8ef8054ae714ce18059109e5f2029960d9d28113..5563df87f2850dd3a732be31f7a4f29d33eb5932 100644
--- a/extensions/renderer/guest_view/guest_view_container.cc
+++ b/extensions/renderer/guest_view/guest_view_container.cc
@@ -23,6 +23,32 @@ static base::LazyInstance<GuestViewContainerMap> g_guest_view_container_map =
 
 namespace extensions {
 
+GuestViewContainer::AttachRequest::AttachRequest(
+    int element_instance_id,
+    int guest_instance_id,
+    scoped_ptr<base::DictionaryValue> params,
+    v8::Handle<v8::Function> callback,
+    v8::Isolate* isolate)
+    : element_instance_id_(element_instance_id),
+      guest_instance_id_(guest_instance_id),
+      params_(params.Pass()),
+      callback_(callback),
+      isolate_(isolate) {
+}
+
+GuestViewContainer::AttachRequest::~AttachRequest() {
+}
+
+

[lazyboy, 2014/10/03 22:10:19]

nit: remove empty line.

[Fady Samuel, 2014/10/03 22:23:59]

Done.

+bool GuestViewContainer::AttachRequest::HasCallback() const {
+  return !callback_.IsEmpty();
+}
+
+v8::Handle<v8::Function>
+GuestViewContainer::AttachRequest::GetCallback() const {
+  return callback_.NewHandle(isolate_);
+}
+
 GuestViewContainer::GuestViewContainer(
     content::RenderFrame* render_frame,
     const std::string& mime_type)
@@ -32,8 +58,7 @@ GuestViewContainer::GuestViewContainer(
       element_instance_id_(guestview::kInstanceIDNone),
       render_view_routing_id_(render_frame->GetRenderView()->GetRoutingID()),
       attached_(false),
-      attach_pending_(false),
-      isolate_(NULL) {
+      ready_(false) {
 }
 
 GuestViewContainer::~GuestViewContainer() {
@@ -52,29 +77,9 @@ GuestViewContainer* GuestViewContainer::FromID(int render_view_routing_id,
   return it == guest_view_containers->end() ? NULL : it->second;
 }
 
-
-void GuestViewContainer::AttachGuest(int element_instance_id,
-                                     int guest_instance_id,
-                                     scoped_ptr<base::DictionaryValue> params,
-                                     v8::Handle<v8::Function> callback,
-                                     v8::Isolate* isolate) {
-  // GuestViewContainer supports reattachment (i.e. attached_ == true) but not
-  // while a current attach process is pending.
-  if (attach_pending_)
-    return;
-
-  // Step 1, send the attach params to chrome/.
-  render_frame()->Send(new ExtensionHostMsg_AttachGuest(render_view_routing_id_,
-                                                        element_instance_id,
-                                                        guest_instance_id,
-                                                        *params));
-
-  // Step 2, attach plugin through content/.
-  render_frame()->AttachGuest(element_instance_id);
-
-  callback_.reset(callback);
-  isolate_ = isolate;
-  attach_pending_ = true;
+void GuestViewContainer::AttachGuest(linked_ptr<AttachRequest> request) {
+  EnqueueAttachRequest(request);
+  PerformPendingAttachRequest();
 }
 
 void GuestViewContainer::SetElementInstanceID(int element_instance_id) {
@@ -100,6 +105,12 @@ void GuestViewContainer::DidReceiveData(const char* data, int data_length) {
   html_string_ += value;
 }
 
+void GuestViewContainer::Ready() {
+  ready_ = true;
+  CHECK(!pending_response_.get());
+  PerformPendingAttachRequest();
+}
+
 void GuestViewContainer::OnDestruct() {
   // GuestViewContainer's lifetime is managed by BrowserPlugin so don't let
   // RenderFrameObserver self-destruct here.
@@ -137,24 +148,34 @@ void GuestViewContainer::OnCreateMimeHandlerViewGuestACK(
 
 void GuestViewContainer::OnGuestAttached(int element_instance_id,
                                          int guest_routing_id) {
+  CHECK(pending_response_.get());
+
   attached_ = true;
-  attach_pending_ = false;
 
   // If we don't have a callback then there's nothing more to do.
-  if (callback_.IsEmpty())
+  if (!pending_response_->HasCallback()) {
+    pending_response_.reset();
+    PerformPendingAttachRequest();
     return;
+  }
 
   content::RenderView* guest_proxy_render_view =
       content::RenderView::FromRoutingID(guest_routing_id);
   // TODO(fsamuel): Should we be reporting an error to JavaScript or DCHECKing?
-  if (!guest_proxy_render_view)
+  if (!guest_proxy_render_view) {
+    pending_response_.reset();
+    PerformPendingAttachRequest();
     return;
+  }
 
-  v8::HandleScope handle_scope(isolate_);
-  v8::Handle<v8::Function> callback = callback_.NewHandle(isolate_);
+  v8::HandleScope handle_scope(pending_response_->isolate());
+  v8::Handle<v8::Function> callback = pending_response_->GetCallback();
   v8::Handle<v8::Context> context = callback->CreationContext();
-  if (context.IsEmpty())
+  if (context.IsEmpty()) {
+    pending_response_.reset();
+    PerformPendingAttachRequest();
     return;
+  }
 
   blink::WebFrame* frame = guest_proxy_render_view->GetWebView()->mainFrame();
   v8::Local<v8::Value> window = frame->mainWorldScriptContext()->Global();
@@ -168,7 +189,39 @@ void GuestViewContainer::OnGuestAttached(int element_instance_id,
   // Call the AttachGuest API's callback with the guest proxy as the first
   // parameter.
   callback->Call(context->Global(), argc, argv);
-  callback_.reset();
+
+  pending_response_.reset();

[lazyboy, 2014/10/03 22:10:19]

The repetitive pattern (4 times) suggests we would want to add a
OnGuestAttachedInternal that does the v8 stuff and OnGuestAttached would call
that and then pending_reponse_.reset() with PerformPendingAttachRequest();

[Fady Samuel, 2014/10/03 22:23:59]

Done.

+  PerformPendingAttachRequest();
+}
+
+void GuestViewContainer::AttachGuestInternal(
+    linked_ptr<AttachRequest> request) {
+  CHECK(!pending_response_.get());
+  // Step 1, send the attach params to chrome/.
+  render_frame()->Send(
+      new ExtensionHostMsg_AttachGuest(render_view_routing_id_,
+                                       request->element_instance_id(),
+                                       request->guest_instance_id(),
+                                       *request->attach_params()));
+
+  // Step 2, attach plugin through content/.
+  render_frame()->AttachGuest(request->element_instance_id());
+
+  pending_response_ = request;
+}
+
+void GuestViewContainer::EnqueueAttachRequest(
+    linked_ptr<AttachRequest> request) {
+  pending_requests_.push_back(request);
+}
+
+void GuestViewContainer::PerformPendingAttachRequest() {
+  if (!ready_ || pending_requests_.empty() || pending_response_.get())
+    return;
+
+  linked_ptr<AttachRequest> pending_request = pending_requests_.front();
+  pending_requests_.pop_front();
+  AttachGuestInternal(pending_request);
 }
 
 // static