diff options
author | sky <sky@chromium.org> | 2015-09-01 17:12:18 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-09-02 00:12:58 +0000 |
commit | 19644b33d97a68c53621eca83f34ccc9097fcd8c (patch) | |
tree | 5d9ba3834e9ed73835e7f3bf2d987ad4b07ef017 /components/html_viewer | |
parent | b9a8751fe3056400f7f524a9e7538f9493f51298 (diff) | |
download | chromium_src-19644b33d97a68c53621eca83f34ccc9097fcd8c.zip chromium_src-19644b33d97a68c53621eca83f34ccc9097fcd8c.tar.gz chromium_src-19644b33d97a68c53621eca83f34ccc9097fcd8c.tar.bz2 |
Makes html_viewer only swap to remote when necessary
Previously on navigation the code preemptively swapped from local to
remote. This is wrong and problematic if the frame was going to be
local after the navigate.
The fix is to change the frame server code to consult the content
handler id when doing a navigation. If the id is the same as the
existing id, then the same renderer is going to be used. If the same
renderer is used OnConnect() is supplied a different argument to indicate the
new connection corresponds to an existing frame. Additionally a
ViewTreeClient is not obtained for the frame (this is because the
html_viewer already has a View for the frame).
BUG=479172,490221,523408
TEST=covered by tests
R=ben@chromium.org
Review URL: https://codereview.chromium.org/1327673004
Cr-Commit-Position: refs/heads/master@{#346794}
Diffstat (limited to 'components/html_viewer')
-rw-r--r-- | components/html_viewer/BUILD.gn | 3 | ||||
-rw-r--r-- | components/html_viewer/DEPS | 2 | ||||
-rw-r--r-- | components/html_viewer/ax_provider_apptest.cc | 5 | ||||
-rw-r--r-- | components/html_viewer/document_resource_waiter.cc | 29 | ||||
-rw-r--r-- | components/html_viewer/document_resource_waiter.h | 16 | ||||
-rw-r--r-- | components/html_viewer/html_document_oopif.cc | 118 | ||||
-rw-r--r-- | components/html_viewer/html_document_oopif.h | 21 | ||||
-rw-r--r-- | components/html_viewer/html_frame.cc | 30 | ||||
-rw-r--r-- | components/html_viewer/html_frame.h | 19 | ||||
-rw-r--r-- | components/html_viewer/html_frame_apptest.cc | 50 | ||||
-rw-r--r-- | components/html_viewer/html_frame_delegate.h | 19 | ||||
-rw-r--r-- | components/html_viewer/html_frame_tree_manager.cc | 35 | ||||
-rw-r--r-- | components/html_viewer/html_viewer_switches.cc | 14 | ||||
-rw-r--r-- | components/html_viewer/html_viewer_switches.h | 19 |
14 files changed, 263 insertions, 117 deletions
diff --git a/components/html_viewer/BUILD.gn b/components/html_viewer/BUILD.gn index 58747dc..0f46ed7 100644 --- a/components/html_viewer/BUILD.gn +++ b/components/html_viewer/BUILD.gn @@ -94,8 +94,6 @@ source_set("lib") { "html_frame_tree_manager.h", "html_viewer.cc", "html_viewer.h", - "html_viewer_switches.cc", - "html_viewer_switches.h", "html_widget.cc", "html_widget.h", "media_factory.cc", @@ -154,6 +152,7 @@ source_set("lib") { "//components/view_manager/public/interfaces", "//components/webcrypto", "//gin", + "//mandoline/tab:switches", "//mandoline/tab/public/interfaces", "//media", "//media/blink", diff --git a/components/html_viewer/DEPS b/components/html_viewer/DEPS index 72b8f90..3a9aa57 100644 --- a/components/html_viewer/DEPS +++ b/components/html_viewer/DEPS @@ -17,6 +17,8 @@ include_rules = [ "+gin", "+mandoline/services", "+mandoline/tab/public/interfaces", + "+mandoline/tab/web_view_switches.cc", + "+mandoline/tab/web_view_switches.h", "+media", "+mojo/application", "+mojo/cc", diff --git a/components/html_viewer/ax_provider_apptest.cc b/components/html_viewer/ax_provider_apptest.cc index 5386a30..27942fa 100644 --- a/components/html_viewer/ax_provider_apptest.cc +++ b/components/html_viewer/ax_provider_apptest.cc @@ -97,8 +97,9 @@ TEST_F(AXProviderTest, HelloWorld) { mandoline::FrameTreeClientPtr frame_tree_client; connection->ConnectToService(&frame_tree_client); - frame_tree_client->OnConnect(frame_tree_server_ptr.Pass(), 1u, - array.Pass()); + frame_tree_client->OnConnect( + frame_tree_server_ptr.Pass(), 1u, embed_view->id(), + mandoline::VIEW_CONNECT_TYPE_USE_NEW, array.Pass(), base::Closure()); // Connect to the AxProvider of the HTML document and get the AxTree. AxProviderPtr ax_provider; diff --git a/components/html_viewer/document_resource_waiter.cc b/components/html_viewer/document_resource_waiter.cc index cbe5060..8506b57 100644 --- a/components/html_viewer/document_resource_waiter.cc +++ b/components/html_viewer/document_resource_waiter.cc @@ -9,6 +9,8 @@ #include "components/html_viewer/html_frame_tree_manager.h" #include "components/view_manager/public/cpp/view.h" +using mandoline::ViewConnectType; + namespace html_viewer { DocumentResourceWaiter::DocumentResourceWaiter(GlobalState* global_state, @@ -19,6 +21,8 @@ DocumentResourceWaiter::DocumentResourceWaiter(GlobalState* global_state, response_(response.Pass()), root_(nullptr), change_id_(0u), + view_id_(0u), + view_connect_type_(mandoline::VIEW_CONNECT_TYPE_USE_NEW), frame_tree_client_binding_(this) {} DocumentResourceWaiter::~DocumentResourceWaiter() { @@ -29,12 +33,18 @@ void DocumentResourceWaiter::Release( frame_tree_client_request, mandoline::FrameTreeServerPtr* frame_tree_server, mojo::Array<mandoline::FrameDataPtr>* frame_data, - uint32_t* change_id) { + uint32_t* change_id, + uint32_t* view_id, + ViewConnectType* view_connect_type, + OnConnectCallback* on_connect_callback) { DCHECK(IsReady()); *frame_tree_client_request = frame_tree_client_request_.Pass(); *frame_tree_server = server_.Pass(); *frame_data = frame_data_.Pass(); *change_id = change_id_; + *view_id = view_id_; + *view_connect_type = view_connect_type_; + *on_connect_callback = on_connect_callback_; } mojo::URLResponsePtr DocumentResourceWaiter::ReleaseURLResponse() { @@ -42,8 +52,9 @@ mojo::URLResponsePtr DocumentResourceWaiter::ReleaseURLResponse() { } bool DocumentResourceWaiter::IsReady() const { - return root_ && root_->viewport_metrics().device_pixel_ratio != 0.0f && - !frame_data_.is_null(); + return (!frame_data_.is_null() && + ((view_connect_type_ == mandoline::VIEW_CONNECT_TYPE_USE_EXISTING) || + (root_ && root_->viewport_metrics().device_pixel_ratio != 0.0f))); } void DocumentResourceWaiter::Bind( @@ -58,11 +69,17 @@ void DocumentResourceWaiter::Bind( void DocumentResourceWaiter::OnConnect( mandoline::FrameTreeServerPtr server, uint32_t change_id, - mojo::Array<mandoline::FrameDataPtr> frame_data) { + uint32_t view_id, + ViewConnectType view_connect_type, + mojo::Array<mandoline::FrameDataPtr> frame_data, + const OnConnectCallback& callback) { DCHECK(frame_data_.is_null()); change_id_ = change_id; + view_id_ = view_id; + view_connect_type_ = view_connect_type; server_ = server.Pass(); frame_data_ = frame_data.Pass(); + on_connect_callback_ = callback; CHECK(frame_data_.size() > 0u); frame_tree_client_request_ = frame_tree_client_binding_.Unbind(); if (IsReady()) @@ -97,9 +114,7 @@ void DocumentResourceWaiter::OnPostMessageEvent( NOTREACHED(); } -void DocumentResourceWaiter::OnWillNavigate( - uint32_t target_frame_id, - const OnWillNavigateCallback& callback) { +void DocumentResourceWaiter::OnWillNavigate(uint32_t target_frame_id) { // It is assumed we receive OnConnect() (which unbinds) before anything else. NOTIMPLEMENTED(); } diff --git a/components/html_viewer/document_resource_waiter.h b/components/html_viewer/document_resource_waiter.h index a2312e4..a5e7824 100644 --- a/components/html_viewer/document_resource_waiter.h +++ b/components/html_viewer/document_resource_waiter.h @@ -35,7 +35,10 @@ class DocumentResourceWaiter : public mandoline::FrameTreeClient { frame_tree_client_request, mandoline::FrameTreeServerPtr* frame_tree_server, mojo::Array<mandoline::FrameDataPtr>* frame_data, - uint32_t* change_id); + uint32_t* view_id, + uint32_t* change_id, + mandoline::ViewConnectType* view_connect_type, + OnConnectCallback* on_connect_callback); mojo::URLResponsePtr ReleaseURLResponse(); @@ -51,7 +54,10 @@ class DocumentResourceWaiter : public mandoline::FrameTreeClient { // mandoline::FrameTreeClient: void OnConnect(mandoline::FrameTreeServerPtr server, uint32_t change_id, - mojo::Array<mandoline::FrameDataPtr> frame_data) override; + uint32_t view_id, + mandoline::ViewConnectType view_connect_type, + mojo::Array<mandoline::FrameDataPtr> frame_data, + const OnConnectCallback& callback) override; void OnFrameAdded(uint32_t change_id, mandoline::FrameDataPtr frame_data) override; void OnFrameRemoved(uint32_t change_id, uint32_t frame_id) override; @@ -61,8 +67,7 @@ class DocumentResourceWaiter : public mandoline::FrameTreeClient { void OnPostMessageEvent(uint32_t source_frame_id, uint32_t target_frame_id, mandoline::HTMLMessageEventPtr event) override; - void OnWillNavigate(uint32_t target_frame_id, - const OnWillNavigateCallback& callback) override; + void OnWillNavigate(uint32_t target_frame_id) override; GlobalState* global_state_; HTMLDocumentOOPIF* document_; @@ -71,6 +76,9 @@ class DocumentResourceWaiter : public mandoline::FrameTreeClient { mandoline::FrameTreeServerPtr server_; mojo::Array<mandoline::FrameDataPtr> frame_data_; uint32_t change_id_; + uint32_t view_id_; + mandoline::ViewConnectType view_connect_type_; + OnConnectCallback on_connect_callback_; // Once we get OnConnect() we unbind |frame_tree_client_binding_| and put it // here. diff --git a/components/html_viewer/html_document_oopif.cc b/components/html_viewer/html_document_oopif.cc index 0ecb553..c9d0333 100644 --- a/components/html_viewer/html_document_oopif.cc +++ b/components/html_viewer/html_document_oopif.cc @@ -45,6 +45,30 @@ bool IsTestInterfaceEnabled() { } // namespace +// A ViewTreeDelegate implementation that delegates to a (swappable) delegate. +// This is used when one HTMLDocumentOOPIF takes over for another delegate +// (OnSwap()). +class ViewTreeDelegateImpl : public mojo::ViewTreeDelegate { + public: + explicit ViewTreeDelegateImpl(mojo::ViewTreeDelegate* delegate) + : delegate_(delegate) {} + ~ViewTreeDelegateImpl() override {} + + void set_delegate(mojo::ViewTreeDelegate* delegate) { delegate_ = delegate; } + + private: + // ViewTreeDelegate: + void OnEmbed(mojo::View* root) override { delegate_->OnEmbed(root); } + void OnUnembed() override { delegate_->OnUnembed(); } + void OnConnectionLost(mojo::ViewTreeConnection* connection) override { + delegate_->OnConnectionLost(connection); + } + + mojo::ViewTreeDelegate* delegate_; + + DISALLOW_COPY_AND_ASSIGN(ViewTreeDelegateImpl); +}; + HTMLDocumentOOPIF::BeforeLoadCache::BeforeLoadCache() { } @@ -53,6 +77,20 @@ HTMLDocumentOOPIF::BeforeLoadCache::~BeforeLoadCache() { STLDeleteElements(&test_interface_requests); } +HTMLDocumentOOPIF::TransferableState::TransferableState() + : owns_view_tree_connection(false), root(nullptr) {} + +HTMLDocumentOOPIF::TransferableState::~TransferableState() {} + +void HTMLDocumentOOPIF::TransferableState::Move(TransferableState* other) { + owns_view_tree_connection = other->owns_view_tree_connection; + root = other->root; + view_tree_delegate_impl = other->view_tree_delegate_impl.Pass(); + + other->root = nullptr; + other->owns_view_tree_connection = false; +} + HTMLDocumentOOPIF::HTMLDocumentOOPIF(mojo::ApplicationImpl* html_document_app, mojo::ApplicationConnection* connection, mojo::URLResponsePtr response, @@ -66,8 +104,7 @@ HTMLDocumentOOPIF::HTMLDocumentOOPIF(mojo::ApplicationImpl* html_document_app, global_state_(global_state), frame_(nullptr), delete_callback_(delete_callback), - factory_(factory), - root_(nullptr) { + factory_(factory) { // TODO(sky): nuke headless. We're not going to care about it anymore. DCHECK(!global_state_->is_headless()); @@ -93,11 +130,19 @@ void HTMLDocumentOOPIF::Destroy() { } else { delete this; } - } else { + } else if (frame_) { // Closing the frame ends up destroying the ViewManager, which triggers - // deleting this (OnViewManagerDestroyed()). - if (frame_) - frame_->Close(); + // deleting this (OnConnectionLost()). + frame_->Close(); + } else if (transferable_state_.root) { + transferable_state_.root->RemoveObserver(this); + // This triggers deleting us. + if (transferable_state_.owns_view_tree_connection) + delete transferable_state_.root->connection(); + else + delete this; + } else { + delete this; } } @@ -115,10 +160,13 @@ void HTMLDocumentOOPIF::LoadIfNecessary() { void HTMLDocumentOOPIF::Load() { DCHECK(resource_waiter_ && resource_waiter_->IsReady()); + // Note: |view| is null if we're taking over for an existing frame. mojo::View* view = resource_waiter_->root(); - global_state_->InitIfNecessary( - view->viewport_metrics().size_in_pixels.To<gfx::Size>(), - view->viewport_metrics().device_pixel_ratio); + if (view) { + global_state_->InitIfNecessary( + view->viewport_metrics().size_in_pixels.To<gfx::Size>(), + view->viewport_metrics().device_pixel_ratio); + } scoped_ptr<WebURLRequestExtraData> extra_data(new WebURLRequestExtraData); extra_data->synthetic_response = @@ -127,15 +175,14 @@ void HTMLDocumentOOPIF::Load() { frame_ = HTMLFrameTreeManager::CreateFrameAndAttachToTree( global_state_, view, resource_waiter_.Pass(), this); - // If the frame wasn't created we can destroy the connection. + // If the frame wasn't created we can destroy ourself. if (!frame_) { - root_->RemoveObserver(this); - // This triggers deleting us. - delete root_->connection(); + Destroy(); return; } - view->RemoveObserver(this); + if (view) + view->RemoveObserver(this); if (devtools_agent_request_.is_pending()) { if (frame_->devtools_agent()) { @@ -164,9 +211,9 @@ HTMLDocumentOOPIF::BeforeLoadCache* HTMLDocumentOOPIF::GetBeforeLoadCache() { } void HTMLDocumentOOPIF::OnEmbed(View* root) { - root_ = root; + transferable_state_.root = root; - // We're an observer until the document is loaded. + // We're an observer until we start the load. root->AddObserver(this); resource_waiter_->set_root(root); @@ -219,8 +266,36 @@ HTMLFactory* HTMLDocumentOOPIF::GetHTMLFactory() { void HTMLDocumentOOPIF::OnFrameSwappedToRemote() { // When the frame becomes remote HTMLDocumentOOPIF is no longer needed. - // Deleting the ViewManager triggers deleting us. - delete root_->connection(); + frame_ = nullptr; + Destroy(); +} + +void HTMLDocumentOOPIF::OnSwap(HTMLFrame* frame, + HTMLFrameDelegate* old_delegate) { + DCHECK(frame->IsLocal()); + DCHECK(frame->view()); + DCHECK(!frame_); + DCHECK(!transferable_state_.root); + if (!old_delegate) { + // We're taking over a child of a local root that isn't associated with a + // delegate. In this case the frame's view is not the root of the + // ViewTreeConnection. + transferable_state_.owns_view_tree_connection = false; + transferable_state_.root = frame->view(); + } else { + HTMLDocumentOOPIF* old_document = + static_cast<HTMLDocumentOOPIF*>(old_delegate); + transferable_state_.Move(&old_document->transferable_state_); + if (transferable_state_.view_tree_delegate_impl) + transferable_state_.view_tree_delegate_impl->set_delegate(this); + old_document->frame_ = nullptr; + old_document->Destroy(); + } +} + +void HTMLDocumentOOPIF::OnFrameDestroyed() { + if (!transferable_state_.owns_view_tree_connection) + delete this; } void HTMLDocumentOOPIF::Create(mojo::ApplicationConnection* connection, @@ -273,7 +348,12 @@ void HTMLDocumentOOPIF::Create( void HTMLDocumentOOPIF::Create( mojo::ApplicationConnection* connection, mojo::InterfaceRequest<mojo::ViewTreeClient> request) { - mojo::ViewTreeConnection::Create(this, request.Pass()); + DCHECK(!transferable_state_.view_tree_delegate_impl); + transferable_state_.view_tree_delegate_impl.reset( + new ViewTreeDelegateImpl(this)); + transferable_state_.owns_view_tree_connection = true; + mojo::ViewTreeConnection::Create( + transferable_state_.view_tree_delegate_impl.get(), request.Pass()); } } // namespace html_viewer diff --git a/components/html_viewer/html_document_oopif.h b/components/html_viewer/html_document_oopif.h index 6495a23..11e863f 100644 --- a/components/html_viewer/html_document_oopif.h +++ b/components/html_viewer/html_document_oopif.h @@ -42,6 +42,7 @@ class HTMLFactory; class HTMLFrame; class HTMLFrameTreeManager; class TestHTMLViewerImpl; +class ViewTreeDelegateImpl; class WebLayerTreeViewImpl; // A view for a single HTML document. @@ -88,6 +89,20 @@ class HTMLDocumentOOPIF std::set<mojo::InterfaceRequest<TestHTMLViewer>*> test_interface_requests; }; + // Any state that needs to be moved when rendering transfers from one frame + // to another is stored here. + struct TransferableState { + TransferableState(); + ~TransferableState(); + + // Takes the state from |other|. + void Move(TransferableState* other); + + bool owns_view_tree_connection; + mojo::View* root; + scoped_ptr<ViewTreeDelegateImpl> view_tree_delegate_impl; + }; + ~HTMLDocumentOOPIF() override; void LoadIfNecessary(); @@ -107,10 +122,12 @@ class HTMLDocumentOOPIF void OnViewDestroyed(mojo::View* view) override; // HTMLFrameDelegate: - void OnFrameDidFinishLoad() override; mojo::ApplicationImpl* GetApp() override; HTMLFactory* GetHTMLFactory() override; + void OnFrameDidFinishLoad() override; void OnFrameSwappedToRemote() override; + void OnSwap(HTMLFrame* frame, HTMLFrameDelegate* old_delegate) override; + void OnFrameDestroyed() override; // mojo::InterfaceFactory<mojo::AxProvider>: void Create(mojo::ApplicationConnection* connection, @@ -158,7 +175,7 @@ class HTMLDocumentOOPIF HTMLFactory* factory_; - mojo::View* root_; + TransferableState transferable_state_; // Cache interface request of DevToolsAgent if |frame_| hasn't been // initialized. diff --git a/components/html_viewer/html_frame.cc b/components/html_viewer/html_frame.cc index bffc07b..bd11d33 100644 --- a/components/html_viewer/html_frame.cc +++ b/components/html_viewer/html_frame.cc @@ -239,6 +239,10 @@ blink::WebWidget* HTMLFrame::GetWebWidget() { return html_widget_ ? html_widget_->GetWidget() : nullptr; } +bool HTMLFrame::IsLocal() const { + return web_frame_->isWebLocalFrame(); +} + bool HTMLFrame::HasLocalDescendant() const { if (IsLocal()) return true; @@ -262,6 +266,9 @@ HTMLFrame::~HTMLFrame() { frame_tree_manager_->OnFrameDestroyed(this); + if (delegate_) + delegate_->OnFrameDestroyed(); + if (view_) { view_->RemoveObserver(this); mojo::ScopedViewPtr::DeleteViewOrViewManager(view_); @@ -294,10 +301,6 @@ void HTMLFrame::SetValueFromClientProperty(const std::string& name, } } -bool HTMLFrame::IsLocal() const { - return web_frame_->isWebLocalFrame(); -} - HTMLFrame* HTMLFrame::GetLocalRoot() { HTMLFrame* frame = this; while (frame && !frame->delegate_) @@ -406,6 +409,13 @@ void HTMLFrame::SwapToLocal( web_layer_.reset(); } +void HTMLFrame::SwapDelegate(HTMLFrameDelegate* delegate) { + DCHECK(IsLocal()); + HTMLFrameDelegate* old_delegate = delegate_; + delegate_ = delegate; + delegate->OnSwap(this, old_delegate); +} + HTMLFrame* HTMLFrame::FindFrameWithWebFrame(blink::WebFrame* web_frame) { if (web_frame_ == web_frame) return this; @@ -486,7 +496,10 @@ void HTMLFrame::OnViewFocusChanged(mojo::View* gained_focus, void HTMLFrame::OnConnect(mandoline::FrameTreeServerPtr server, uint32_t change_id, - mojo::Array<mandoline::FrameDataPtr> frame_data) { + uint32_t view_id, + mandoline::ViewConnectType view_connect_type, + mojo::Array<mandoline::FrameDataPtr> frame_data, + const OnConnectCallback& callback) { // OnConnect() is only sent once, and has been received (by // DocumentResourceWaiter) by the time we get here. NOTREACHED(); @@ -557,17 +570,12 @@ void HTMLFrame::OnPostMessageEvent(uint32_t source_frame_id, msg_event); } -void HTMLFrame::OnWillNavigate(uint32_t target_frame_id, - const OnWillNavigateCallback& callback) { - // Assume this process won't service the connection and swap to remote. - // It's entirely possible this process will service the connection and we - // don't need to swap, but the naive approach is much simpler. +void HTMLFrame::OnWillNavigate(uint32_t target_frame_id) { HTMLFrame* target = frame_tree_manager_->root_->FindFrame(target_frame_id); if (target && target->IsLocal() && target != frame_tree_manager_->local_root_) { target->SwapToRemote(); } - callback.Run(); } blink::WebMediaPlayer* HTMLFrame::createMediaPlayer( diff --git a/components/html_viewer/html_frame.h b/components/html_viewer/html_frame.h index a3ca028..eaa30af 100644 --- a/components/html_viewer/html_frame.h +++ b/components/html_viewer/html_frame.h @@ -136,6 +136,9 @@ class HTMLFrame : public blink::WebFrameClient, // frame. DevToolsAgentImpl* devtools_agent() { return devtools_agent_.get(); } + // Returns true if the Frame is local, false if remote. + bool IsLocal() const; + // Returns true if this or one of the frames descendants is local. bool HasLocalDescendant() const; @@ -197,9 +200,6 @@ class HTMLFrame : public blink::WebFrameClient, void SetValueFromClientProperty(const std::string& name, mojo::Array<uint8_t> new_data); - // Returns true if the Frame is local, false if remote. - bool IsLocal() const; - // The local root is the first ancestor (starting at this) that has its own // connection. HTMLFrame* GetLocalRoot(); @@ -228,6 +228,11 @@ class HTMLFrame : public blink::WebFrameClient, mojo::View* view, const mojo::Map<mojo::String, mojo::Array<uint8_t>>& properties); + // Invoked when changing the delegate. This informs the new delegate to take + // over. This is used when a different connection is going to take over + // responsibility for the frame. + void SwapDelegate(HTMLFrameDelegate* delegate); + GlobalState* global_state() { return frame_tree_manager_->global_state(); } // Returns the Frame associated with the specified WebFrame. @@ -248,7 +253,10 @@ class HTMLFrame : public blink::WebFrameClient, // mandoline::FrameTreeClient: void OnConnect(mandoline::FrameTreeServerPtr server, uint32_t change_id, - mojo::Array<mandoline::FrameDataPtr> frame_data) override; + uint32_t view_id, + mandoline::ViewConnectType view_connect_type, + mojo::Array<mandoline::FrameDataPtr> frame_data, + const OnConnectCallback& callback) override; void OnFrameAdded(uint32_t change_id, mandoline::FrameDataPtr frame_data) override; void OnFrameRemoved(uint32_t change_id, uint32_t frame_id) override; @@ -259,8 +267,7 @@ class HTMLFrame : public blink::WebFrameClient, uint32_t source_frame_id, uint32_t target_frame_id, mandoline::HTMLMessageEventPtr serialized_event) override; - void OnWillNavigate(uint32_t target_frame_id, - const OnWillNavigateCallback& callback) override; + void OnWillNavigate(uint32_t target_frame_id) override; // blink::WebRemoteFrameClient: virtual void frameDetached(blink::WebRemoteFrameClient::DetachType type); diff --git a/components/html_viewer/html_frame_apptest.cc b/components/html_viewer/html_frame_apptest.cc index 13381b0..f98c250 100644 --- a/components/html_viewer/html_frame_apptest.cc +++ b/components/html_viewer/html_frame_apptest.cc @@ -4,6 +4,7 @@ #include "base/auto_reset.h" #include "base/bind.h" +#include "base/callback.h" #include "base/command_line.h" #include "base/json/json_reader.h" #include "base/run_loop.h" @@ -27,6 +28,7 @@ using mandoline::Frame; using mandoline::FrameConnection; using mandoline::FrameTree; using mandoline::FrameTreeClient; +using mandoline::FrameTreeDelegate; namespace mojo { @@ -37,6 +39,11 @@ const char kAddFrameWithEmptyPageScript[] = "iframe.src = \"http://127.0.0.1:%u/files/empty_page.html\";" "document.body.appendChild(iframe);"; +void OnGotContentHandlerForRoot(bool* got_callback) { + *got_callback = true; + ignore_result(ViewManagerTestBase::QuitRunLoop()); +} + mojo::ApplicationConnection* ApplicationConnectionForFrame(Frame* frame) { return static_cast<FrameConnection*>(frame->user_data()) ->application_connection(); @@ -101,18 +108,11 @@ class TestFrameTreeDelegateImpl : public mandoline::TestFrameTreeDelegate { } // TestFrameTreeDelegate: - bool CanNavigateFrame( - Frame* target, - mojo::URLRequestPtr request, - mandoline::FrameTreeClient** frame_tree_client, - scoped_ptr<mandoline::FrameUserData>* frame_user_data, - mojo::ViewTreeClientPtr* tree_client) override { - scoped_ptr<FrameConnection> frame_connection(new FrameConnection); - frame_connection->Init(app_, request.Pass(), tree_client); - *frame_tree_client = frame_connection->frame_tree_client(); - *frame_user_data = frame_connection.Pass(); - - return true; + void CanNavigateFrame(Frame* target, + mojo::URLRequestPtr request, + const CanNavigateFrameCallback& callback) override { + FrameConnection::CreateConnectionForCanNavigateFrame( + app_, target, request.Pass(), callback); } void DidStartNavigation(Frame* frame) override { @@ -147,6 +147,10 @@ class HTMLFrameTest : public ViewManagerTestBase { new TestFrameTreeDelegateImpl(application_impl())); FrameConnection* root_connection = InitFrameTree(embed_view, "http://127.0.0.1:%u/files/empty_page2.html"); + if (!root_connection) { + ADD_FAILURE() << "unable to establish root connection"; + return nullptr; + } const std::string frame_text = GetFrameText(root_connection->application_connection()); if (frame_text != "child2") { @@ -195,14 +199,19 @@ class HTMLFrameTest : public ViewManagerTestBase { frame_tree_delegate_.reset( new TestFrameTreeDelegateImpl(application_impl())); scoped_ptr<FrameConnection> frame_connection(new FrameConnection); + bool got_callback = false; + frame_connection->Init( + application_impl(), BuildRequestForURL(url_string), + base::Bind(&OnGotContentHandlerForRoot, &got_callback)); + ignore_result(ViewManagerTestBase::DoRunLoopWithTimeout()); + if (!got_callback) + return nullptr; FrameConnection* result = frame_connection.get(); - ViewTreeClientPtr tree_client; - frame_connection->Init(application_impl(), BuildRequestForURL(url_string), - &tree_client); FrameTreeClient* frame_tree_client = frame_connection->frame_tree_client(); - frame_tree_.reset(new FrameTree(view, frame_tree_delegate_.get(), - frame_tree_client, - frame_connection.Pass(), + ViewTreeClientPtr tree_client = frame_connection->GetViewTreeClient(); + frame_tree_.reset(new FrameTree(result->GetContentHandlerID(), view, + frame_tree_delegate_.get(), + frame_tree_client, frame_connection.Pass(), Frame::ClientPropertyMap())); frame_tree_delegate_->set_frame_tree(frame_tree_.get()); view->Embed(tree_client.Pass()); @@ -250,6 +259,7 @@ TEST_F(HTMLFrameTest, PageWithSingleFrame) { FrameConnection* root_connection = InitFrameTree( embed_view, "http://127.0.0.1:%u/files/page_with_single_frame.html"); + ASSERT_TRUE(root_connection); ASSERT_EQ("Page with single frame", GetFrameText(root_connection->application_connection())); @@ -276,8 +286,8 @@ TEST_F(HTMLFrameTest, PageWithSingleFrame) { TEST_F(HTMLFrameTest, ChangeLocationOfChildFrame) { View* embed_view = window_manager()->CreateView(); - InitFrameTree(embed_view, - "http://127.0.0.1:%u/files/page_with_single_frame.html"); + ASSERT_TRUE(InitFrameTree( + embed_view, "http://127.0.0.1:%u/files/page_with_single_frame.html")); // page_with_single_frame contains a child frame. The child frame should // create a new View and Frame. diff --git a/components/html_viewer/html_frame_delegate.h b/components/html_viewer/html_frame_delegate.h index 12ad38f..9418aa6 100644 --- a/components/html_viewer/html_frame_delegate.h +++ b/components/html_viewer/html_frame_delegate.h @@ -12,23 +12,32 @@ class ApplicationImpl; namespace html_viewer { class HTMLFactory; +class HTMLFrame; class HTMLFrameDelegate { public: + // Returns the ApplicationImpl for the frame. + virtual mojo::ApplicationImpl* GetApp() = 0; + + // Returns the factory for creating various classes. + virtual HTMLFactory* GetHTMLFactory() = 0; + // Invoked when the Frame the delegate is attached to finishes loading. This // is not invoked for any child frames, only the frame returned from // HTMLFrameTreeManager::CreateFrameAndAttachToTree(). virtual void OnFrameDidFinishLoad() = 0; - // Returns the ApplicationImpl for the frame. - virtual mojo::ApplicationImpl* GetApp() = 0; - // Invoked when the HTMLFrame the delegate is associated with is swapped // to a remote frame. virtual void OnFrameSwappedToRemote() = 0; - // Returns the factory for creating various classes. - virtual HTMLFactory* GetHTMLFactory() = 0; + // Invoked when this delegate is to take over from |old_delegate| (which may + // be null). This is invoked when a new connection is established and we + // are already rendering to the supplied frame. + virtual void OnSwap(HTMLFrame* frame, HTMLFrameDelegate* old_delegate) = 0; + + // Invoked when the HTMLFrame is destroyed. + virtual void OnFrameDestroyed() = 0; protected: virtual ~HTMLFrameDelegate() {} diff --git a/components/html_viewer/html_frame_tree_manager.cc b/components/html_viewer/html_frame_tree_manager.cc index 93bbfcb..e2c64c3 100644 --- a/components/html_viewer/html_frame_tree_manager.cc +++ b/components/html_viewer/html_frame_tree_manager.cc @@ -15,8 +15,8 @@ #include "components/html_viewer/html_factory.h" #include "components/html_viewer/html_frame.h" #include "components/html_viewer/html_frame_delegate.h" -#include "components/html_viewer/html_viewer_switches.h" #include "components/view_manager/public/cpp/view_tree_connection.h" +#include "mandoline/tab/web_view_switches.h" #include "third_party/WebKit/public/web/WebLocalFrame.h" #include "third_party/WebKit/public/web/WebRemoteFrame.h" #include "third_party/WebKit/public/web/WebTreeScopeType.h" @@ -59,16 +59,28 @@ HTMLFrame* HTMLFrameTreeManager::CreateFrameAndAttachToTree( mandoline::FrameTreeServerPtr frame_tree_server; mojo::Array<mandoline::FrameDataPtr> frame_data; uint32_t change_id; + uint32_t view_id; + mandoline::ViewConnectType view_connect_type; + mandoline::FrameTreeClient::OnConnectCallback on_connect_callback; resource_waiter->Release(&frame_tree_client_request, &frame_tree_server, - &frame_data, &change_id); + &frame_data, &change_id, &view_id, + &view_connect_type, &on_connect_callback); resource_waiter.reset(); + on_connect_callback.Run(); + HTMLFrameTreeManager* frame_tree = nullptr; if (!base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kOOPIFAlwaysCreateNewFrameTree)) { - if (instances_->count(frame_data[0]->frame_id)) - frame_tree = (*instances_)[frame_data[0]->frame_id]; + web_view::switches::kOOPIFAlwaysCreateNewFrameTree) && + instances_->count(frame_data[0]->frame_id)) { + frame_tree = (*instances_)[frame_data[0]->frame_id]; + } + + if (view_connect_type == mandoline::VIEW_CONNECT_TYPE_USE_EXISTING && + !frame_tree) { + DVLOG(1) << "was told to use existing view but do not have frame tree"; + return nullptr; } if (!frame_tree) { @@ -76,6 +88,17 @@ HTMLFrame* HTMLFrameTreeManager::CreateFrameAndAttachToTree( frame_tree->Init(delegate, view, frame_data, change_id); if (frame_data[0]->frame_id == view->id()) (*instances_)[frame_data[0]->frame_id] = frame_tree; + } else if (view_connect_type == mandoline::VIEW_CONNECT_TYPE_USE_EXISTING) { + HTMLFrame* existing_frame = frame_tree->root_->FindFrame(view_id); + if (!existing_frame) { + DVLOG(1) << "was told to use existing view but could not find view"; + return nullptr; + } + if (!existing_frame->IsLocal()) { + DVLOG(1) << "was told to use existing view, but frame is remote"; + return nullptr; + } + existing_frame->SwapDelegate(delegate); } else { // We're going to share a frame tree. There are two possibilities: // . We already know about the frame, in which case we swap it to local. @@ -111,7 +134,7 @@ HTMLFrame* HTMLFrameTreeManager::CreateFrameAndAttachToTree( } } - HTMLFrame* frame = frame_tree->root_->FindFrame(view->id()); + HTMLFrame* frame = frame_tree->root_->FindFrame(view_id); DCHECK(frame); frame->Bind(frame_tree_server.Pass(), frame_tree_client_request.Pass()); return frame; diff --git a/components/html_viewer/html_viewer_switches.cc b/components/html_viewer/html_viewer_switches.cc deleted file mode 100644 index 80c1020..0000000 --- a/components/html_viewer/html_viewer_switches.cc +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2015 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 "components/html_viewer/html_viewer_switches.h" - -namespace html_viewer { -namespace switches { - -const char kOOPIFAlwaysCreateNewFrameTree[] = - "oopif-always-create-new-frame-tree"; - -} // namespace switches -} // namespace html_viewer diff --git a/components/html_viewer/html_viewer_switches.h b/components/html_viewer/html_viewer_switches.h deleted file mode 100644 index 23214d8..0000000 --- a/components/html_viewer/html_viewer_switches.h +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright 2015 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 COMPONENTS_HTML_VIEWER_HTML_VIEWER_SWITCHES_H_ -#define COMPONENTS_HTML_VIEWER_HTML_VIEWER_SWITCHES_H_ - -namespace html_viewer { -namespace switches { - -// If true a new HTMLFrameTreeManager is always created, even if a matching -// HTMLFrameTreeManager is found. This is useful for tests (or debugging) that -// want to synthesize what happens with multi-processes in a single process. -extern const char kOOPIFAlwaysCreateNewFrameTree[]; - -} // namespace switches -} // namespace html_viewer; - -#endif // COMPONENTS_HTML_VIEWER_HTML_VIEWER_SWITCHES_H_ |