diff options
author | haibinlu <haibinlu@chromium.org> | 2016-01-11 15:51:48 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-01-11 23:53:21 +0000 |
commit | 6bd6e37171662b6cbf1ff628c30688a6d4965ece (patch) | |
tree | 3fbeb24e9fdec7834a691331e98c26c74b2c9475 /blimp | |
parent | f97f99e6e4f67c134638f4b933718c29a3a70118 (diff) | |
download | chromium_src-6bd6e37171662b6cbf1ff628c30688a6d4965ece.zip chromium_src-6bd6e37171662b6cbf1ff628c30688a6d4965ece.tar.gz chromium_src-6bd6e37171662b6cbf1ff628c30688a6d4965ece.tar.bz2 |
[Blimp] Always invoke callback after a feature finishes processing a message.
Network layer waits for the callback to be invoked with OK before it reads next message. If a feature does not invokes callback, it will block all features from receiving more messages.
BUG=576350
Review URL: https://codereview.chromium.org/1571413003
Cr-Commit-Position: refs/heads/master@{#368722}
Diffstat (limited to 'blimp')
-rw-r--r-- | blimp/client/session/navigation_feature.cc | 5 | ||||
-rw-r--r-- | blimp/client/session/navigation_feature_unittest.cc | 5 | ||||
-rw-r--r-- | blimp/client/session/render_widget_feature.cc | 10 | ||||
-rw-r--r-- | blimp/client/session/render_widget_feature_unittest.cc | 9 | ||||
-rw-r--r-- | blimp/client/session/tab_control_feature.cc | 4 | ||||
-rw-r--r-- | blimp/engine/browser/blimp_engine_session.cc | 15 | ||||
-rw-r--r-- | blimp/engine/browser/engine_render_widget_feature.cc | 5 | ||||
-rw-r--r-- | blimp/engine/browser/engine_render_widget_feature_unittest.cc | 11 | ||||
-rw-r--r-- | blimp/net/blimp_message_demultiplexer.cc | 1 | ||||
-rw-r--r-- | blimp/net/blimp_message_pump.cc | 1 |
10 files changed, 42 insertions, 24 deletions
diff --git a/blimp/client/session/navigation_feature.cc b/blimp/client/session/navigation_feature.cc index c02931a..86b0a09 100644 --- a/blimp/client/session/navigation_feature.cc +++ b/blimp/client/session/navigation_feature.cc @@ -78,6 +78,7 @@ void NavigationFeature::GoBack(int tab_id) { void NavigationFeature::ProcessMessage( scoped_ptr<BlimpMessage> message, const net::CompletionCallback& callback) { + DCHECK(!callback.is_null()); DCHECK(message->type() == BlimpMessage::NAVIGATION); int tab_id = message->target_tab_id(); @@ -85,6 +86,7 @@ void NavigationFeature::ProcessMessage( const NavigationMessage& navigation_message = message->navigation(); NavigationFeatureDelegate* delegate = FindDelegate(tab_id); + DCHECK(delegate) << "NavigationFeatureDelegate not found for tab " << tab_id; switch (navigation_message.type()) { case NavigationMessage::NAVIGATION_STATE_CHANGED: { const NavigationStateChangeMessage& details = @@ -112,8 +114,7 @@ void NavigationFeature::ProcessMessage( NOTREACHED(); } - if (!callback.is_null()) - callback.Run(net::OK); + callback.Run(net::OK); } NavigationFeature::NavigationFeatureDelegate* NavigationFeature::FindDelegate( diff --git a/blimp/client/session/navigation_feature_unittest.cc b/blimp/client/session/navigation_feature_unittest.cc index fb154e66..138924e 100644 --- a/blimp/client/session/navigation_feature_unittest.cc +++ b/blimp/client/session/navigation_feature_unittest.cc @@ -10,6 +10,7 @@ #include "blimp/common/proto/blimp_message.pb.h" #include "blimp/net/test_common.h" #include "net/base/net_errors.h" +#include "net/base/test_completion_callback.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/skia/include/core/SkBitmap.h" @@ -50,7 +51,9 @@ void SendMockNavigationStateChangedMessage(BlimpMessageProcessor* processor, if (loading) state->set_loading(*loading); - processor->ProcessMessage(std::move(message), net::CompletionCallback()); + net::TestCompletionCallback cb; + processor->ProcessMessage(std::move(message), cb.callback()); + EXPECT_EQ(net::OK, cb.WaitForResult()); } MATCHER_P2(EqualsNavigateToUrlText, tab_id, text, "") { diff --git a/blimp/client/session/render_widget_feature.cc b/blimp/client/session/render_widget_feature.cc index c75372d..fddbe47 100644 --- a/blimp/client/session/render_widget_feature.cc +++ b/blimp/client/session/render_widget_feature.cc @@ -82,14 +82,14 @@ void RenderWidgetFeature::RemoveDelegate(const int tab_id) { void RenderWidgetFeature::ProcessMessage( scoped_ptr<BlimpMessage> message, const net::CompletionCallback& callback) { + DCHECK(!callback.is_null()); DCHECK(message->type() == BlimpMessage::RENDER_WIDGET || message->type() == BlimpMessage::COMPOSITOR); int target_tab_id = message->target_tab_id(); RenderWidgetFeatureDelegate* delegate = FindDelegate(target_tab_id); - DCHECK(delegate); - if (!delegate) - return; + DCHECK(delegate) << "RenderWidgetFeatureDelegate not found for " + << target_tab_id; switch (message->type()) { case BlimpMessage::RENDER_WIDGET: @@ -112,9 +112,7 @@ void RenderWidgetFeature::ProcessMessage( NOTIMPLEMENTED(); } - if (!callback.is_null()) { - callback.Run(net::OK); - } + callback.Run(net::OK); } RenderWidgetFeature::RenderWidgetFeatureDelegate* diff --git a/blimp/client/session/render_widget_feature_unittest.cc b/blimp/client/session/render_widget_feature_unittest.cc index 9629fde..49171c6 100644 --- a/blimp/client/session/render_widget_feature_unittest.cc +++ b/blimp/client/session/render_widget_feature_unittest.cc @@ -13,6 +13,7 @@ #include "blimp/net/test_common.h" #include "cc/proto/compositor_message.pb.h" #include "net/base/net_errors.h" +#include "net/base/test_completion_callback.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -54,7 +55,9 @@ void SendRenderWidgetMessage(BlimpMessageProcessor* processor, scoped_ptr<BlimpMessage> message = CreateBlimpMessage(&details, tab_id); details->set_type(RenderWidgetMessage::INITIALIZE); details->set_render_widget_id(rw_id); - processor->ProcessMessage(std::move(message), net::CompletionCallback()); + net::TestCompletionCallback cb; + processor->ProcessMessage(std::move(message), cb.callback()); + EXPECT_EQ(net::OK, cb.WaitForResult()); } void SendCompositorMessage(BlimpMessageProcessor* processor, @@ -63,7 +66,9 @@ void SendCompositorMessage(BlimpMessageProcessor* processor, CompositorMessage* details; scoped_ptr<BlimpMessage> message = CreateBlimpMessage(&details, tab_id); details->set_render_widget_id(rw_id); - processor->ProcessMessage(std::move(message), net::CompletionCallback()); + net::TestCompletionCallback cb; + processor->ProcessMessage(std::move(message), cb.callback()); + EXPECT_EQ(net::OK, cb.WaitForResult()); } } // namespace diff --git a/blimp/client/session/tab_control_feature.cc b/blimp/client/session/tab_control_feature.cc index 33c4524..b7b23cd 100644 --- a/blimp/client/session/tab_control_feature.cc +++ b/blimp/client/session/tab_control_feature.cc @@ -8,6 +8,7 @@ #include "blimp/common/proto/blimp_message.pb.h" #include "blimp/common/proto/tab_control.pb.h" #include "blimp/net/blimp_message_processor.h" +#include "net/base/net_errors.h" namespace blimp { namespace client { @@ -61,6 +62,9 @@ void TabControlFeature::CloseTab(int tab_id) { void TabControlFeature::ProcessMessage( scoped_ptr<BlimpMessage> message, const net::CompletionCallback& callback) { + DCHECK(!callback.is_null()); + callback.Run(net::OK); + NOTIMPLEMENTED(); } diff --git a/blimp/engine/browser/blimp_engine_session.cc b/blimp/engine/browser/blimp_engine_session.cc index 6e5f794..d541e89 100644 --- a/blimp/engine/browser/blimp_engine_session.cc +++ b/blimp/engine/browser/blimp_engine_session.cc @@ -353,14 +353,16 @@ void BlimpEngineSession::OnCompositorMessageReceived( void BlimpEngineSession::ProcessMessage( scoped_ptr<BlimpMessage> message, const net::CompletionCallback& callback) { + DCHECK(!callback.is_null()); DCHECK(message->type() == BlimpMessage::TAB_CONTROL || message->type() == BlimpMessage::NAVIGATION); - bool result = true; + net::Error result = net::OK; if (message->type() == BlimpMessage::TAB_CONTROL) { switch (message->tab_control().type()) { case TabControlMessage::CREATE_TAB: - result = CreateWebContents(message->target_tab_id()); + if (!CreateWebContents(message->target_tab_id())) + result = net::ERR_FAILED; break; case TabControlMessage::CLOSE_TAB: CloseWebContents(message->target_tab_id()); @@ -371,6 +373,7 @@ void BlimpEngineSession::ProcessMessage( break; default: NOTIMPLEMENTED(); + result = net::ERR_NOT_IMPLEMENTED; } } else if (message->type() == BlimpMessage::NAVIGATION && web_contents_) { switch (message->navigation().type()) { @@ -389,14 +392,14 @@ void BlimpEngineSession::ProcessMessage( break; default: NOTIMPLEMENTED(); + result = net::ERR_NOT_IMPLEMENTED; } } else { - result = false; + DVLOG(1) << "No WebContents for navigation control"; + result = net::ERR_FAILED; } - if (!callback.is_null()) { - callback.Run(result ? net::OK : net::ERR_FAILED); - } + callback.Run(result); } void BlimpEngineSession::AddNewContents(content::WebContents* source, diff --git a/blimp/engine/browser/engine_render_widget_feature.cc b/blimp/engine/browser/engine_render_widget_feature.cc index fe68ae6..35bead3 100644 --- a/blimp/engine/browser/engine_render_widget_feature.cc +++ b/blimp/engine/browser/engine_render_widget_feature.cc @@ -85,6 +85,7 @@ void EngineRenderWidgetFeature::RemoveDelegate(const int tab_id) { void EngineRenderWidgetFeature::ProcessMessage( scoped_ptr<BlimpMessage> message, const net::CompletionCallback& callback) { + DCHECK(!callback.is_null()); DCHECK(message->type() == BlimpMessage::RENDER_WIDGET || message->type() == BlimpMessage::INPUT || message->type() == BlimpMessage::COMPOSITOR); @@ -118,9 +119,7 @@ void EngineRenderWidgetFeature::ProcessMessage( NOTREACHED(); } - if (!callback.is_null()) { - callback.Run(net::OK); - } + callback.Run(net::OK); } EngineRenderWidgetFeature::RenderWidgetMessageDelegate* diff --git a/blimp/engine/browser/engine_render_widget_feature_unittest.cc b/blimp/engine/browser/engine_render_widget_feature_unittest.cc index 9eba4c8..6433d8a 100644 --- a/blimp/engine/browser/engine_render_widget_feature_unittest.cc +++ b/blimp/engine/browser/engine_render_widget_feature_unittest.cc @@ -14,6 +14,7 @@ #include "blimp/net/input_message_generator.h" #include "blimp/net/test_common.h" #include "net/base/net_errors.h" +#include "net/base/test_completion_callback.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/WebKit/public/web/WebInputEvent.h" @@ -82,8 +83,9 @@ void SendInputMessage(BlimpMessageProcessor* processor, message->set_target_tab_id(tab_id); message->mutable_input()->set_render_widget_id(rw_id); - processor->ProcessMessage(std::move(message), - net::CompletionCallback()); + net::TestCompletionCallback cb; + processor->ProcessMessage(std::move(message), cb.callback()); + EXPECT_EQ(net::OK, cb.WaitForResult()); } void SendCompositorMessage(BlimpMessageProcessor* processor, @@ -94,8 +96,9 @@ void SendCompositorMessage(BlimpMessageProcessor* processor, scoped_ptr<BlimpMessage> message = CreateBlimpMessage(&details, tab_id); details->set_render_widget_id(rw_id); details->set_payload(payload.data(), base::checked_cast<int>(payload.size())); - processor->ProcessMessage(std::move(message), - net::CompletionCallback()); + net::TestCompletionCallback cb; + processor->ProcessMessage(std::move(message), cb.callback()); + EXPECT_EQ(net::OK, cb.WaitForResult()); } } // namespace diff --git a/blimp/net/blimp_message_demultiplexer.cc b/blimp/net/blimp_message_demultiplexer.cc index 1444c9fd..8c69702 100644 --- a/blimp/net/blimp_message_demultiplexer.cc +++ b/blimp/net/blimp_message_demultiplexer.cc @@ -29,6 +29,7 @@ void BlimpMessageDemultiplexer::AddProcessor(BlimpMessage::Type type, void BlimpMessageDemultiplexer::ProcessMessage( scoped_ptr<BlimpMessage> message, const net::CompletionCallback& callback) { + DVLOG(2) << "ProcessMessage : " << *message; auto receiver_iter = feature_receiver_map_.find(message->type()); if (receiver_iter == feature_receiver_map_.end()) { DLOG(ERROR) << "No registered receiver for " << *message << "."; diff --git a/blimp/net/blimp_message_pump.cc b/blimp/net/blimp_message_pump.cc index ae264e1..c3d6380 100644 --- a/blimp/net/blimp_message_pump.cc +++ b/blimp/net/blimp_message_pump.cc @@ -59,6 +59,7 @@ void BlimpMessagePump::OnReadPacketComplete(int result) { if (result == net::OK) { scoped_ptr<BlimpMessage> message(new BlimpMessage); if (message->ParseFromArray(buffer_->StartOfBuffer(), buffer_->offset())) { + DVLOG(2) << "OnReadPacketComplete, result=" << *message; processor_->ProcessMessage(std::move(message), process_msg_callback_.callback()); } else { |