Skip to content

Commit

Permalink
Consolidate AppControllDelegate to ExtensionsDelegate.
Browse files Browse the repository at this point in the history 
* Prep for athena on chrome.

Another notable change:
* pass app_id to AppActivity. it's known at creation time.

Note: this depends on https://codereview.chromium.org/505273002/

BUG=397167

Review URL: https://codereview.chromium.org/501093003

Cr-Commit-Position: refs/heads/master@{#292264}
  • Loading branch information
@mitoshima
mitoshima authored and Commit bot committed Aug 27, 2014
1 parent 86c315c commit 07f5926
Show file tree
Hide file tree
Showing 19 changed files with 98 additions and 170 deletions.
4 changes: 2 additions & 2 deletions athena/activity/public/activity_factory.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ class ATHENA_EXPORT ActivityFactory {
// The returned activity should own |app_window|.
// TODO(oshima): Consolidate these two methods to create AppActivity
// once crbug.com/403726 is finished.
virtual Activity* CreateAppActivity(
extensions::ShellAppWindow* app_window) = 0;
virtual Activity* CreateAppActivity(extensions::ShellAppWindow* app_window,
const std::string& id) = 0;

// Create an activity of an app with |app_window| for chrome environment.
virtual Activity* CreateAppActivity(apps::AppWindow* app_window) = 0;
Expand Down
4 changes: 0 additions & 4 deletions athena/athena.gyp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,6 @@
'content/app_registry_impl.cc',
'content/content_activity_factory.cc',
'content/content_app_model_builder.cc',
'content/delegate/app_content_control_delegate_impl.cc',
'content/public/app_content_control_delegate.h',
'content/public/app_registry.h',
'content/content_activity_factory.h',
'content/public/content_activity_factory_creator.h',
Expand Down Expand Up @@ -181,7 +179,6 @@
'resources/athena_resources.gyp:athena_resources',
],
'sources': [
'content/public/app_content_control_delegate.h',
'extensions/test/test_extensions_delegate.cc',
'test/athena_test_base.cc',
'test/athena_test_base.h',
Expand All @@ -191,7 +188,6 @@
'test/sample_activity.h',
'test/sample_activity_factory.cc',
'test/sample_activity_factory.h',
'test/test_app_content_control_delegate_impl.cc',
'test/test_app_model_builder.cc',
'test/test_app_model_builder.h',
'wm/test/window_manager_impl_test_api.cc',
Expand Down
17 changes: 5 additions & 12 deletions athena/content/app_activity.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

#include "athena/activity/public/activity_manager.h"
#include "athena/content/app_activity_registry.h"
#include "athena/content/public/app_content_control_delegate.h"
#include "athena/content/public/app_registry.h"
#include "content/public/browser/web_contents.h"
#include "ui/views/controls/webview/webview.h"
Expand All @@ -15,8 +14,9 @@
namespace athena {

// TODO(mukai): specifies the same accelerators of WebActivity.
AppActivity::AppActivity()
: web_view_(NULL),
AppActivity::AppActivity(const std::string& app_id)
: app_id_(app_id),
web_view_(NULL),
current_state_(ACTIVITY_UNLOADED),
app_activity_registry_(NULL) {
}
Expand Down Expand Up @@ -120,6 +120,7 @@ views::View* AppActivity::GetContentsView() {
SetCurrentState(ACTIVITY_INVISIBLE);
Observe(web_contents);
overview_mode_image_ = gfx::ImageSkia();
RegisterActivity();
}
return web_view_;
}
Expand All @@ -142,13 +143,6 @@ void AppActivity::DidUpdateFaviconURL(
ActivityManager::Get()->UpdateActivity(this);
}

void AppActivity::DidStartNavigationToPendingEntry(
const GURL& url,
content::NavigationController::ReloadType reload_type) {
if (!app_activity_registry_)
RegisterActivity();
}

// Register an |activity| with an application.
// Note: This should only get called once for an |app_window| of the
// |activity|.
Expand All @@ -157,8 +151,7 @@ void AppActivity::RegisterActivity() {
AppRegistry* app_registry = AppRegistry::Get();
// Get the application's registry.
app_activity_registry_ = app_registry->GetAppActivityRegistry(
app_registry->GetDelegate()->GetApplicationID(web_contents),
web_contents->GetBrowserContext());
app_id_, web_contents->GetBrowserContext());
DCHECK(app_activity_registry_);
// Register the activity.
app_activity_registry_->RegisterAppActivity(this);
Expand Down
7 changes: 3 additions & 4 deletions athena/content/app_activity.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class AppActivity : public Activity,
public ActivityViewModel,
public content::WebContentsObserver {
public:
AppActivity();
explicit AppActivity(const std::string& app_id);
virtual ~AppActivity();

// Activity:
Expand All @@ -53,9 +53,6 @@ class AppActivity : public Activity,
bool explicit_set) OVERRIDE;
virtual void DidUpdateFaviconURL(
const std::vector<content::FaviconURL>& candidates) OVERRIDE;
virtual void DidStartNavigationToPendingEntry(
const GURL& url,
content::NavigationController::ReloadType reload_type) OVERRIDE;

protected:
virtual content::WebContents* GetWebContents() = 0;
Expand All @@ -64,6 +61,8 @@ class AppActivity : public Activity,
// Register this activity with its application.
void RegisterActivity();

const std::string app_id_;

views::WebView* web_view_;

// The current state for this activity.
Expand Down
8 changes: 3 additions & 5 deletions athena/content/app_activity_registry.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
#include "athena/activity/public/activity_manager.h"
#include "athena/content/app_activity.h"
#include "athena/content/app_activity_proxy.h"
#include "athena/content/public/app_content_control_delegate.h"
#include "athena/content/public/app_registry.h"
#include "athena/extensions/public/extensions_delegate.h"
#include "ui/aura/window.h"
#include "ui/views/view.h"
#include "ui/views/widget/widget.h"
Expand Down Expand Up @@ -91,8 +91,7 @@ void AppActivityRegistry::Unload() {
MoveBeforeMruApplicationWindow(unloaded_activity_proxy_->GetWindow());

// Unload the application. This operation will be asynchronous.
if (!AppRegistry::Get()->GetDelegate()->UnloadApplication(app_id_,
browser_context_)) {
if (!ExtensionsDelegate::Get(browser_context_)->UnloadApp(app_id_)) {
while(!activity_list_.empty())
delete activity_list_.back();
}
Expand All @@ -110,8 +109,7 @@ void AppActivityRegistry::ProxyDestroyed(AppActivityProxy* proxy) {
void AppActivityRegistry::RestartApplication(AppActivityProxy* proxy) {
DCHECK_EQ(unloaded_activity_proxy_, proxy);
// Restart the application.
AppRegistry::Get()->GetDelegate()->RestartApplication(app_id_,
browser_context_);
ExtensionsDelegate::Get(browser_context_)->LaunchApp(app_id_);
// Remove the activity from the Activity manager.
ActivityManager::Get()->RemoveActivity(unloaded_activity_proxy_);
delete unloaded_activity_proxy_; // Will call ProxyDestroyed.
Expand Down
107 changes: 49 additions & 58 deletions athena/content/app_activity_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
#include "athena/activity/public/activity_manager.h"
#include "athena/content/app_activity.h"
#include "athena/content/app_activity_registry.h"
#include "athena/content/public/app_content_control_delegate.h"
#include "athena/content/public/app_registry.h"
#include "athena/extensions/public/extensions_delegate.h"
#include "athena/test/athena_test_base.h"
#include "extensions/common/extension_set.h"
#include "ui/aura/window.h"
#include "ui/views/view.h"
#include "ui/views/widget/widget.h"
Expand All @@ -29,11 +30,10 @@ const char kDummyApp2[] = "bbbbbbb";
// A dummy test app activity which works without content / ShellAppWindow.
class TestAppActivity : public AppActivity {
public:
explicit TestAppActivity(const std::string& app_id) :
AppActivity(),
app_id_(app_id),
view_(new views::View()),
current_state_(ACTIVITY_VISIBLE) {
explicit TestAppActivity(const std::string& app_id)
: AppActivity(app_id),
view_(new views::View()),
current_state_(ACTIVITY_VISIBLE) {
app_activity_registry_ =
AppRegistry::Get()->GetAppActivityRegistry(app_id, NULL);
app_activity_registry_->RegisterAppActivity(this);
Expand Down Expand Up @@ -67,9 +67,7 @@ class TestAppActivity : public AppActivity {
}

// AppActivity:
virtual content::WebContents* GetWebContents() OVERRIDE {
return NULL;
}
virtual content::WebContents* GetWebContents() OVERRIDE { return NULL; }

// ActivityViewModel:
virtual void Init() OVERRIDE {}
Expand All @@ -83,9 +81,6 @@ class TestAppActivity : public AppActivity {
// If known the registry which holds all activities for the associated app.
AppActivityRegistry* app_activity_registry_;

// The application ID.
const std::string& app_id_;

// The title of the activity.
base::string16 title_;

Expand All @@ -99,61 +94,57 @@ class TestAppActivity : public AppActivity {
};

// An AppContentDelegateClass which we can query for call stats.
class TestAppContentControlDelegate : public AppContentControlDelegate {
class TestExtensionsDelegate : public ExtensionsDelegate {
public:
TestAppContentControlDelegate() : unload_called_(0),
restart_called_(0) {}
virtual ~TestAppContentControlDelegate() {}

int unload_called() { return unload_called_; }
int restart_called() { return restart_called_; }
void SetExtensionID(const std::string& extension_id) {
extension_id_to_return_ = extension_id;
}
TestExtensionsDelegate() : unload_called_(0), restart_called_(0) {}
virtual ~TestExtensionsDelegate() {}

int unload_called() const { return unload_called_; }
int restart_called() const { return restart_called_; }

// ExtensionsDelegate:
virtual content::BrowserContext* GetBrowserContext() const OVERRIDE {
return NULL;
}
virtual const extensions::ExtensionSet& GetInstalledExtensions() OVERRIDE {
return extension_set_;
}
// Unload an application. Returns true when unloaded.
virtual bool UnloadApplication(
const std::string& app_id,
content::BrowserContext* browser_context) OVERRIDE {
virtual bool UnloadApp(const std::string& app_id) OVERRIDE {
unload_called_++;
// Since we did not close anything we let the framework clean up.
return false;
}
// Restarts an application. Returns true when the restart was initiated.
virtual bool RestartApplication(
const std::string& app_id,
content::BrowserContext* browser_context) OVERRIDE {
virtual bool LaunchApp(const std::string& app_id) OVERRIDE {
restart_called_++;
return true;
}
// Returns the application ID (or an empty string) for a given web content.
virtual std::string GetApplicationID(
content::WebContents* web_contents) OVERRIDE {
return extension_id_to_return_;
}

private:
int unload_called_;
int restart_called_;
std::string extension_id_to_return_;

DISALLOW_COPY_AND_ASSIGN(TestAppContentControlDelegate);
extensions::ExtensionSet extension_set_;

DISALLOW_COPY_AND_ASSIGN(TestExtensionsDelegate);
};

} // namespace

// Our testing base.
class AppActivityTest : public AthenaTestBase {
public:
AppActivityTest() : test_app_content_control_delegate_(NULL) {}
AppActivityTest() : test_extensions_delegate_(NULL) {}
virtual ~AppActivityTest() {}

// AthenaTestBase:
virtual void SetUp() OVERRIDE {
AthenaTestBase::SetUp();
// Create and install our TestAppContentDelegate with instrumentation.
test_app_content_control_delegate_ = new TestAppContentControlDelegate();
AppRegistry::Get()->SetDelegate(test_app_content_control_delegate_);
ExtensionsDelegate::Shutdown();
// The instance will be deleted by ExtensionsDelegate::Shutdown().
test_extensions_delegate_ = new TestExtensionsDelegate();
}

// A function to create an Activity.
Expand Down Expand Up @@ -181,12 +172,12 @@ class AppActivityTest : public AthenaTestBase {
}

protected:
TestAppContentControlDelegate* test_app_content_control_delegate() {
return test_app_content_control_delegate_;
TestExtensionsDelegate* test_extensions_delegate() {
return test_extensions_delegate_;
}

private:
TestAppContentControlDelegate* test_app_content_control_delegate_;
TestExtensionsDelegate* test_extensions_delegate_;

DISALLOW_COPY_AND_ASSIGN(AppActivityTest);
};
Expand All @@ -203,8 +194,8 @@ TEST_F(AppActivityTest, OneAppActivity) {
CloseActivity(app_activity);
}
EXPECT_EQ(0, AppRegistry::Get()->NumberOfApplications());
EXPECT_EQ(0, test_app_content_control_delegate()->unload_called());
EXPECT_EQ(0, test_app_content_control_delegate()->restart_called());
EXPECT_EQ(0, test_extensions_delegate()->unload_called());
EXPECT_EQ(0, test_extensions_delegate()->restart_called());
}

// Test running of two applications.
Expand All @@ -222,8 +213,8 @@ TEST_F(AppActivityTest, TwoAppsWithOneActivityEach) {
CloseActivity(app_activity2);
}
EXPECT_EQ(0, AppRegistry::Get()->NumberOfApplications());
EXPECT_EQ(0, test_app_content_control_delegate()->unload_called());
EXPECT_EQ(0, test_app_content_control_delegate()->restart_called());
EXPECT_EQ(0, test_extensions_delegate()->unload_called());
EXPECT_EQ(0, test_extensions_delegate()->restart_called());
}

// Create and destroy two activities for the same application.
Expand Down Expand Up @@ -255,8 +246,8 @@ TEST_F(AppActivityTest, TwoAppActivities) {
CloseActivity(app_activity1);
}
EXPECT_EQ(0, AppRegistry::Get()->NumberOfApplications());
EXPECT_EQ(0, test_app_content_control_delegate()->unload_called());
EXPECT_EQ(0, test_app_content_control_delegate()->restart_called());
EXPECT_EQ(0, test_extensions_delegate()->unload_called());
EXPECT_EQ(0, test_extensions_delegate()->restart_called());
}

// Test unload and the creation of the proxy, then "closing the activity".
Expand All @@ -273,13 +264,13 @@ TEST_F(AppActivityTest, TestUnloadFollowedByClose) {
// Calling Unload now should not do anything since at least one activity in
// the registry is still visible.
app_activity_registry->Unload();
EXPECT_EQ(0, test_app_content_control_delegate()->unload_called());
EXPECT_EQ(0, test_extensions_delegate()->unload_called());

// After setting our activity to unloaded however the application should get
// unloaded as requested.
app_activity->SetCurrentState(Activity::ACTIVITY_UNLOADED);
app_activity_registry->Unload();
EXPECT_EQ(1, test_app_content_control_delegate()->unload_called());
EXPECT_EQ(1, test_extensions_delegate()->unload_called());

// Check that our created application is gone, and instead a proxy got
// created.
Expand All @@ -297,8 +288,8 @@ TEST_F(AppActivityTest, TestUnloadFollowedByClose) {
CloseActivity(activity_proxy);

EXPECT_EQ(0, AppRegistry::Get()->NumberOfApplications());
EXPECT_EQ(1, test_app_content_control_delegate()->unload_called());
EXPECT_EQ(0, test_app_content_control_delegate()->restart_called());
EXPECT_EQ(1, test_extensions_delegate()->unload_called());
EXPECT_EQ(0, test_extensions_delegate()->restart_called());
}

// Test that when unloading an app while multiple apps / activities are present,
Expand Down Expand Up @@ -357,13 +348,13 @@ TEST_F(AppActivityTest, TestMultipleActivityUnloadLock) {
// After setting all activities to UNLOADED the application should unload.
app_activity1->SetCurrentState(Activity::ACTIVITY_UNLOADED);
app_activity1->app_activity_registry()->Unload();
EXPECT_EQ(0, test_app_content_control_delegate()->unload_called());
EXPECT_EQ(0, test_extensions_delegate()->unload_called());
app_activity2->SetCurrentState(Activity::ACTIVITY_UNLOADED);
app_activity2->app_activity_registry()->Unload();
EXPECT_EQ(0, test_app_content_control_delegate()->unload_called());
EXPECT_EQ(0, test_extensions_delegate()->unload_called());
app_activity3->SetCurrentState(Activity::ACTIVITY_UNLOADED);
app_activity3->app_activity_registry()->Unload();
EXPECT_EQ(1, test_app_content_control_delegate()->unload_called());
EXPECT_EQ(1, test_extensions_delegate()->unload_called());

// Now there should only be the proxy activity left.
ASSERT_EQ(1, AppRegistry::Get()->NumberOfApplications());
Expand All @@ -382,8 +373,8 @@ TEST_F(AppActivityTest, TestMultipleActivityUnloadLock) {
CloseActivity(activity_proxy);

EXPECT_EQ(0, AppRegistry::Get()->NumberOfApplications());
EXPECT_EQ(1, test_app_content_control_delegate()->unload_called());
EXPECT_EQ(0, test_app_content_control_delegate()->restart_called());
EXPECT_EQ(1, test_extensions_delegate()->unload_called());
EXPECT_EQ(0, test_extensions_delegate()->restart_called());
}

// Test that activating the proxy will reload the application.
Expand All @@ -397,14 +388,14 @@ TEST_F(AppActivityTest, TestUnloadWithReload) {
// Unload the activity.
app_activity->SetCurrentState(Activity::ACTIVITY_UNLOADED);
app_activity_registry->Unload();
EXPECT_EQ(1, test_app_content_control_delegate()->unload_called());
EXPECT_EQ(1, test_extensions_delegate()->unload_called());

// Try to activate the activity again. This will force the application to
// reload.
Activity* activity_proxy =
app_activity_registry->unloaded_activity_proxy_for_test();
activity_proxy->SetCurrentState(Activity::ACTIVITY_VISIBLE);
EXPECT_EQ(1, test_app_content_control_delegate()->restart_called());
EXPECT_EQ(1, test_extensions_delegate()->restart_called());

// However - the restart in this test framework does not really restart and
// all objects should be gone now.
Expand Down
Loading

0 comments on commit 07f5926

Please sign in to comment.