From e9f195ef08c50293b7388b8ed05c12174d2dc139 Mon Sep 17 00:00:00 2001 From: "rockot@chromium.org" Date: Thu, 13 Jun 2013 06:37:42 +0000 Subject: The cache was designed to load an extension's stored geometry from sync only when the extension is first loaded. Because the cache was being lazily created on first access, it would never exist on browser startup when all installed extensions were being loaded. This will attempt to load an extension's geometry from storage on first access as well, if it is not already loaded. BUG=246810 Review URL: https://chromiumcodereview.appspot.com/16012019 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@206011 0039d316-1c4b-4281-b951-d872f2087c98 --- apps/shell_window_geometry_cache.cc | 17 +++++++++++------ apps/shell_window_geometry_cache.h | 4 ++-- apps/shell_window_geometry_cache_unittest.cc | 6 +----- 3 files changed, 14 insertions(+), 13 deletions(-) (limited to 'apps') diff --git a/apps/shell_window_geometry_cache.cc b/apps/shell_window_geometry_cache.cc index 19901d9..f72dfcb 100644 --- a/apps/shell_window_geometry_cache.cc +++ b/apps/shell_window_geometry_cache.cc @@ -124,14 +124,19 @@ bool ShellWindowGeometryCache::GetGeometry( const std::string& extension_id, const std::string& window_id, gfx::Rect* bounds, - ui::WindowShowState* window_state) const { + ui::WindowShowState* window_state) { std::map::const_iterator extension_data_it = cache_.find(extension_id); - // Not in the map means loading data for the extension didn't finish yet. - if (extension_data_it == cache_.end()) - return false; + // Not in the map means loading data for the extension didn't finish yet or + // the cache was not constructed until after the extension was loaded. + // Attempt to load from sync to address the latter case. + if (extension_data_it == cache_.end()) { + LoadGeometryFromStorage(extension_id); + extension_data_it = cache_.find(extension_id); + DCHECK(extension_data_it != cache_.end()); + } ExtensionData::const_iterator window_data = extension_data_it->second.find( window_id); @@ -157,7 +162,7 @@ void ShellWindowGeometryCache::Observe( case chrome::NOTIFICATION_EXTENSION_LOADED: { std::string extension_id = content::Details(details).ptr()->id(); - OnExtensionLoaded(extension_id); + LoadGeometryFromStorage(extension_id); break; } case chrome::NOTIFICATION_EXTENSION_UNLOADED: { @@ -177,7 +182,7 @@ void ShellWindowGeometryCache::SetSyncDelayForTests(int timeout_ms) { sync_delay_ = base::TimeDelta::FromMilliseconds(timeout_ms); } -void ShellWindowGeometryCache::OnExtensionLoaded( +void ShellWindowGeometryCache::LoadGeometryFromStorage( const std::string& extension_id) { ExtensionData& extension_data = cache_[extension_id]; diff --git a/apps/shell_window_geometry_cache.h b/apps/shell_window_geometry_cache.h index 91ea132..334b9fb 100644 --- a/apps/shell_window_geometry_cache.h +++ b/apps/shell_window_geometry_cache.h @@ -77,7 +77,7 @@ class ShellWindowGeometryCache bool GetGeometry(const std::string& extension_id, const std::string& window_id, gfx::Rect* bounds, - ui::WindowShowState* state) const; + ui::WindowShowState* state); // BrowserContextKeyedService virtual void Shutdown() OVERRIDE; @@ -110,7 +110,7 @@ class ShellWindowGeometryCache const content::NotificationSource& source, const content::NotificationDetails& details) OVERRIDE; - void OnExtensionLoaded(const std::string& extension_id); + void LoadGeometryFromStorage(const std::string& extension_id); void OnExtensionUnloaded(const std::string& extension_id); void SyncToStorage(); diff --git a/apps/shell_window_geometry_cache_unittest.cc b/apps/shell_window_geometry_cache_unittest.cc index 8566b4c..85ed137 100644 --- a/apps/shell_window_geometry_cache_unittest.cc +++ b/apps/shell_window_geometry_cache_unittest.cc @@ -79,7 +79,7 @@ void ShellWindowGeometryCacheTest::WaitForSync() { void ShellWindowGeometryCacheTest::LoadExtension( const std::string& extension_id) { - cache_->OnExtensionLoaded(extension_id); + cache_->LoadGeometryFromStorage(extension_id); WaitForSync(); } @@ -171,10 +171,6 @@ TEST_F(ShellWindowGeometryCacheTest, SaveGeometryAndStateToStore) { ASSERT_TRUE(dict->GetInteger(window_id + ".state", &v)); ASSERT_EQ(state, v); - // check to make sure cache indeed doesn't know about this extension anymore - ASSERT_FALSE(cache_->GetGeometry( - extension_id, window_id, &new_bounds, &new_state)); - // reload extension LoadExtension(extension_id); // and make sure the geometry got reloaded properly too -- cgit v1.1