Commit d464f5a8 authored by nyquist's avatar nyquist Committed by Commit bot

[blimp] Add support for having multiple tabs

There is still only a single Blimp tab, and the logic is for now very
simple: If there is already a blimp tab available, a new tab will be
WebContents based. If there are no blimp tabs, a blimp-based tab will
be created.

Also, when tabs are closed, it takes a while before the tab is really
destroyed, in case the user wants to undo the action. This feature
stays, but if the user selects "New Tab" or clicks the new tab button,
all the pending closures are first committed, ensuring that the tab
to be created might become a blimp tab, even if the current blimp tab
was pending closure.

Before this CL, the value 0 was always used fro the tab ID. This meant
that there would be a mixup between tabs that are closed and new tabs
that are created. This CL changes this so that each BlimpContentsImpl
gets its own unique ID, created client-side, and transferred to the
engine to be used for all communication regarding that tab, by an
update to the TabControlFeature to now include the ID.

The NavigationFeature is also updated to support receiving messages
after BlimpContentsImpl has been destroyed, which may happen, and
it now just logs those errors.

To ensure that this is visible in the tab switcher as well, a
workaround is added to ensure that the theme color is always shown,
even when on the new tab page. This is important in case where there
are many new tab pages, and one can not see which one is the blimp
tab, since the theme color is only updated when the tab navigates
away from the NTP.

To support the multiple IDs on the engine side, the Tab class is now
the implementor of the
EngineRenderWidgetFeature::RenderWidgetMessageDelegate instead of the
BlimpEngineSession. IME and compositor protos use the ID of the current
tab for their messages.

BUG=644467, 644774

Review-Url: https://codereview.chromium.org/2325893002
Cr-Commit-Position: refs/heads/master@{#417518}
parent 0fd51b5d
...@@ -19,6 +19,10 @@ ...@@ -19,6 +19,10 @@
using base::android::JavaParamRef; using base::android::JavaParamRef;
namespace {
const int kDummyBlimpContentsId = 0;
} // namespace
namespace blimp { namespace blimp {
namespace client { namespace client {
namespace app { namespace app {
...@@ -68,7 +72,8 @@ BlimpView::BlimpView(JNIEnv* env, ...@@ -68,7 +72,8 @@ BlimpView::BlimpView(JNIEnv* env,
&BlimpView::OnSwapBuffersCompleted, weak_ptr_factory_.GetWeakPtr())); &BlimpView::OnSwapBuffersCompleted, weak_ptr_factory_.GetWeakPtr()));
compositor_manager_ = base::MakeUnique<BlimpCompositorManager>( compositor_manager_ = base::MakeUnique<BlimpCompositorManager>(
render_widget_feature, compositor_dependencies_.get()); kDummyBlimpContentsId, render_widget_feature,
compositor_dependencies_.get());
compositor_->SetContentLayer(compositor_manager_->layer()); compositor_->SetContentLayer(compositor_manager_->layer());
java_obj_.Reset(env, jobj); java_obj_.Reset(env, jobj);
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
namespace blimp { namespace blimp {
namespace { namespace {
constexpr int kDummyBlimpContentsId = 0;
constexpr int kPointer1Id = 0; constexpr int kPointer1Id = 0;
constexpr int kPointer2Id = 1; constexpr int kPointer2Id = 1;
constexpr int kZoomOffsetMultiplier = 4; constexpr int kZoomOffsetMultiplier = 4;
...@@ -45,7 +46,8 @@ BlimpDisplayManager::BlimpDisplayManager( ...@@ -45,7 +46,8 @@ BlimpDisplayManager::BlimpDisplayManager(
compositor_->SetSize(platform_window_->GetBounds().size()); compositor_->SetSize(platform_window_->GetBounds().size());
compositor_manager_ = base::MakeUnique<BlimpCompositorManager>( compositor_manager_ = base::MakeUnique<BlimpCompositorManager>(
render_widget_feature, compositor_dependencies_.get()); kDummyBlimpContentsId, render_widget_feature,
compositor_dependencies_.get());
compositor_->SetContentLayer(compositor_manager_->layer()); compositor_->SetContentLayer(compositor_manager_->layer());
......
...@@ -11,14 +11,12 @@ ...@@ -11,14 +11,12 @@
namespace blimp { namespace blimp {
namespace client { namespace client {
namespace {
const int kDummyTabId = 0;
} // namespace
BlimpCompositorManager::BlimpCompositorManager( BlimpCompositorManager::BlimpCompositorManager(
int blimp_contents_id,
RenderWidgetFeature* render_widget_feature, RenderWidgetFeature* render_widget_feature,
BlimpCompositorDependencies* compositor_dependencies) BlimpCompositorDependencies* compositor_dependencies)
: render_widget_feature_(render_widget_feature), : blimp_contents_id_(blimp_contents_id),
render_widget_feature_(render_widget_feature),
visible_(false), visible_(false),
layer_(cc::Layer::Create()), layer_(cc::Layer::Create()),
active_compositor_(nullptr), active_compositor_(nullptr),
...@@ -26,11 +24,11 @@ BlimpCompositorManager::BlimpCompositorManager( ...@@ -26,11 +24,11 @@ BlimpCompositorManager::BlimpCompositorManager(
DCHECK(render_widget_feature_); DCHECK(render_widget_feature_);
DCHECK(compositor_dependencies_); DCHECK(compositor_dependencies_);
render_widget_feature_->SetDelegate(kDummyTabId, this); render_widget_feature_->SetDelegate(blimp_contents_id_, this);
} }
BlimpCompositorManager::~BlimpCompositorManager() { BlimpCompositorManager::~BlimpCompositorManager() {
render_widget_feature_->RemoveDelegate(kDummyTabId); render_widget_feature_->RemoveDelegate(blimp_contents_id_);
} }
void BlimpCompositorManager::SetVisible(bool visible) { void BlimpCompositorManager::SetVisible(bool visible) {
...@@ -106,15 +104,15 @@ void BlimpCompositorManager::OnCompositorMessageReceived( ...@@ -106,15 +104,15 @@ void BlimpCompositorManager::OnCompositorMessageReceived(
void BlimpCompositorManager::SendWebGestureEvent( void BlimpCompositorManager::SendWebGestureEvent(
int render_widget_id, int render_widget_id,
const blink::WebGestureEvent& gesture_event) { const blink::WebGestureEvent& gesture_event) {
render_widget_feature_->SendWebGestureEvent(kDummyTabId, render_widget_id, render_widget_feature_->SendWebGestureEvent(blimp_contents_id_,
gesture_event); render_widget_id, gesture_event);
} }
void BlimpCompositorManager::SendCompositorMessage( void BlimpCompositorManager::SendCompositorMessage(
int render_widget_id, int render_widget_id,
const cc::proto::CompositorMessage& message) { const cc::proto::CompositorMessage& message) {
render_widget_feature_->SendCompositorMessage(kDummyTabId, render_widget_id, render_widget_feature_->SendCompositorMessage(blimp_contents_id_,
message); render_widget_id, message);
} }
BlimpCompositor* BlimpCompositorManager::GetCompositor(int render_widget_id) { BlimpCompositor* BlimpCompositorManager::GetCompositor(int render_widget_id) {
......
...@@ -28,6 +28,7 @@ class BlimpCompositorManager ...@@ -28,6 +28,7 @@ class BlimpCompositorManager
public BlimpCompositorClient { public BlimpCompositorClient {
public: public:
explicit BlimpCompositorManager( explicit BlimpCompositorManager(
int blimp_contents_id,
RenderWidgetFeature* render_widget_feature, RenderWidgetFeature* render_widget_feature,
BlimpCompositorDependencies* compositor_dependencies); BlimpCompositorDependencies* compositor_dependencies);
~BlimpCompositorManager() override; ~BlimpCompositorManager() override;
...@@ -67,6 +68,8 @@ class BlimpCompositorManager ...@@ -67,6 +68,8 @@ class BlimpCompositorManager
int render_widget_id, int render_widget_id,
const cc::proto::CompositorMessage& message) override; const cc::proto::CompositorMessage& message) override;
int blimp_contents_id_;
// The bridge to the network layer that does the proto/RenderWidget id work. // The bridge to the network layer that does the proto/RenderWidget id work.
// BlimpCompositorManager does not own this and it is expected to outlive this // BlimpCompositorManager does not own this and it is expected to outlive this
// BlimpCompositorManager instance. // BlimpCompositorManager instance.
......
...@@ -23,6 +23,8 @@ namespace blimp { ...@@ -23,6 +23,8 @@ namespace blimp {
namespace client { namespace client {
namespace { namespace {
const int kDummyBlimpContentsId = 0;
class MockRenderWidgetFeature : public RenderWidgetFeature { class MockRenderWidgetFeature : public RenderWidgetFeature {
public: public:
MOCK_METHOD3(SendCompositorMessage, MOCK_METHOD3(SendCompositorMessage,
...@@ -55,10 +57,12 @@ class MockBlimpCompositor : public BlimpCompositor { ...@@ -55,10 +57,12 @@ class MockBlimpCompositor : public BlimpCompositor {
class BlimpCompositorManagerForTesting : public BlimpCompositorManager { class BlimpCompositorManagerForTesting : public BlimpCompositorManager {
public: public:
explicit BlimpCompositorManagerForTesting( explicit BlimpCompositorManagerForTesting(
int blimp_contents_id,
RenderWidgetFeature* render_widget_feature, RenderWidgetFeature* render_widget_feature,
BlimpCompositorDependencies* compositor_dependencies) BlimpCompositorDependencies* compositor_dependencies)
: BlimpCompositorManager(render_widget_feature, compositor_dependencies) { : BlimpCompositorManager(blimp_contents_id,
} render_widget_feature,
compositor_dependencies) {}
using BlimpCompositorManager::GetCompositor; using BlimpCompositorManager::GetCompositor;
...@@ -81,7 +85,8 @@ class BlimpCompositorManagerTest : public testing::Test { ...@@ -81,7 +85,8 @@ class BlimpCompositorManagerTest : public testing::Test {
base::MakeUnique<MockCompositorDependencies>()); base::MakeUnique<MockCompositorDependencies>());
compositor_manager_ = base::MakeUnique<BlimpCompositorManagerForTesting>( compositor_manager_ = base::MakeUnique<BlimpCompositorManagerForTesting>(
&render_widget_feature_, compositor_dependencies_.get()); kDummyBlimpContentsId, &render_widget_feature_,
compositor_dependencies_.get());
} }
void TearDown() override { void TearDown() override {
......
...@@ -33,8 +33,8 @@ BlimpContentsImpl::BlimpContentsImpl( ...@@ -33,8 +33,8 @@ BlimpContentsImpl::BlimpContentsImpl(
NavigationFeature* navigation_feature, NavigationFeature* navigation_feature,
RenderWidgetFeature* render_widget_feature, RenderWidgetFeature* render_widget_feature,
TabControlFeature* tab_control_feature) TabControlFeature* tab_control_feature)
: navigation_controller_(this, navigation_feature), : navigation_controller_(id, this, navigation_feature),
compositor_manager_(render_widget_feature, compositor_deps), compositor_manager_(id, render_widget_feature, compositor_deps),
id_(id), id_(id),
ime_feature_(ime_feature), ime_feature_(ime_feature),
window_(window), window_(window),
......
...@@ -27,7 +27,7 @@ namespace { ...@@ -27,7 +27,7 @@ namespace {
const char kExampleURL[] = "https://www.example.com/"; const char kExampleURL[] = "https://www.example.com/";
const char kOtherExampleURL[] = "https://www.otherexample.com/"; const char kOtherExampleURL[] = "https://www.otherexample.com/";
const int kDummyTabId = 0; const int kDummyBlimpContentsId = 0;
class MockBlimpContentsObserver : public BlimpContentsObserver { class MockBlimpContentsObserver : public BlimpContentsObserver {
public: public:
...@@ -75,9 +75,9 @@ TEST_F(BlimpContentsImplTest, LoadURLAndNotifyObservers) { ...@@ -75,9 +75,9 @@ TEST_F(BlimpContentsImplTest, LoadURLAndNotifyObservers) {
RenderWidgetFeature render_widget_feature; RenderWidgetFeature render_widget_feature;
BlimpCompositorDependencies compositor_deps( BlimpCompositorDependencies compositor_deps(
base::MakeUnique<MockCompositorDependencies>()); base::MakeUnique<MockCompositorDependencies>());
BlimpContentsImpl blimp_contents(kDummyTabId, window_, &compositor_deps, BlimpContentsImpl blimp_contents(
&ime_feature, &navigation_feature, kDummyBlimpContentsId, window_, &compositor_deps, &ime_feature,
&render_widget_feature, nullptr); &navigation_feature, &render_widget_feature, nullptr);
BlimpNavigationControllerImpl& navigation_controller = BlimpNavigationControllerImpl& navigation_controller =
blimp_contents.GetNavigationController(); blimp_contents.GetNavigationController();
...@@ -114,7 +114,7 @@ TEST_F(BlimpContentsImplTest, SetSizeAndScaleThroughTabControlFeature) { ...@@ -114,7 +114,7 @@ TEST_F(BlimpContentsImplTest, SetSizeAndScaleThroughTabControlFeature) {
BlimpCompositorDependencies compositor_deps( BlimpCompositorDependencies compositor_deps(
base::MakeUnique<MockCompositorDependencies>()); base::MakeUnique<MockCompositorDependencies>());
BlimpContentsImpl blimp_contents( BlimpContentsImpl blimp_contents(
kDummyTabId, window_, &compositor_deps, &ime_feature, nullptr, kDummyBlimpContentsId, window_, &compositor_deps, &ime_feature, nullptr,
&render_widget_feature, &tab_control_feature); &render_widget_feature, &tab_control_feature);
EXPECT_CALL(tab_control_feature, EXPECT_CALL(tab_control_feature,
......
...@@ -14,10 +14,6 @@ ...@@ -14,10 +14,6 @@
#include "blimp/client/core/render_widget/render_widget_feature.h" #include "blimp/client/core/render_widget/render_widget_feature.h"
#include "blimp/client/public/contents/blimp_contents_observer.h" #include "blimp/client/public/contents/blimp_contents_observer.h"
namespace {
const int kDummyTabId = 0;
}
namespace blimp { namespace blimp {
namespace client { namespace client {
...@@ -102,10 +98,7 @@ BlimpContentsImpl* BlimpContentsManager::GetBlimpContents(int id) { ...@@ -102,10 +98,7 @@ BlimpContentsImpl* BlimpContentsManager::GetBlimpContents(int id) {
} }
int BlimpContentsManager::CreateBlimpContentsId() { int BlimpContentsManager::CreateBlimpContentsId() {
// TODO(mlliu): currently, Blimp only supports a single tab, so returning a return next_blimp_contents_id_++;
// dummy tab id. Need to return real case id when Blimp supports multiple
// tabs.
return kDummyTabId;
} }
void BlimpContentsManager::EraseObserverFromMap(int id) { void BlimpContentsManager::EraseObserverFromMap(int id) {
......
...@@ -62,6 +62,10 @@ class BlimpContentsManager { ...@@ -62,6 +62,10 @@ class BlimpContentsManager {
void EraseObserverFromMap(int id); void EraseObserverFromMap(int id);
base::WeakPtr<BlimpContentsManager> GetWeakPtr(); base::WeakPtr<BlimpContentsManager> GetWeakPtr();
// The ID to use whenever a new BlimpContentsImpl is created. Incremented
// after each use.
int next_blimp_contents_id_ = 0;
// BlimpContentsManager owns the BlimpContentsDeletionObserver for the // BlimpContentsManager owns the BlimpContentsDeletionObserver for the
// contents it creates, with the content id being the key to help manage the // contents it creates, with the content id being the key to help manage the
// lifetime of the observers. // lifetime of the observers.
......
...@@ -24,7 +24,7 @@ ...@@ -24,7 +24,7 @@
using testing::_; using testing::_;
namespace { namespace {
const int kDummyTabId = 0; const int kDummyBlimpContentsId = 0;
} }
namespace blimp { namespace blimp {
...@@ -93,7 +93,7 @@ TEST_F(BlimpContentsManagerTest, GetNonExistingBlimpContents) { ...@@ -93,7 +93,7 @@ TEST_F(BlimpContentsManagerTest, GetNonExistingBlimpContents) {
&tab_control_feature); &tab_control_feature);
BlimpContentsImpl* existing_contents = BlimpContentsImpl* existing_contents =
blimp_contents_manager.GetBlimpContents(kDummyTabId); blimp_contents_manager.GetBlimpContents(kDummyBlimpContentsId);
EXPECT_EQ(nullptr, existing_contents); EXPECT_EQ(nullptr, existing_contents);
} }
......
...@@ -19,7 +19,7 @@ ...@@ -19,7 +19,7 @@
#endif // defined(OS_ANDROID) #endif // defined(OS_ANDROID)
namespace { namespace {
const int kDummyTabId = 0; const int kDummyBlimpContentsId = 0;
} }
namespace blimp { namespace blimp {
...@@ -60,7 +60,7 @@ TEST_F(BlimpContentsObserverTest, ObserverDies) { ...@@ -60,7 +60,7 @@ TEST_F(BlimpContentsObserverTest, ObserverDies) {
RenderWidgetFeature render_widget_feature; RenderWidgetFeature render_widget_feature;
BlimpCompositorDependencies compositor_deps( BlimpCompositorDependencies compositor_deps(
base::MakeUnique<MockCompositorDependencies>()); base::MakeUnique<MockCompositorDependencies>());
BlimpContentsImpl contents(kDummyTabId, window_, &compositor_deps, BlimpContentsImpl contents(kDummyBlimpContentsId, window_, &compositor_deps,
&ime_feature, nullptr, &render_widget_feature, &ime_feature, nullptr, &render_widget_feature,
nullptr); nullptr);
...@@ -81,8 +81,8 @@ TEST_F(BlimpContentsObserverTest, ContentsDies) { ...@@ -81,8 +81,8 @@ TEST_F(BlimpContentsObserverTest, ContentsDies) {
base::MakeUnique<MockCompositorDependencies>()); base::MakeUnique<MockCompositorDependencies>());
std::unique_ptr<BlimpContentsImpl> contents = std::unique_ptr<BlimpContentsImpl> contents =
base::MakeUnique<BlimpContentsImpl>( base::MakeUnique<BlimpContentsImpl>(
kDummyTabId, window_, &compositor_deps, &ime_feature, nullptr, kDummyBlimpContentsId, window_, &compositor_deps, &ime_feature,
&render_widget_feature, nullptr); nullptr, &render_widget_feature, nullptr);
observer.reset(new TestBlimpContentsObserver(contents.get())); observer.reset(new TestBlimpContentsObserver(contents.get()));
EXPECT_CALL(*observer, OnContentsDestroyed()).Times(1); EXPECT_CALL(*observer, OnContentsDestroyed()).Times(1);
EXPECT_EQ(observer->blimp_contents(), contents.get()); EXPECT_EQ(observer->blimp_contents(), contents.get());
......
...@@ -6,35 +6,33 @@ ...@@ -6,35 +6,33 @@
#include "blimp/client/core/contents/blimp_navigation_controller_delegate.h" #include "blimp/client/core/contents/blimp_navigation_controller_delegate.h"
namespace {
// TODO(shaktisahu): NavigationFeature currently needs a tab_id. Remove this
// later when it is fully integrated with BlimpClientContext.
const int kDummyTabId = 0;
} // namespace
namespace blimp { namespace blimp {
namespace client { namespace client {
BlimpNavigationControllerImpl::BlimpNavigationControllerImpl( BlimpNavigationControllerImpl::BlimpNavigationControllerImpl(
int blimp_contents_id,
BlimpNavigationControllerDelegate* delegate, BlimpNavigationControllerDelegate* delegate,
NavigationFeature* feature) NavigationFeature* feature)
: navigation_feature_(feature), delegate_(delegate) { : blimp_contents_id_(blimp_contents_id),
navigation_feature_(feature),
delegate_(delegate) {
if (navigation_feature_) if (navigation_feature_)
navigation_feature_->SetDelegate(kDummyTabId, this); navigation_feature_->SetDelegate(blimp_contents_id_, this);
} }
BlimpNavigationControllerImpl::~BlimpNavigationControllerImpl() { BlimpNavigationControllerImpl::~BlimpNavigationControllerImpl() {
if (navigation_feature_) if (navigation_feature_)
navigation_feature_->RemoveDelegate(kDummyTabId); navigation_feature_->RemoveDelegate(blimp_contents_id_);
} }
void BlimpNavigationControllerImpl::LoadURL(const GURL& url) { void BlimpNavigationControllerImpl::LoadURL(const GURL& url) {
current_url_ = url; current_url_ = url;
navigation_feature_->NavigateToUrlText(kDummyTabId, current_url_.spec()); navigation_feature_->NavigateToUrlText(blimp_contents_id_,
current_url_.spec());
} }
void BlimpNavigationControllerImpl::Reload() { void BlimpNavigationControllerImpl::Reload() {
navigation_feature_->Reload(kDummyTabId); navigation_feature_->Reload(blimp_contents_id_);
} }
bool BlimpNavigationControllerImpl::CanGoBack() const { bool BlimpNavigationControllerImpl::CanGoBack() const {
...@@ -48,11 +46,11 @@ bool BlimpNavigationControllerImpl::CanGoForward() const { ...@@ -48,11 +46,11 @@ bool BlimpNavigationControllerImpl::CanGoForward() const {
} }
void BlimpNavigationControllerImpl::GoBack() { void BlimpNavigationControllerImpl::GoBack() {
navigation_feature_->GoBack(kDummyTabId); navigation_feature_->GoBack(blimp_contents_id_);
} }
void BlimpNavigationControllerImpl::GoForward() { void BlimpNavigationControllerImpl::GoForward() {
navigation_feature_->GoForward(kDummyTabId); navigation_feature_->GoForward(blimp_contents_id_);
} }
const GURL& BlimpNavigationControllerImpl::GetURL() { const GURL& BlimpNavigationControllerImpl::GetURL() {
......
...@@ -23,7 +23,8 @@ class BlimpNavigationControllerImpl ...@@ -23,7 +23,8 @@ class BlimpNavigationControllerImpl
public NavigationFeature::NavigationFeatureDelegate { public NavigationFeature::NavigationFeatureDelegate {
public: public:
// The life time of |feature| must remain valid throughout |this|. // The life time of |feature| must remain valid throughout |this|.
BlimpNavigationControllerImpl(BlimpNavigationControllerDelegate* delegate, BlimpNavigationControllerImpl(int blimp_contents_id,
BlimpNavigationControllerDelegate* delegate,
NavigationFeature* feature); NavigationFeature* feature);
~BlimpNavigationControllerImpl() override; ~BlimpNavigationControllerImpl() override;
...@@ -45,6 +46,9 @@ class BlimpNavigationControllerImpl ...@@ -45,6 +46,9 @@ class BlimpNavigationControllerImpl
void OnLoadingChanged(int tab_id, bool loading) override; void OnLoadingChanged(int tab_id, bool loading) override;
void OnPageLoadStatusUpdate(int tab_id, bool completed) override; void OnPageLoadStatusUpdate(int tab_id, bool completed) override;
// The ID for the BlimpContentsImpl this is a controller for.
int blimp_contents_id_;
// The |navigation_feature_| is responsible for sending and receiving the // The |navigation_feature_| is responsible for sending and receiving the
// navigation state to the server over the proto channel. // navigation state to the server over the proto channel.
NavigationFeature* navigation_feature_; NavigationFeature* navigation_feature_;
......
...@@ -16,6 +16,7 @@ namespace blimp { ...@@ -16,6 +16,7 @@ namespace blimp {
namespace client { namespace client {
namespace { namespace {
const int kDummyBlimpContentsId = 0;
const GURL kExampleURL = GURL("https://www.example.com/"); const GURL kExampleURL = GURL("https://www.example.com/");
class MockBlimpNavigationControllerDelegate class MockBlimpNavigationControllerDelegate
...@@ -36,7 +37,8 @@ TEST(BlimpNavigationControllerImplTest, BackForwardNavigation) { ...@@ -36,7 +37,8 @@ TEST(BlimpNavigationControllerImplTest, BackForwardNavigation) {
testing::StrictMock<MockBlimpNavigationControllerDelegate> delegate; testing::StrictMock<MockBlimpNavigationControllerDelegate> delegate;
testing::StrictMock<FakeNavigationFeature> feature; testing::StrictMock<FakeNavigationFeature> feature;
BlimpNavigationControllerImpl navigation_controller(&delegate, &feature); BlimpNavigationControllerImpl navigation_controller(kDummyBlimpContentsId,
&delegate, &feature);
feature.SetDelegate(1, &navigation_controller); feature.SetDelegate(1, &navigation_controller);
EXPECT_CALL(delegate, OnNavigationStateChanged()); EXPECT_CALL(delegate, OnNavigationStateChanged());
...@@ -62,7 +64,8 @@ TEST(BlimpNavigationControllerImplTest, Loading) { ...@@ -62,7 +64,8 @@ TEST(BlimpNavigationControllerImplTest, Loading) {
testing::StrictMock<MockBlimpNavigationControllerDelegate> delegate; testing::StrictMock<MockBlimpNavigationControllerDelegate> delegate;
testing::StrictMock<FakeNavigationFeature> feature; testing::StrictMock<FakeNavigationFeature> feature;
BlimpNavigationControllerImpl navigation_controller(&delegate, &feature); BlimpNavigationControllerImpl navigation_controller(kDummyBlimpContentsId,
&delegate, &feature);
feature.SetDelegate(1, &navigation_controller); feature.SetDelegate(1, &navigation_controller);
EXPECT_CALL(delegate, OnNavigationStateChanged()); EXPECT_CALL(delegate, OnNavigationStateChanged());
......
...@@ -94,7 +94,13 @@ void NavigationFeature::ProcessMessage( ...@@ -94,7 +94,13 @@ void NavigationFeature::ProcessMessage(
const NavigationMessage& navigation_message = message->navigation(); const NavigationMessage& navigation_message = message->navigation();
NavigationFeatureDelegate* delegate = FindDelegate(tab_id); NavigationFeatureDelegate* delegate = FindDelegate(tab_id);
DCHECK(delegate) << "NavigationFeatureDelegate not found for tab " << tab_id; if (!delegate) {
VLOG(1) << "NavigationFeatureDelegate not found for " << tab_id
<< ". Ignoring.";
callback.Run(net::OK);
return;
}
switch (navigation_message.type()) { switch (navigation_message.type()) {
case NavigationMessage::NAVIGATION_STATE_CHANGED: { case NavigationMessage::NAVIGATION_STATE_CHANGED: {
const NavigationStateChangeMessage& details = const NavigationStateChangeMessage& details =
......
...@@ -43,6 +43,7 @@ void TabControlFeature::CreateTab(int tab_id) { ...@@ -43,6 +43,7 @@ void TabControlFeature::CreateTab(int tab_id) {
TabControlMessage* tab_control; TabControlMessage* tab_control;
std::unique_ptr<BlimpMessage> message = CreateBlimpMessage(&tab_control); std::unique_ptr<BlimpMessage> message = CreateBlimpMessage(&tab_control);
tab_control->mutable_create_tab(); tab_control->mutable_create_tab();
message->set_target_tab_id(tab_id);
outgoing_message_processor_->ProcessMessage(std::move(message), outgoing_message_processor_->ProcessMessage(std::move(message),
net::CompletionCallback()); net::CompletionCallback());
} }
...@@ -51,6 +52,7 @@ void TabControlFeature::CloseTab(int tab_id) { ...@@ -51,6 +52,7 @@ void TabControlFeature::CloseTab(int tab_id) {
TabControlMessage* tab_control; TabControlMessage* tab_control;
std::unique_ptr<BlimpMessage> message = CreateBlimpMessage(&tab_control); std::unique_ptr<BlimpMessage> message = CreateBlimpMessage(&tab_control);
tab_control->mutable_close_tab(); tab_control->mutable_close_tab();
message->set_target_tab_id(tab_id);
outgoing_message_processor_->ProcessMessage(std::move(message), outgoing_message_processor_->ProcessMessage(std::move(message),
net::CompletionCallback()); net::CompletionCallback());
} }
......
...@@ -67,7 +67,6 @@ namespace blimp { ...@@ -67,7 +67,6 @@ namespace blimp {
namespace engine { namespace engine {
namespace { namespace {
const int kDummyTabId = 0;
const float kDefaultScaleFactor = 1.f; const float kDefaultScaleFactor = 1.f;
const int kDefaultDisplayWidth = 800; const int kDefaultDisplayWidth = 800;
const int kDefaultDisplayHeight = 600; const int kDefaultDisplayHeight = 600;
...@@ -236,7 +235,6 @@ BlimpEngineSession::BlimpEngineSession( ...@@ -236,7 +235,6 @@ BlimpEngineSession::BlimpEngineSession(
screen_->UpdateDisplayScaleAndSize( screen_->UpdateDisplayScaleAndSize(
kDefaultScaleFactor, kDefaultScaleFactor,
gfx::Size(kDefaultDisplayWidth, kDefaultDisplayHeight)); gfx::Size(kDefaultDisplayWidth, kDefaultDisplayHeight));
render_widget_feature_.SetDelegate(kDummyTabId, this);
std::unique_ptr<HeliumBlobSenderDelegate> helium_blob_delegate( std::unique_ptr<HeliumBlobSenderDelegate> helium_blob_delegate(
new HeliumBlobSenderDelegate); new HeliumBlobSenderDelegate);
...@@ -252,8 +250,6 @@ BlimpEngineSession::BlimpEngineSession( ...@@ -252,8 +250,6 @@ BlimpEngineSession::BlimpEngineSession(
} }
BlimpEngineSession::~BlimpEngineSession() { BlimpEngineSession::~BlimpEngineSession() {
render_widget_feature_.RemoveDelegate(kDummyTabId);
window_tree_host_->GetInputMethod()->RemoveObserver(this); window_tree_host_->GetInputMethod()->RemoveObserver(this);
// Ensure that all tabs are torn down first, since teardown will // Ensure that all tabs are torn down first, since teardown will
...@@ -385,22 +381,6 @@ void BlimpEngineSession::HandleResize(float device_pixel_ratio, ...@@ -385,22 +381,6 @@ void BlimpEngineSession::HandleResize(float device_pixel_ratio,
} }
} }
void BlimpEngineSession::OnWebGestureEvent(
content::RenderWidgetHost* render_widget_host,
std::unique_ptr<blink::WebGestureEvent> event) {
TRACE_EVENT1("blimp", "BlimpEngineSession::OnWebGestureEvent", "type",
event->type);
render_widget_host->ForwardGestureEvent(*event);
}
void BlimpEngineSession::OnCompositorMessageReceived(
content::RenderWidgetHost* render_widget_host,
const std::vector<uint8_t>& message) {
TRACE_EVENT0("blimp", "BlimpEngineSession::OnCompositorMessageReceived");
render_widget_host->HandleCompositorProto(message);
}
void BlimpEngineSession::OnTextInputTypeChanged( void BlimpEngineSession::OnTextInputTypeChanged(
const ui::TextInputClient* client) {} const ui::TextInputClient* client) {}
...@@ -428,7 +408,7 @@ void BlimpEngineSession::OnTextInputStateChanged( ...@@ -428,7 +408,7 @@ void BlimpEngineSession::OnTextInputStateChanged(
// OnShowImeIfNeeded is used instead to send show IME request to client. // OnShowImeIfNeeded is used instead to send show IME request to client.
if (type == ui::TEXT_INPUT_TYPE_NONE) if (type == ui::TEXT_INPUT_TYPE_NONE)
render_widget_feature_.SendHideImeRequest( render_widget_feature_.SendHideImeRequest(
kDummyTabId, tab_->tab_id(),
tab_->web_contents()->GetRenderWidgetHostView()->GetRenderWidgetHost()); tab_->web_contents()->GetRenderWidgetHostView()->GetRenderWidgetHost());
} }
...@@ -443,7 +423,7 @@ void BlimpEngineSession::OnShowImeIfNeeded() { ...@@ -443,7 +423,7 @@ void BlimpEngineSession::OnShowImeIfNeeded() {
return; return;
render_widget_feature_.SendShowImeRequest( render_widget_feature_.SendShowImeRequest(
kDummyTabId, tab_->tab_id(),
tab_->web_contents()->GetRenderWidgetHostView()->GetRenderWidgetHost(), tab_->web_contents()->GetRenderWidgetHostView()->GetRenderWidgetHost(),
window_tree_host_->GetInputMethod()->GetTextInputClient()); window_tree_host_->GetInputMethod()->GetTextInputClient());
} }
...@@ -560,8 +540,11 @@ void BlimpEngineSession::ForwardCompositorProto( ...@@ -560,8 +540,11 @@ void BlimpEngineSession::ForwardCompositorProto(
content::RenderWidgetHost* render_widget_host, content::RenderWidgetHost* render_widget_host,
const std::vector<uint8_t>& proto) { const std::vector<uint8_t>& proto) {
TRACE_EVENT0("blimp", "BlimpEngineSession::ForwardCompositorProto"); TRACE_EVENT0("blimp", "BlimpEngineSession::ForwardCompositorProto");
render_widget_feature_.SendCompositorMessage(kDummyTabId, render_widget_host, if (!tab_) {
proto); return;
}
render_widget_feature_.SendCompositorMessage(tab_->tab_id(),
render_widget_host, proto);
} }
void BlimpEngineSession::NavigationStateChanged( void BlimpEngineSession::NavigationStateChanged(
......
...@@ -72,11 +72,9 @@ class BlimpWindowTreeHost; ...@@ -72,11 +72,9 @@ class BlimpWindowTreeHost;
class EngineNetworkComponents; class EngineNetworkComponents;
class Tab; class Tab;
class BlimpEngineSession class BlimpEngineSession : public BlimpMessageProcessor,
: public BlimpMessageProcessor, public content::WebContentsDelegate,
public content::WebContentsDelegate, public ui::InputMethodObserver {
public ui::InputMethodObserver,
public EngineRenderWidgetFeature::RenderWidgetMessageDelegate {
public: public:
using GetPortCallback = base::Callback<void(uint16_t)>; using GetPortCallback = base::Callback<void(uint16_t)>;
...@@ -124,15 +122,6 @@ class BlimpEngineSession ...@@ -124,15 +122,6 @@ class BlimpEngineSession
// |device_pixel_ratio|. // |device_pixel_ratio|.
void HandleResize(float device_pixel_ratio, const gfx::Size& size); void HandleResize(float device_pixel_ratio, const gfx::Size& size);
// RenderWidgetMessage handler methods.
// RenderWidgetMessageDelegate implementation.
void OnWebGestureEvent(
content::RenderWidgetHost* render_widget_host,
std::unique_ptr<blink::WebGestureEvent> event) override;
void OnCompositorMessageReceived(
content::RenderWidgetHost* render_widget_host,
const std::vector<uint8_t>& message) override;
// content::WebContentsDelegate implementation. // content::WebContentsDelegate implementation.
content::WebContents* OpenURLFromTab( content::WebContents* OpenURLFromTab(
content::WebContents* source, content::WebContents* source,
......
...@@ -42,10 +42,14 @@ Tab::Tab(std::unique_ptr<content::WebContents> web_contents, ...@@ -42,10 +42,14 @@ Tab::Tab(std::unique_ptr<content::WebContents> web_contents,
// that to override user agent string from BlimpContentRendererClient. // that to override user agent string from BlimpContentRendererClient.
web_contents_->SetUserAgentOverride(GetBlimpEngineUserAgent()); web_contents_->SetUserAgentOverride(GetBlimpEngineUserAgent());
render_widget_feature_->SetDelegate(tab_id_, this);
Observe(web_contents_.get()); Observe(web_contents_.get());
} }
Tab::~Tab() {} Tab::~Tab() {
render_widget_feature_->RemoveDelegate(tab_id_);
}
void Tab::Resize(float device_pixel_ratio, const gfx::Size& size_in_dips) { void Tab::Resize(float device_pixel_ratio, const gfx::Size& size_in_dips) {
DVLOG(1) << "Resize to " << size_in_dips.ToString() << ", " DVLOG(1) << "Resize to " << size_in_dips.ToString() << ", "
...@@ -154,5 +158,19 @@ void Tab::SendPageLoadStatusUpdate(PageLoadStatus load_status) { ...@@ -154,5 +158,19 @@ void Tab::SendPageLoadStatusUpdate(PageLoadStatus load_status) {
net::CompletionCallback()); net::CompletionCallback());
} }
void Tab::OnWebGestureEvent(content::RenderWidgetHost* render_widget_host,
std::unique_ptr<blink::WebGestureEvent> event) {
TRACE_EVENT1("blimp", "Tab::OnWebGestureEvent", "type", event->type);
render_widget_host->ForwardGestureEvent(*event);
}
void Tab::OnCompositorMessageReceived(
content::RenderWidgetHost* render_widget_host,
const std::vector<uint8_t>& message) {
TRACE_EVENT0("blimp", "Tab::OnCompositorMessageReceived");
render_widget_host->HandleCompositorProto(message);
}
} // namespace engine } // namespace engine
} // namespace blimp } // namespace blimp
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#define BLIMP_ENGINE_SESSION_TAB_H_ #define BLIMP_ENGINE_SESSION_TAB_H_
#include "base/macros.h" #include "base/macros.h"
#include "blimp/engine/feature/engine_render_widget_feature.h"
#include "blimp/engine/session/page_load_tracker.h" #include "blimp/engine/session/page_load_tracker.h"
#include "content/public/browser/invalidate_type.h" #include "content/public/browser/invalidate_type.h"
#include "content/public/browser/web_contents_observer.h" #include "content/public/browser/web_contents_observer.h"
...@@ -25,7 +26,9 @@ class EngineRenderWidgetFeature; ...@@ -25,7 +26,9 @@ class EngineRenderWidgetFeature;
// Owns WebContents, handles operations such as navigation requests in the tab, // Owns WebContents, handles operations such as navigation requests in the tab,
// and has one-to-one mapping to a client tab. // and has one-to-one mapping to a client tab.
class Tab : public content::WebContentsObserver, public PageLoadTrackerClient { class Tab : public content::WebContentsObserver,
public PageLoadTrackerClient,
public EngineRenderWidgetFeature::RenderWidgetMessageDelegate {
public: public:
// Caller ensures |render_widget_feature| and |navigation_message_sender| // Caller ensures |render_widget_feature| and |navigation_message_sender|
// outlives this object. // outlives this object.
...@@ -57,6 +60,15 @@ class Tab : public content::WebContentsObserver, public PageLoadTrackerClient { ...@@ -57,6 +60,15 @@ class Tab : public content::WebContentsObserver, public PageLoadTrackerClient {
// PageLoadTrackerClient implementation. // PageLoadTrackerClient implementation.
void SendPageLoadStatusUpdate(PageLoadStatus load_status) override; void SendPageLoadStatusUpdate(PageLoadStatus load_status) override;
// RenderWidgetMessage handler methods.
// RenderWidgetMessageDelegate implementation.
void OnWebGestureEvent(
content::RenderWidgetHost* render_widget_host,
std::unique_ptr<blink::WebGestureEvent> event) override;
void OnCompositorMessageReceived(
content::RenderWidgetHost* render_widget_host,
const std::vector<uint8_t>& message) override;
private: private:
// content::WebContentsObserver implementation. // content::WebContentsObserver implementation.
void RenderViewCreated(content::RenderViewHost* render_view_host) override; void RenderViewCreated(content::RenderViewHost* render_view_host) override;
......
...@@ -513,6 +513,7 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode ...@@ -513,6 +513,7 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
OnClickListener newTabClickHandler = new OnClickListener() { OnClickListener newTabClickHandler = new OnClickListener() {
@Override @Override
public void onClick(View v) { public void onClick(View v) {
getTabModelSelector().getModel(false).commitAllTabClosures();
// This assumes that the keyboard can not be seen at the same time as the // This assumes that the keyboard can not be seen at the same time as the
// newtab button on the toolbar. // newtab button on the toolbar.
getCurrentTabCreator().launchNTP(); getCurrentTabCreator().launchNTP();
...@@ -1057,11 +1058,13 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode ...@@ -1057,11 +1058,13 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
if (id == R.id.move_to_other_window_menu_id) { if (id == R.id.move_to_other_window_menu_id) {
if (currentTab != null) moveTabToOtherWindow(currentTab); if (currentTab != null) moveTabToOtherWindow(currentTab);
} else if (id == R.id.new_tab_menu_id) { } else if (id == R.id.new_tab_menu_id) {
getTabModelSelector().getModel(false).commitAllTabClosures();
RecordUserAction.record("MobileMenuNewTab"); RecordUserAction.record("MobileMenuNewTab");
RecordUserAction.record("MobileNewTabOpened"); RecordUserAction.record("MobileNewTabOpened");
getTabCreator(false).launchUrl(UrlConstants.NTP_URL, TabLaunchType.FROM_CHROME_UI); getTabCreator(false).launchUrl(UrlConstants.NTP_URL, TabLaunchType.FROM_CHROME_UI);
} else if (id == R.id.new_incognito_tab_menu_id) { } else if (id == R.id.new_incognito_tab_menu_id) {
if (PrefServiceBridge.getInstance().isIncognitoModeEnabled()) { if (PrefServiceBridge.getInstance().isIncognitoModeEnabled()) {
getTabModelSelector().getModel(false).commitAllTabClosures();
// This action must be recorded before opening the incognito tab since UMA actions // This action must be recorded before opening the incognito tab since UMA actions
// are dropped when an incognito tab is open. // are dropped when an incognito tab is open.
RecordUserAction.record("MobileMenuNewIncognitoTab"); RecordUserAction.record("MobileMenuNewIncognitoTab");
......
...@@ -260,8 +260,14 @@ public class LayoutManagerDocument extends LayoutManager ...@@ -260,8 +260,14 @@ public class LayoutManagerDocument extends LayoutManager
String url = tab.getUrl(); String url = tab.getUrl();
boolean isNativePage = url != null && url.startsWith(UrlConstants.CHROME_NATIVE_SCHEME); boolean isNativePage = url != null && url.startsWith(UrlConstants.CHROME_NATIVE_SCHEME);
int themeColor = tab.getThemeColor(); int themeColor = tab.getThemeColor();
boolean canUseLiveTexture = // TODO(xingliu): Remove this override themeColor for Blimp tabs. See crbug.com/644774.
tab.getContentViewCore() != null && !tab.isShowingSadTab() && !isNativePage; if (tab.isBlimpTab() && tab.getBlimpContents() != null) {
themeColor = tab.getBlimpContents().getThemeColor();
}
boolean canUseLiveTexture = tab.isBlimpTab()
|| tab.getContentViewCore() != null && !tab.isShowingSadTab() && !isNativePage;
boolean needsUpdate = layoutTab.initFromHost(tab.getBackgroundColor(), tab.shouldStall(), boolean needsUpdate = layoutTab.initFromHost(tab.getBackgroundColor(), tab.shouldStall(),
canUseLiveTexture, themeColor, ColorUtils.getTextBoxColorForToolbarBackground( canUseLiveTexture, themeColor, ColorUtils.getTextBoxColorForToolbarBackground(
mContext.getResources(), tab, themeColor), mContext.getResources(), tab, themeColor),
......
...@@ -806,6 +806,7 @@ public class StripLayoutHelper { ...@@ -806,6 +806,7 @@ public class StripLayoutHelper {
resetResizeTimeout(false); resetResizeTimeout(false);
if (mNewTabButton.click(x, y) && mModel != null) { if (mNewTabButton.click(x, y) && mModel != null) {
if (!mModel.isIncognito()) mModel.commitAllTabClosures();
mTabCreator.launchNTP(); mTabCreator.launchNTP();
return; return;
} }
...@@ -852,6 +853,7 @@ public class StripLayoutHelper { ...@@ -852,6 +853,7 @@ public class StripLayoutHelper {
mInteractingTab = null; mInteractingTab = null;
mReorderState = REORDER_SCROLL_NONE; mReorderState = REORDER_SCROLL_NONE;
if (mNewTabButton.onUpOrCancel() && mModel != null) { if (mNewTabButton.onUpOrCancel() && mModel != null) {
if (!mModel.isIncognito()) mModel.commitAllTabClosures();
mTabCreator.launchNTP(); mTabCreator.launchNTP();
} }
} }
......
...@@ -59,6 +59,7 @@ public class EmptyBackgroundViewTablet extends FrameLayout { ...@@ -59,6 +59,7 @@ public class EmptyBackgroundViewTablet extends FrameLayout {
@Override @Override
public void onClick(View v) { public void onClick(View v) {
if (mTabCreator == null) return; if (mTabCreator == null) return;
mTabModelSelector.getModel(false).commitAllTabClosures();
mTabCreator.launchNTP(); mTabCreator.launchNTP();
} }
}); });
......
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