Commit f68022ef authored by W. James MacLean's avatar W. James MacLean Committed by Commit Bot

guest_view.js should handle actionQueue items in order.

At present, the implementation of destroyImpl() in guest_view_iframe.js
calls handleCallback() in the *middle* of the function, meaning
remaining items in the action queue may complete before destroyImpl()
finishes updating the state, leading to subsequent commands being
dropped if functions like create/attach encounter the non-updated
state.

This fixes a regression introduced in
https://chromium-review.googlesource.com/c/chromium/src/+/528392. It's
not obvious if it introduced any failures, but the original CL bypassed
browser tests as extensions/renderer/BUILD.gn was missing the relevant
files (restored in
https://chromium-review.googlesource.com/c/chromium/src/+/812109).

Bug: 
Change-Id: I83db799b73174d3e996677333e01192bf5321460
Reviewed-on: https://chromium-review.googlesource.com/812005Reviewed-by: default avatarKevin McNee <mcnee@chromium.org>
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522438}
parent 40a824f7
...@@ -259,7 +259,10 @@ GuestViewImpl.prototype.destroyImpl = function(callback) { ...@@ -259,7 +259,10 @@ GuestViewImpl.prototype.destroyImpl = function(callback) {
GuestViewInternal.destroyGuest( GuestViewInternal.destroyGuest(
this.id, $Function.bind(this.handleCallback, this, callback)); this.id, $Function.bind(this.handleCallback, this, callback));
// Reset the state of the destroyed guest; // Reset the state of the destroyed guest; it's ok to do this after shipping
// the callback to the GuestViewInternal api, since it runs asynchronously,
// and the changes below will happen before the next item from the action
// queue is executed.
this.contentWindow = null; this.contentWindow = null;
this.id = 0; this.id = 0;
this.internalInstanceId = 0; this.internalInstanceId = 0;
......
...@@ -127,8 +127,6 @@ GuestViewImpl.prototype.destroyImpl = function(callback) { ...@@ -127,8 +127,6 @@ GuestViewImpl.prototype.destroyImpl = function(callback) {
GuestViewInternalNatives.DetachGuest(this.internalInstanceId); GuestViewInternalNatives.DetachGuest(this.internalInstanceId);
} }
this.handleCallback(callback);
// Reset the state of the destroyed guest; // Reset the state of the destroyed guest;
this.contentWindow = null; this.contentWindow = null;
this.id = 0; this.id = 0;
...@@ -137,4 +135,8 @@ GuestViewImpl.prototype.destroyImpl = function(callback) { ...@@ -137,4 +135,8 @@ GuestViewImpl.prototype.destroyImpl = function(callback) {
if (ResizeEvent.hasListener(this.callOnResize)) { if (ResizeEvent.hasListener(this.callOnResize)) {
ResizeEvent.removeListener(this.callOnResize); ResizeEvent.removeListener(this.callOnResize);
} }
// Handle callback at end to avoid handling items in the action queue out of
// order, since the callback is run synchronously here.
this.handleCallback(callback);
}; };
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment