diff --git a/DEPS b/DEPS index 7773e974..641a2e8c 100644 --- a/DEPS +++ b/DEPS
@@ -44,7 +44,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other. - 'v8_revision': '4162e53c387a2e152e6cba1441b42304aa0c9660', + 'v8_revision': 'a7e622ba330d046295b61bf6fa9f296a99aafa77', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling swarming_client # and whatever else without interference from each other.
diff --git a/build/get_landmines.py b/build/get_landmines.py index 05e7b1a5..481240f 100755 --- a/build/get_landmines.py +++ b/build/get_landmines.py
@@ -74,6 +74,7 @@ print 'Clobber to get past mojo gen build error (crbug.com/679607)' if platform() == 'win': print 'Clobber Windows to fix strange PCH-not-rebuilt errors.' + print 'CLobber all to fix GN breakage (crbug.com/736215)' def main(): print_landmines()
diff --git a/chrome/browser/extensions/BUILD.gn b/chrome/browser/extensions/BUILD.gn index 50a96e4..5698009 100644 --- a/chrome/browser/extensions/BUILD.gn +++ b/chrome/browser/extensions/BUILD.gn
@@ -739,10 +739,6 @@ "tab_helper.h", "theme_installed_infobar_delegate.cc", "theme_installed_infobar_delegate.h", - "token_cache/token_cache_service.cc", - "token_cache/token_cache_service.h", - "token_cache/token_cache_service_factory.cc", - "token_cache/token_cache_service_factory.h", "unpacked_installer.cc", "unpacked_installer.h", "update_install_gate.cc",
diff --git a/chrome/browser/extensions/api/identity/identity_api.cc b/chrome/browser/extensions/api/identity/identity_api.cc index 42f1882b..539f8e5 100644 --- a/chrome/browser/extensions/api/identity/identity_api.cc +++ b/chrome/browser/extensions/api/identity/identity_api.cc
@@ -189,12 +189,6 @@ return g_factory.Pointer(); } -void IdentityAPI::OnAccountAdded(const gaia::AccountIds& ids) { -} - -void IdentityAPI::OnAccountRemoved(const gaia::AccountIds& ids) { -} - void IdentityAPI::OnAccountSignInChanged(const gaia::AccountIds& ids, bool is_signed_in) { api::identity::AccountInfo account_info;
diff --git a/chrome/browser/extensions/api/identity/identity_api.h b/chrome/browser/extensions/api/identity/identity_api.h index a0e196c..83a293e 100644 --- a/chrome/browser/extensions/api/identity/identity_api.h +++ b/chrome/browser/extensions/api/identity/identity_api.h
@@ -101,8 +101,6 @@ static BrowserContextKeyedAPIFactory<IdentityAPI>* GetFactoryInstance(); // gaia::AccountTracker::Observer implementation: - void OnAccountAdded(const gaia::AccountIds& ids) override; - void OnAccountRemoved(const gaia::AccountIds& ids) override; void OnAccountSignInChanged(const gaia::AccountIds& ids, bool is_signed_in) override;
diff --git a/chrome/browser/extensions/api/identity/identity_get_auth_token_function.cc b/chrome/browser/extensions/api/identity/identity_get_auth_token_function.cc index 3b4df3f..3c819cc9 100644 --- a/chrome/browser/extensions/api/identity/identity_get_auth_token_function.cc +++ b/chrome/browser/extensions/api/identity/identity_get_auth_token_function.cc
@@ -106,26 +106,11 @@ std::set<std::string> scopes(oauth2_info.scopes.begin(), oauth2_info.scopes.end()); - - std::string account_key = GetPrimaryAccountId(GetProfile()); + std::string gaia_id; if (params->details.get()) { - if (params->details->account.get()) { - std::string detail_key = - AccountTrackerServiceFactory::GetForProfile(GetProfile()) - ->FindAccountInfoByGaiaId(params->details->account->id) - .account_id; - - if (detail_key != account_key) { - if (detail_key.empty() || !switches::IsExtensionsMultiAccount()) { - // TODO(courage): should this be a different error? - error_ = identity_constants::kUserNotSignedIn; - return false; - } - - account_key = detail_key; - } - } + if (params->details->account.get()) + gaia_id = params->details->account->id; if (params->details->scopes.get()) { scopes = std::set<std::string>(params->details->scopes->begin(), @@ -138,12 +123,65 @@ return false; } - token_key_.reset( - new ExtensionTokenKey(extension()->id(), account_key, scopes)); - // From here on out, results must be returned asynchronously. StartAsyncRun(); + GetIdentityManager()->GetPrimaryAccountInfo( + base::Bind(&IdentityGetAuthTokenFunction::OnReceivedPrimaryAccountInfo, + this, scopes, gaia_id)); + + return true; +} + +void IdentityGetAuthTokenFunction::OnReceivedPrimaryAccountInfo( + const std::set<std::string>& scopes, + const std::string& extension_gaia_id, + const base::Optional<AccountInfo>& account_info) { + std::string primary_gaia_id; + if (account_info) + primary_gaia_id = account_info->gaia; + + // Detect and handle the case where the extension is using an account other + // than the primary account. + if (!extension_gaia_id.empty() && extension_gaia_id != primary_gaia_id) { + if (!switches::IsExtensionsMultiAccount()) { + // TODO(courage): should this be a different error? + CompleteFunctionWithError(identity_constants::kUserNotSignedIn); + return; + } + + // Get the AccountInfo for the account that the extension wishes to use. + identity_manager_->GetAccountInfoFromGaiaId( + extension_gaia_id, + base::Bind( + &IdentityGetAuthTokenFunction::OnReceivedExtensionAccountInfo, this, + false /* not primary account */, scopes)); + return; + } + + // The extension is using the primary account. + OnReceivedExtensionAccountInfo(true /* primary account */, scopes, + account_info); +} + +void IdentityGetAuthTokenFunction::OnReceivedExtensionAccountInfo( + bool is_primary_account, + const std::set<std::string>& scopes, + const base::Optional<AccountInfo>& account_info) { + std::string account_id; + if (account_info) + account_id = account_info->account_id; + + if (!is_primary_account && account_id.empty()) { + // It is not possible to sign in the user to an account other than the + // primary account, so just error out here. + CompleteFunctionWithError(identity_constants::kUserNotSignedIn); + return; + } + + token_key_.reset( + new ExtensionTokenKey(extension()->id(), account_id, scopes)); + #if defined(OS_CHROMEOS) policy::BrowserPolicyConnectorChromeOS* connector = g_browser_process->platform_part()->browser_policy_connector_chromeos(); @@ -154,26 +192,24 @@ if (connector->IsEnterpriseManaged() && (is_kiosk || is_public_session)) { if (is_public_session && !IsOriginWhitelistedInPublicSession()) { CompleteFunctionWithError(identity_constants::kUserNotSignedIn); - return true; + return; } StartMintTokenFlow(IdentityMintRequestQueue::MINT_TYPE_NONINTERACTIVE); - return true; + return; } #endif if (!HasLoginToken()) { if (!should_prompt_for_signin_) { CompleteFunctionWithError(identity_constants::kUserNotSignedIn); - return true; + return; } // Display a login prompt. StartSigninFlow(); } else { StartMintTokenFlow(IdentityMintRequestQueue::MINT_TYPE_NONINTERACTIVE); } - - return true; } void IdentityGetAuthTokenFunction::StartAsyncRun() { @@ -539,10 +575,16 @@ signin_flow_.reset(); login_token_request_.reset(); identity_manager_.reset(); - extensions::IdentityAPI::GetFactoryInstance() - ->Get(GetProfile()) - ->mint_queue() - ->RequestCancel(*token_key_, this); + + // Note that if |token_key_| hasn't yet been populated then this instance has + // definitely not made a request with the MintQueue. + if (token_key_) { + extensions::IdentityAPI::GetFactoryInstance() + ->Get(GetProfile()) + ->mint_queue() + ->RequestCancel(*token_key_, this); + } + CompleteFunctionWithError(identity_constants::kCanceled); } @@ -595,8 +637,7 @@ } #endif - ConnectToIdentityManager(); - identity_manager_->GetAccessToken( + GetIdentityManager()->GetAccessToken( token_key_->account_id, ::identity::ScopeSet(), "extensions_identity_api", base::Bind(&IdentityGetAuthTokenFunction::OnGetAccessTokenComplete, base::Unretained(this))); @@ -673,13 +714,14 @@ return client_id; } -void IdentityGetAuthTokenFunction::ConnectToIdentityManager() { - if (identity_manager_.is_bound()) - return; - - content::BrowserContext::GetConnectorFor(GetProfile()) - ->BindInterface(::identity::mojom::kServiceName, - mojo::MakeRequest(&identity_manager_)); +::identity::mojom::IdentityManager* +IdentityGetAuthTokenFunction::GetIdentityManager() { + if (!identity_manager_.is_bound()) { + content::BrowserContext::GetConnectorFor(GetProfile()) + ->BindInterface(::identity::mojom::kServiceName, + mojo::MakeRequest(&identity_manager_)); + } + return identity_manager_.get(); } } // namespace extensions
diff --git a/chrome/browser/extensions/api/identity/identity_get_auth_token_function.h b/chrome/browser/extensions/api/identity/identity_get_auth_token_function.h index a0694545..e766df0 100644 --- a/chrome/browser/extensions/api/identity/identity_get_auth_token_function.h +++ b/chrome/browser/extensions/api/identity/identity_get_auth_token_function.h
@@ -106,6 +106,27 @@ FRIEND_TEST_ALL_PREFIXES(GetAuthTokenFunctionTest, InteractiveQueueShutdown); FRIEND_TEST_ALL_PREFIXES(GetAuthTokenFunctionTest, NoninteractiveShutdown); + // Called by the IdentityManager in response to this class' request for the + // primary account info. Extra arguments that are bound internally at the time + // of calling the IdentityManager: + // |scopes|: The scopes that this instance should use for access token + // requests. + // |extension_gaia_id|: The GAIA ID that was set in the parameters for this + // instance, or empty if this was not in the parameters. + void OnReceivedPrimaryAccountInfo( + const std::set<std::string>& scopes, + const std::string& extension_gaia_id, + const base::Optional<AccountInfo>& account_info); + + // Called when the AccountInfo that this instance should use is available. + // |is_primary_account| is a bool specifying whether the account being used is + // the primary account. |scopes| is the set of scopes that this instance + // should use for access token requests. + void OnReceivedExtensionAccountInfo( + bool is_primary_account, + const std::set<std::string>& scopes, + const base::Optional<AccountInfo>& account_info); + // ExtensionFunction: bool RunAsync() override; @@ -152,8 +173,8 @@ std::string GetOAuth2ClientId() const; - // Connects to the Identity Manager. No-op if already connected. - void ConnectToIdentityManager(); + // Gets the Identity Manager, lazily binding it. + ::identity::mojom::IdentityManager* GetIdentityManager(); bool interactive_; bool should_prompt_for_scopes_;
diff --git a/chrome/browser/extensions/browser_context_keyed_service_factories.cc b/chrome/browser/extensions/browser_context_keyed_service_factories.cc index ff7642d..48f415a 100644 --- a/chrome/browser/extensions/browser_context_keyed_service_factories.cc +++ b/chrome/browser/extensions/browser_context_keyed_service_factories.cc
@@ -48,7 +48,6 @@ #include "chrome/browser/extensions/install_verifier_factory.h" #include "chrome/browser/extensions/menu_manager_factory.h" #include "chrome/browser/extensions/plugin_manager.h" -#include "chrome/browser/extensions/token_cache/token_cache_service_factory.h" #include "chrome/browser/extensions/warning_badge_service_factory.h" #include "chrome/browser/speech/extension_api/tts_extension_api.h" #include "chrome/browser/ui/toolbar/toolbar_actions_model_factory.h" @@ -142,7 +141,6 @@ #if defined(OS_CHROMEOS) file_manager::EventRouterFactory::GetInstance(); #endif - TokenCacheServiceFactory::GetInstance(); ToolbarActionsModelFactory::GetInstance(); extensions::ExtensionGCMAppHandler::GetFactoryInstance(); }
diff --git a/chrome/browser/extensions/token_cache/token_cache_service.cc b/chrome/browser/extensions/token_cache/token_cache_service.cc deleted file mode 100644 index 6ee5422..0000000 --- a/chrome/browser/extensions/token_cache/token_cache_service.cc +++ /dev/null
@@ -1,78 +0,0 @@ -// Copyright (c) 2013 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 "chrome/browser/extensions/token_cache/token_cache_service.h" - -#include "base/logging.h" -#include "chrome/browser/signin/signin_manager_factory.h" -#include "components/signin/core/browser/signin_manager.h" - -using base::Time; -using base::TimeDelta; - -namespace extensions { - -TokenCacheService::TokenCacheService(Profile* profile) : profile_(profile) { - SigninManagerFactory::GetForProfile(profile)->AddObserver(this); -} - -TokenCacheService::~TokenCacheService() { -} - -void TokenCacheService::StoreToken(const std::string& token_name, - const std::string& token_value, - base::TimeDelta time_to_live) { - TokenCacheData token_data; - - // Get the current time, and make sure that the token has not already expired. - Time expiration_time; - TimeDelta zero_delta; - - // Negative time deltas are meaningless to this function. - DCHECK(time_to_live >= zero_delta); - - if (zero_delta < time_to_live) { - expiration_time = Time::Now(); - expiration_time += time_to_live; - } - - token_data.token = token_value; - token_data.expiration_time = expiration_time; - - // Add the token to our cache, overwriting any existing token with this name. - token_cache_[token_name] = token_data; -} - -// Retrieve a token for the currently logged in user. This returns an empty -// string if the token was not found or timed out. -std::string TokenCacheService::RetrieveToken(const std::string& token_name) { - std::map<std::string, TokenCacheData>::iterator it = - token_cache_.find(token_name); - - if (it != token_cache_.end()) { - Time now = Time::Now(); - if (it->second.expiration_time.is_null() || - now < it->second.expiration_time) { - return it->second.token; - } else { - // Remove this entry if it is expired. - token_cache_.erase(it); - return std::string(); - } - } - - return std::string(); -} - -void TokenCacheService::Shutdown() { - SigninManagerFactory::GetForProfile(const_cast<Profile*>(profile_)) - ->RemoveObserver(this); -} - -void TokenCacheService::GoogleSignedOut(const std::string& account_id, - const std::string& username) { - token_cache_.clear(); -} - -} // namespace extensions
diff --git a/chrome/browser/extensions/token_cache/token_cache_service.h b/chrome/browser/extensions/token_cache/token_cache_service.h deleted file mode 100644 index b96afaf..0000000 --- a/chrome/browser/extensions/token_cache/token_cache_service.h +++ /dev/null
@@ -1,67 +0,0 @@ -// Copyright (c) 2013 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 CHROME_BROWSER_EXTENSIONS_TOKEN_CACHE_TOKEN_CACHE_SERVICE_H_ -#define CHROME_BROWSER_EXTENSIONS_TOKEN_CACHE_TOKEN_CACHE_SERVICE_H_ - -#include <map> -#include <string> - -#include "base/compiler_specific.h" -#include "base/gtest_prod_util.h" -#include "base/macros.h" -#include "base/time/time.h" -#include "components/keyed_service/core/keyed_service.h" -#include "components/signin/core/browser/signin_manager_base.h" - -class Profile; - -namespace extensions { - -// This class caches tokens for the current user. It will clear tokens out -// when the user logs out or after the specified timeout interval, or when -// the instance of chrome shuts down. -class TokenCacheService : public KeyedService, - public SigninManagerBase::Observer { - public: - explicit TokenCacheService(Profile* profile); - ~TokenCacheService() override; - - // Store a token for the currently logged in user. We will look it up later by - // the name given here, and we will expire the token after the timeout. For a - // timeout of 0, we never expire the token. After time_to_live expires, the - // token will be expired. - void StoreToken(const std::string& token_name, const std::string& token_value, - base::TimeDelta time_to_live); - - // Retrieve a token for the currently logged in user. This returns an empty - // string if the token was not found or timed out. - std::string RetrieveToken(const std::string& token_name); - - // KeyedService: - void Shutdown() override; - - private: - friend class TokenCacheTest; - FRIEND_TEST_ALL_PREFIXES(TokenCacheTest, SignoutTest); - - // SigninManagerBase::Observer: - void GoogleSignedOut(const std::string& account_id, - const std::string& username) override; - - struct TokenCacheData { - std::string token; - base::Time expiration_time; - }; - - // Map the token name (string) to token data. - std::map<std::string, TokenCacheData> token_cache_; - const Profile* const profile_; - - DISALLOW_COPY_AND_ASSIGN(TokenCacheService); -}; - -} // namespace extensions - -#endif // CHROME_BROWSER_EXTENSIONS_TOKEN_CACHE_TOKEN_CACHE_SERVICE_H_
diff --git a/chrome/browser/extensions/token_cache/token_cache_service_factory.cc b/chrome/browser/extensions/token_cache/token_cache_service_factory.cc deleted file mode 100644 index ee4b399..0000000 --- a/chrome/browser/extensions/token_cache/token_cache_service_factory.cc +++ /dev/null
@@ -1,37 +0,0 @@ -// Copyright (c) 2013 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 "chrome/browser/extensions/token_cache/token_cache_service_factory.h" - -#include "chrome/browser/extensions/token_cache/token_cache_service.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/signin/signin_manager_factory.h" -#include "components/keyed_service/content/browser_context_dependency_manager.h" - -// static -extensions::TokenCacheService* -TokenCacheServiceFactory::GetForProfile(Profile* profile) { - return static_cast<extensions::TokenCacheService*>( - GetInstance()->GetServiceForBrowserContext(profile, true)); - } - -// static -TokenCacheServiceFactory* TokenCacheServiceFactory::GetInstance() { - return base::Singleton<TokenCacheServiceFactory>::get(); -} - -TokenCacheServiceFactory::TokenCacheServiceFactory() - : BrowserContextKeyedServiceFactory( - "TokenCacheService", - BrowserContextDependencyManager::GetInstance()) { - DependsOn(SigninManagerFactory::GetInstance()); -} - -TokenCacheServiceFactory::~TokenCacheServiceFactory() { -} - -KeyedService* TokenCacheServiceFactory::BuildServiceInstanceFor( - content::BrowserContext* profile) const { - return new extensions::TokenCacheService(static_cast<Profile*>(profile)); -}
diff --git a/chrome/browser/extensions/token_cache/token_cache_service_factory.h b/chrome/browser/extensions/token_cache/token_cache_service_factory.h deleted file mode 100644 index ac11defe..0000000 --- a/chrome/browser/extensions/token_cache/token_cache_service_factory.h +++ /dev/null
@@ -1,37 +0,0 @@ -// Copyright (c) 2013 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 CHROME_BROWSER_EXTENSIONS_TOKEN_CACHE_TOKEN_CACHE_SERVICE_FACTORY_H_ -#define CHROME_BROWSER_EXTENSIONS_TOKEN_CACHE_TOKEN_CACHE_SERVICE_FACTORY_H_ - -#include "base/compiler_specific.h" -#include "base/macros.h" -#include "base/memory/singleton.h" -#include "components/keyed_service/content/browser_context_keyed_service_factory.h" - -class Profile; - -namespace extensions { -class TokenCacheService; -} // namespace extensions - -class TokenCacheServiceFactory : public BrowserContextKeyedServiceFactory { - public: - static extensions::TokenCacheService* GetForProfile(Profile* profile); - static TokenCacheServiceFactory* GetInstance(); - - private: - TokenCacheServiceFactory(); - ~TokenCacheServiceFactory() override; - - friend struct base::DefaultSingletonTraits<TokenCacheServiceFactory>; - - // Inherited from BrowserContextKeyedServiceFactory: - KeyedService* BuildServiceInstanceFor( - content::BrowserContext* profile) const override; - - DISALLOW_COPY_AND_ASSIGN(TokenCacheServiceFactory); -}; - -#endif // CHROME_BROWSER_EXTENSIONS_TOKEN_CACHE_TOKEN_CACHE_SERVICE_FACTORY_H_
diff --git a/chrome/browser/extensions/token_cache/token_cache_service_unittest.cc b/chrome/browser/extensions/token_cache/token_cache_service_unittest.cc deleted file mode 100644 index 9734ed7..0000000 --- a/chrome/browser/extensions/token_cache/token_cache_service_unittest.cc +++ /dev/null
@@ -1,112 +0,0 @@ -// Copyright (c) 2013 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 <stddef.h> - -#include "base/compiler_specific.h" -#include "base/time/time.h" -#include "chrome/browser/extensions/token_cache/token_cache_service.h" -#include "chrome/test/base/testing_profile.h" -#include "content/public/test/test_browser_thread_bundle.h" -#include "testing/gtest/include/gtest/gtest.h" - -using base::Time; -using base::TimeDelta; - -namespace extensions { - -class TokenCacheTest : public testing::Test { - public: - TokenCacheTest() : cache_(&profile_) {} - ~TokenCacheTest() override { cache_.Shutdown(); } - - size_t CacheSize() { - return cache_.token_cache_.size(); - } - - bool HasMatch(const std::string& token_name) { - return cache_.RetrieveToken(token_name) != std::string(); - } - - void InsertExpiredToken(const std::string& token_name, - const std::string& token_value) { - EXPECT_TRUE(!HasMatch(token_name)); - - // Compute a time value for yesterday - Time now = Time::Now(); - TimeDelta one_day = one_day.FromDays(1); - Time yesterday = now - one_day; - - TokenCacheService::TokenCacheData token_data; - token_data.token = token_value; - token_data.expiration_time = yesterday; - - cache_.token_cache_[token_name] = token_data; - } - - protected: - content::TestBrowserThreadBundle thread_bundle_; - TestingProfile profile_; - TokenCacheService cache_; -}; - -TEST_F(TokenCacheTest, SaveTokenTest) { - TimeDelta zero; - cache_.StoreToken("foo", "bar", zero); - - EXPECT_EQ(1U, CacheSize()); - EXPECT_TRUE(HasMatch("foo")); -} - -TEST_F(TokenCacheTest, RetrieveTokenTest) { - TimeDelta zero; - cache_.StoreToken("Mozart", "Eine Kleine Nacht Musik", zero); - cache_.StoreToken("Bach", "Brandenburg Concerto #3", zero); - cache_.StoreToken("Beethoven", "Emperor Piano Concerto #5", zero); - cache_.StoreToken("Handel", "Water Music", zero); - cache_.StoreToken("Chopin", "Heroic", zero); - - std::string found_token = cache_.RetrieveToken("Chopin"); - EXPECT_EQ("Heroic", found_token); -} - -TEST_F(TokenCacheTest, ReplaceTokenTest) { - TimeDelta zero; - cache_.StoreToken("Chopin", "Heroic", zero); - - std::string found_token = cache_.RetrieveToken("Chopin"); - EXPECT_EQ("Heroic", found_token); - - cache_.StoreToken("Chopin", "Military", zero); - - found_token = cache_.RetrieveToken("Chopin"); - EXPECT_EQ("Military", found_token); - EXPECT_EQ(1U, CacheSize()); -} - -TEST_F(TokenCacheTest, SignoutTest) { - TimeDelta zero; - cache_.StoreToken("foo", "bar", zero); - - EXPECT_EQ(1U, CacheSize()); - EXPECT_TRUE(HasMatch("foo")); - - cache_.GoogleSignedOut("foo", "foo"); - - EXPECT_EQ(0U, CacheSize()); - EXPECT_FALSE(HasMatch("foo")); -} - -TEST_F(TokenCacheTest, TokenExpireTest) { - // Use the fact that we are friends to insert an expired token. - InsertExpiredToken("foo", "bar"); - - EXPECT_EQ(1U, CacheSize()); - - // If we attempt to find the token, the attempt should fail. - EXPECT_FALSE(HasMatch("foo")); - EXPECT_EQ(0U, CacheSize()); -} - -} // namespace extensions
diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc index 5fca438d..a923357 100644 --- a/chrome/browser/profiles/profile_impl.cc +++ b/chrome/browser/profiles/profile_impl.cc
@@ -75,6 +75,7 @@ #include "chrome/browser/push_messaging/push_messaging_service_impl.h" #include "chrome/browser/safe_browsing/safe_browsing_service.h" #include "chrome/browser/sessions/session_service_factory.h" +#include "chrome/browser/signin/account_tracker_service_factory.h" #include "chrome/browser/signin/profile_oauth2_token_service_factory.h" #include "chrome/browser/signin/signin_manager_factory.h" #include "chrome/browser/signin/signin_ui_util.h" @@ -1415,9 +1416,11 @@ } std::unique_ptr<service_manager::Service> ProfileImpl::CreateIdentityService() { + AccountTrackerService* account_tracker = + AccountTrackerServiceFactory::GetForProfile(this); SigninManagerBase* signin_manager = SigninManagerFactory::GetForProfile(this); ProfileOAuth2TokenService* token_service = ProfileOAuth2TokenServiceFactory::GetForProfile(this); - return base::MakeUnique<identity::IdentityService>(signin_manager, - token_service); + return base::MakeUnique<identity::IdentityService>( + account_tracker, signin_manager, token_service); }
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index 9070ab1..8463fb1 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn
@@ -3988,7 +3988,6 @@ "../browser/extensions/shared_module_service_unittest.cc", "../browser/extensions/standard_management_policy_provider_unittest.cc", "../browser/extensions/tab_helper_unittest.cc", - "../browser/extensions/token_cache/token_cache_service_unittest.cc", "../browser/extensions/update_install_gate_unittest.cc", "../browser/extensions/updater/extension_cache_fake.cc", "../browser/extensions/updater/extension_cache_fake.h",
diff --git a/components/crash/content/browser/crash_dump_manager_android.cc b/components/crash/content/browser/crash_dump_manager_android.cc index c7b7628c..fdcfa65 100644 --- a/components/crash/content/browser/crash_dump_manager_android.cc +++ b/components/crash/content/browser/crash_dump_manager_android.cc
@@ -18,8 +18,8 @@ #include "base/rand_util.h" #include "base/stl_util.h" #include "base/strings/stringprintf.h" +#include "base/task_scheduler/post_task.h" #include "components/crash/content/app/breakpad_linux.h" -#include "content/public/browser/browser_thread.h" #include "content/public/browser/child_process_data.h" #include "content/public/browser/file_descriptor_info.h" #include "content/public/browser/notification_service.h" @@ -27,8 +27,6 @@ #include "content/public/browser/render_process_host.h" #include "jni/CrashDumpManager_jni.h" -using content::BrowserThread; - namespace breakpad { CrashDumpManager::CrashDumpManager(const base::FilePath& crash_dump_dir, @@ -40,8 +38,6 @@ void CrashDumpManager::OnChildStart(int child_process_id, content::FileDescriptorInfo* mappings) { - DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER); - if (!breakpad::IsCrashReporterEnabled()) return; @@ -79,7 +75,6 @@ content::ProcessType process_type, base::TerminationStatus termination_status, base::android::ApplicationState app_state) { - DCHECK_CURRENTLY_ON(BrowserThread::FILE); int64_t file_size = 0; int r = base::GetFileSize(minidump_path, &file_size); DCHECK(r) << "Failed to retrieve size for minidump " @@ -185,8 +180,8 @@ minidump_path = iter->second; child_process_id_to_minidump_path_.erase(iter); } - BrowserThread::PostTask( - BrowserThread::FILE, FROM_HERE, + base::PostTaskWithTraits( + FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND}, base::Bind(&CrashDumpManager::ProcessMinidump, minidump_path, crash_dump_dir_, pid, process_type, termination_status, app_state));
diff --git a/components/crash/content/browser/crash_dump_observer_android.cc b/components/crash/content/browser/crash_dump_observer_android.cc index 72189b1..acb05e0 100644 --- a/components/crash/content/browser/crash_dump_observer_android.cc +++ b/components/crash/content/browser/crash_dump_observer_android.cc
@@ -85,7 +85,6 @@ void CrashDumpObserver::BrowserChildProcessStarted( int child_process_id, content::FileDescriptorInfo* mappings) { - DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER); std::vector<Client*> registered_clients_copy; { base::AutoLock auto_lock(registered_clients_lock_);
diff --git a/components/favicon/core/favicon_handler.cc b/components/favicon/core/favicon_handler.cc index aa57bfcd..46d6235 100644 --- a/components/favicon/core/favicon_handler.cc +++ b/components/favicon/core/favicon_handler.cc
@@ -31,6 +31,7 @@ // Size (along each axis) of a touch icon. This currently corresponds to // the apple touch icon for iPad. +// TODO(crbug.com/736290): Consider changing this to 192x192 for Android. const int kTouchIconSize = 144; // Return true if |bitmap_result| is expired. @@ -347,7 +348,8 @@ // If no manifest available, proceed with the regular candidates only. if (manifest_url_.is_empty()) { - OnGotFinalIconURLCandidates(candidates); + OnGotFinalIconURLCandidates(candidates, + GetDesiredPixelSizes(handler_type_)); return; } @@ -396,7 +398,9 @@ manifest_download_request_.Cancel(); if (!candidates.empty()) { - OnGotFinalIconURLCandidates(candidates); + // When reading icons from web manifests, prefer kNonTouchLargestIconSize. + OnGotFinalIconURLCandidates(candidates, + std::vector<int>(1U, kNonTouchLargestIconSize)); return; } @@ -409,14 +413,14 @@ service_->UnableToDownloadFavicon(manifest_url_); manifest_url_ = GURL(); - OnGotFinalIconURLCandidates(non_manifest_original_candidates_); + OnGotFinalIconURLCandidates(non_manifest_original_candidates_, + GetDesiredPixelSizes(handler_type_)); } void FaviconHandler::OnGotFinalIconURLCandidates( - const std::vector<FaviconURL>& candidates) { + const std::vector<FaviconURL>& candidates, + const std::vector<int>& desired_pixel_sizes) { std::vector<FaviconCandidate> sorted_candidates; - const std::vector<int> desired_pixel_sizes = - GetDesiredPixelSizes(handler_type_); for (const FaviconURL& candidate : candidates) { if (!candidate.icon_url.is_empty() && (candidate.icon_type & icon_types_)) { sorted_candidates.push_back(
diff --git a/components/favicon/core/favicon_handler.h b/components/favicon/core/favicon_handler.h index 5b4c7aa3..610ffbcf 100644 --- a/components/favicon/core/favicon_handler.h +++ b/components/favicon/core/favicon_handler.h
@@ -206,7 +206,8 @@ // Called when the actual list of favicon candidates to be processed is // available, which can be either icon URLs listed in the HTML head instead // or, if a Web Manifest was provided, the list of icons there. - void OnGotFinalIconURLCandidates(const std::vector<FaviconURL>& candidates); + void OnGotFinalIconURLCandidates(const std::vector<FaviconURL>& candidates, + const std::vector<int>& desired_pixel_sizes); // Called when the history request for favicon data mapped to |url_| has // completed and the renderer has told us the icon URLs used by |url_|
diff --git a/components/favicon/core/favicon_handler_unittest.cc b/components/favicon/core/favicon_handler_unittest.cc index be6a075..bd7be8f 100644 --- a/components/favicon/core/favicon_handler_unittest.cc +++ b/components/favicon/core/favicon_handler_unittest.cc
@@ -1567,6 +1567,30 @@ EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL16x16)); } +// Test that icons from a web manifest use a desired size of 192x192. +TEST_F(FaviconHandlerManifestsEnabledTest, Prefer192x192IconFromManifest) { + const GURL kIconURL144x144 = GURL("http://www.google.com/favicon144x144"); + const GURL kIconURL192x192 = GURL("http://www.google.com/favicon192x192"); + + delegate_.fake_image_downloader().Add(kIconURL144x144, IntVector{144}); + delegate_.fake_image_downloader().Add(kIconURL192x192, IntVector{192}); + + const std::vector<favicon::FaviconURL> kManifestIcons = { + FaviconURL(kIconURL144x144, WEB_MANIFEST_ICON, + SizeVector(1U, gfx::Size(144, 144))), + FaviconURL(kIconURL192x192, WEB_MANIFEST_ICON, + SizeVector(1U, gfx::Size(192, 192))), + }; + + delegate_.fake_manifest_downloader().Add(kManifestURL, kManifestIcons); + + RunHandlerWithCandidates(FaviconDriverObserver::TOUCH_LARGEST, + std::vector<favicon::FaviconURL>(), kManifestURL); + + EXPECT_THAT(delegate_.downloads(), + ElementsAre(kManifestURL, kIconURL192x192)); +} + // Test that the manifest and icon are redownloaded if the icon cached for the // page URL expired. TEST_F(FaviconHandlerManifestsEnabledTest, GetFaviconFromExpiredManifest) {
diff --git a/components/gcm_driver/gcm_account_tracker.cc b/components/gcm_driver/gcm_account_tracker.cc index f3be5ff4d..052b0597 100644 --- a/components/gcm_driver/gcm_account_tracker.cc +++ b/components/gcm_driver/gcm_account_tracker.cc
@@ -101,17 +101,6 @@ GetTimeToNextTokenReporting()); } -void GCMAccountTracker::OnAccountAdded(const gaia::AccountIds& ids) { - DVLOG(1) << "Account added: " << ids.email; - // We listen for the account signing in, which happens after account is added. -} - -void GCMAccountTracker::OnAccountRemoved(const gaia::AccountIds& ids) { - DVLOG(1) << "Account removed: " << ids.email; - // We listen for the account signing out, which happens before account is - // removed. -} - void GCMAccountTracker::OnAccountSignInChanged(const gaia::AccountIds& ids, bool is_signed_in) { if (is_signed_in)
diff --git a/components/gcm_driver/gcm_account_tracker.h b/components/gcm_driver/gcm_account_tracker.h index ec72cde..dd968be 100644 --- a/components/gcm_driver/gcm_account_tracker.h +++ b/components/gcm_driver/gcm_account_tracker.h
@@ -96,8 +96,6 @@ typedef std::map<std::string, AccountInfo> AccountInfos; // AccountTracker::Observer overrides. - void OnAccountAdded(const gaia::AccountIds& ids) override; - void OnAccountRemoved(const gaia::AccountIds& ids) override; void OnAccountSignInChanged(const gaia::AccountIds& ids, bool is_signed_in) override;
diff --git a/components/ntp_snippets/remote/remote_suggestions_fetcher_impl_unittest.cc b/components/ntp_snippets/remote/remote_suggestions_fetcher_impl_unittest.cc index 376f1fa..1526cc92e 100644 --- a/components/ntp_snippets/remote/remote_suggestions_fetcher_impl_unittest.cc +++ b/components/ntp_snippets/remote/remote_suggestions_fetcher_impl_unittest.cc
@@ -227,10 +227,10 @@ int id, const GURL& url, net::URLFetcher::RequestType request_type, - net::URLFetcherDelegate* d, + net::URLFetcherDelegate* delegate, net::NetworkTrafficAnnotationTag traffic_annotation) override { return base::MakeUnique<net::FakeURLFetcher>( - url, d, /*response_data=*/std::string(), net::HTTP_NOT_FOUND, + url, delegate, /*response_data=*/std::string(), net::HTTP_NOT_FOUND, net::URLRequestStatus::FAILED); } }; @@ -955,6 +955,8 @@ ToSnippetsAvailableCallback(&mock_callback())); FastForwardUntilNoTasksRemain(); EXPECT_THAT(fetcher().GetLastJsonForDebugging(), Eq(kJsonStr)); + EXPECT_THAT(fetcher().GetLastStatusForDebugging(), + StartsWith("Invalid / empty list")); EXPECT_THAT( histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchResult"), ElementsAre(base::Bucket(/*min=*/5, /*count=*/1))); @@ -965,6 +967,65 @@ Not(IsEmpty())); } +TEST_F(RemoteSuggestionsSignedOutFetcherTest, + ShouldReportInvalidListErrorForIncompleteSuggestionButValidJson) { + // This is valid json, but it does not represent a valid suggestion + // (fullPageUrl is missing). + const std::string kValidJsonStr = + "{\"categories\" : [{" + " \"id\": 1," + " \"localizedTitle\": \"Articles for You\"," + " \"suggestions\" : [{" + " \"ids\" : [\"http://localhost/foobar\"]," + " \"title\" : \"Foo Barred from Baz\"," + " \"snippet\" : \"...\"," + " \"INVALID_fullPageUrl\" : \"http://localhost/foobar\"," + " \"creationTime\" : \"2016-06-30T11:01:37.000Z\"," + " \"expirationTime\" : \"2016-07-01T11:01:37.000Z\"," + " \"attribution\" : \"Foo News\"," + " \"imageUrl\" : \"http://localhost/foobar.jpg\"," + " \"ampUrl\" : \"http://localhost/amp\"," + " \"faviconUrl\" : \"http://localhost/favicon.ico\" " + " }]" + "}]}"; + SetFakeResponse(/*response_data=*/kValidJsonStr, net::HTTP_OK, + net::URLRequestStatus::SUCCESS); + EXPECT_CALL( + mock_callback(), + Run(Field(&Status::code, StatusCode::TEMPORARY_ERROR), + /*fetched_categories=*/Property( + &base::Optional<std::vector<FetchedCategory>>::has_value, false))) + .Times(1); + fetcher().FetchSnippets(test_params(), + ToSnippetsAvailableCallback(&mock_callback())); + FastForwardUntilNoTasksRemain(); + EXPECT_THAT(fetcher().GetLastStatusForDebugging(), + StartsWith("Invalid / empty list")); + EXPECT_THAT( + histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchResult"), + ElementsAre(base::Bucket(/*min=*/5, /*count=*/1))); + EXPECT_THAT(histogram_tester().GetAllSamples( + "NewTabPage.Snippets.FetchHttpResponseOrErrorCode"), + ElementsAre(base::Bucket(/*min=*/200, /*count=*/1))); + EXPECT_THAT(histogram_tester().GetAllSamples("NewTabPage.Snippets.FetchTime"), + Not(IsEmpty())); +} + +TEST_F(RemoteSuggestionsSignedOutFetcherTest, + ShouldReportRequestFailureAsTemporaryError) { + SetFakeResponse(/*response_data=*/std::string(), net::HTTP_NOT_FOUND, + net::URLRequestStatus::FAILED); + EXPECT_CALL( + mock_callback(), + Run(Field(&Status::code, StatusCode::TEMPORARY_ERROR), + /*fetched_categories=*/Property( + &base::Optional<std::vector<FetchedCategory>>::has_value, false))) + .Times(1); + fetcher().FetchSnippets(test_params(), + ToSnippetsAvailableCallback(&mock_callback())); + FastForwardUntilNoTasksRemain(); +} + // This test actually verifies that the test setup itself is sane, to prevent // hard-to-reproduce test failures. TEST_F(RemoteSuggestionsSignedOutFetcherTest,
diff --git a/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc b/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc index d2b95e9..d8760fc7 100644 --- a/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc +++ b/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc
@@ -310,19 +310,6 @@ return json_str.substr(0, json_str.size() - 1); } -std::string GetIncompleteSuggestion() { - std::string json_str = GetSuggestion(); - // Rename the "url" entry. The result is syntactically valid json that will - // fail to parse as suggestions. - size_t pos = json_str.find("\"fullPageUrl\""); - if (pos == std::string::npos) { - NOTREACHED(); - return std::string(); - } - json_str[pos + 1] = 'x'; - return json_str; -} - using ServeImageCallback = base::Callback<void( const std::string&, base::Callback<void(const std::string&, @@ -501,9 +488,10 @@ // TODO(vitaliii): Rewrite this function to initialize a test class member // instead of creating a new provider. std::unique_ptr<RemoteSuggestionsProviderImpl> MakeSuggestionsProvider( - bool set_empty_response = true) { + bool use_mock_suggestions_fetcher, + bool set_empty_response) { auto provider = MakeSuggestionsProviderWithoutInitialization( - /*use_mock_suggestions_fetcher=*/false, + use_mock_suggestions_fetcher, /*use_mock_prefetched_pages_tracker=*/false); WaitForSuggestionsProviderInitialization(provider.get(), set_empty_response); @@ -593,7 +581,8 @@ bool set_empty_response) { provider->reset(); observer_.reset(); - *provider = MakeSuggestionsProvider(set_empty_response); + *provider = MakeSuggestionsProvider(/*use_mock_suggestions_fetcher=*/false, + set_empty_response); } void SetCategoryRanker(std::unique_ptr<CategoryRanker> category_ranker) { @@ -732,7 +721,8 @@ TEST_F(RemoteSuggestionsProviderImplTest, Full) { std::string json_str(GetTestJson({GetSuggestion()})); - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); LoadFromJSONString(provider.get(), json_str); @@ -758,7 +748,8 @@ // Don't send an initial response -- we want to test what happens without any // server status. - auto provider = MakeSuggestionsProvider(/*set_empty_response=*/false); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/false); // The articles category should be there by default, and have a title. CategoryInfo info_before = provider->GetCategoryInfo(articles_category()); @@ -787,7 +778,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, MultipleCategories) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); std::string json_str = MultiCategoryJsonBuilder() .AddCategory({GetSuggestionN(0)}, /*remote_category_id=*/1) @@ -835,7 +827,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, ArticleCategoryInfo) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); CategoryInfo article_info = provider->GetCategoryInfo(articles_category()); EXPECT_THAT(article_info.additional_action(), Eq(ContentSuggestionsAdditionalAction::FETCH)); @@ -843,7 +836,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, ExperimentalCategoryInfo) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); std::string json_str = MultiCategoryJsonBuilder() .AddCategory({GetSuggestionN(0)}, /*remote_category_id=*/1) @@ -880,7 +874,8 @@ EXPECT_CALL(*raw_mock_ranker, AppendCategoryIfNecessary(Category::FromRemoteCategory(12))); } - auto provider = MakeSuggestionsProvider(/*set_empty_response=*/false); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/false); LoadFromJSONString(provider.get(), json_str); } @@ -913,7 +908,8 @@ InsertCategoryAfterIfNecessary(Category::FromRemoteCategory(12), articles_category())); } - auto provider = MakeSuggestionsProvider(/*set_empty_response=*/false); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/false); LoadFromJSONString(provider.get(), json_str); } @@ -932,12 +928,14 @@ EXPECT_CALL(*raw_mock_ranker, InsertCategoryBeforeIfNecessary(_, _)).Times(0); EXPECT_CALL(*raw_mock_ranker, AppendCategoryIfNecessary(Category::FromRemoteCategory(11))); - auto provider = MakeSuggestionsProvider(/*set_empty_response=*/false); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/false); LoadFromJSONString(provider.get(), json_str); } TEST_F(RemoteSuggestionsProviderImplTest, PersistCategoryInfos) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); // TODO(vitaliii): Use |articles_category()| instead of constant ID below. std::string json_str = MultiCategoryJsonBuilder() @@ -989,7 +987,8 @@ .AddCategory({GetSuggestionN(1)}, /*remote_category_id=*/13) .AddCategory({GetSuggestionN(2)}, /*remote_category_id=*/12) .Build(); - auto provider = MakeSuggestionsProvider(/*set_empty_response=*/false); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/false); LoadFromJSONString(provider.get(), json_str); // We manually recreate the provider to simulate Chrome restart and enforce a @@ -1018,7 +1017,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, PersistSuggestions) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); std::string json_str = MultiCategoryJsonBuilder() .AddCategory({GetSuggestionN(0)}, /*remote_category_id=*/1) @@ -1041,7 +1041,8 @@ TEST_F(RemoteSuggestionsProviderImplTest, DontNotifyIfNotAvailable) { // Get some suggestions into the database. - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); std::string json_str = MultiCategoryJsonBuilder() .AddCategory({GetSuggestionN(0)}, @@ -1072,7 +1073,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, Clear) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); std::string json_str(GetTestJson({GetSuggestion()})); @@ -1086,7 +1088,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, ReplaceSuggestions) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); std::string first("http://first"); LoadFromJSONString(provider.get(), @@ -1104,7 +1107,8 @@ TEST_F(RemoteSuggestionsProviderImplTest, ShouldResolveFetchedSuggestionThumbnail) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); LoadFromJSONString(provider.get(), GetTestJson({GetSuggestionWithUrl("http://first")})); @@ -1125,7 +1129,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, ShouldFetchMore) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); LoadFromJSONString(provider.get(), GetTestJson({GetSuggestionWithUrl("http://first")})); @@ -1147,7 +1152,8 @@ TEST_F(RemoteSuggestionsProviderImplTest, ShouldResolveFetchedMoreSuggestionThumbnail) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); auto assert_only_first_suggestion_received = base::Bind([](Status status, std::vector<ContentSuggestion> suggestions) { @@ -1323,7 +1329,8 @@ TEST_F(RemoteSuggestionsProviderImplTest, ClearHistoryShouldDeleteArchivedSuggestions) { - auto provider = MakeSuggestionsProvider(/*set_empty_response=*/false); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/false); // First get suggestions into the archived state which happens through // subsequent fetches. Then we verify the entries are gone from the 'archived' // state by trying to load their images (and we shouldn't even know the URLs @@ -1385,117 +1392,95 @@ base::RunLoop().RunUntilIdle(); } -TEST_F(RemoteSuggestionsProviderImplTest, ReturnTemporaryErrorForInvalidJson) { - auto provider = MakeSuggestionsProvider(); - - MockFunction<void(Status, const std::vector<ContentSuggestion>&)> loaded; - EXPECT_CALL(loaded, Call(Field(&Status::code, StatusCode::TEMPORARY_ERROR), - IsEmpty())); - LoadMoreFromJSONString(provider.get(), articles_category(), - "invalid json string}]}", - /*known_ids=*/std::set<std::string>(), - base::Bind(&SuggestionsLoaded, &loaded)); - EXPECT_THAT(suggestions_fetcher()->GetLastStatusForDebugging(), - StartsWith("Received invalid JSON")); -} - TEST_F(RemoteSuggestionsProviderImplTest, - ReturnTemporaryErrorForInvalidSuggestion) { - auto provider = MakeSuggestionsProvider(); + ShouldForwardTemporaryErrorFromFetcher) { + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/true, /*set_empty_response=*/true); + auto* mock_fetcher = static_cast<StrictMock<MockRemoteSuggestionsFetcher>*>( + suggestions_fetcher()); + RemoteSuggestionsFetcher::SnippetsAvailableCallback snippets_callback; MockFunction<void(Status, const std::vector<ContentSuggestion>&)> loaded; - EXPECT_CALL(loaded, Call(Field(&Status::code, StatusCode::TEMPORARY_ERROR), - IsEmpty())); - LoadMoreFromJSONString(provider.get(), articles_category(), - GetTestJson({GetIncompleteSuggestion()}), - /*known_ids=*/std::set<std::string>(), - base::Bind(&SuggestionsLoaded, &loaded)); - EXPECT_THAT(suggestions_fetcher()->GetLastStatusForDebugging(), - StartsWith("Invalid / empty list")); -} - -TEST_F(RemoteSuggestionsProviderImplTest, - ReturnTemporaryErrorForRequestFailure) { - // Created SuggestionsProvider will fail by default with unsuccessful request. - auto provider = MakeSuggestionsProvider(/*set_empty_response=*/false); - - MockFunction<void(Status, const std::vector<ContentSuggestion>&)> loaded; - EXPECT_CALL(loaded, Call(Field(&Status::code, StatusCode::TEMPORARY_ERROR), - IsEmpty())); + EXPECT_CALL(*mock_fetcher, FetchSnippets(_, _)) + .WillOnce(MoveSecondArgumentPointeeTo(&snippets_callback)); + EXPECT_CALL(*scheduler(), AcquireQuotaForInteractiveFetch()) + .WillOnce(Return(true)) + .RetiresOnSaturation(); provider->Fetch(articles_category(), /*known_ids=*/std::set<std::string>(), base::Bind(&SuggestionsLoaded, &loaded)); - base::RunLoop().RunUntilIdle(); -} -TEST_F(RemoteSuggestionsProviderImplTest, ReturnTemporaryErrorForHttpFailure) { - auto provider = MakeSuggestionsProvider(); - SetUpHttpError(); - - MockFunction<void(Status, const std::vector<ContentSuggestion>&)> loaded; EXPECT_CALL(loaded, Call(Field(&Status::code, StatusCode::TEMPORARY_ERROR), IsEmpty())); - provider->Fetch(articles_category(), - /*known_ids=*/std::set<std::string>(), - base::Bind(&SuggestionsLoaded, &loaded)); - base::RunLoop().RunUntilIdle(); + ASSERT_FALSE(snippets_callback.is_null()); + std::move(snippets_callback) + .Run(Status(StatusCode::TEMPORARY_ERROR, "Received invalid JSON"), + base::nullopt); } -TEST_F(RemoteSuggestionsProviderImplTest, LoadInvalidJson) { - auto provider = MakeSuggestionsProvider(); +TEST_F(RemoteSuggestionsProviderImplTest, + ShouldNotAddNewSuggestionsAfterFetchError) { + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/true, /*set_empty_response=*/true); + auto* mock_fetcher = static_cast<StrictMock<MockRemoteSuggestionsFetcher>*>( + suggestions_fetcher()); - LoadFromJSONString(provider.get(), GetTestJson({GetInvalidSuggestion()})); - EXPECT_THAT(suggestions_fetcher()->GetLastStatusForDebugging(), - StartsWith("Received invalid JSON")); + RemoteSuggestionsFetcher::SnippetsAvailableCallback snippets_callback; + MockFunction<void(Status, const std::vector<ContentSuggestion>&)> loaded; + EXPECT_CALL(*mock_fetcher, FetchSnippets(_, _)) + .WillOnce(MoveSecondArgumentPointeeTo(&snippets_callback)); + FetchSuggestions(provider.get(), /*interactive_request=*/false, + RemoteSuggestionsProvider::FetchStatusCallback()); + std::move(snippets_callback) + .Run(Status(StatusCode::TEMPORARY_ERROR, "Received invalid JSON"), + base::nullopt); EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), IsEmpty()); } TEST_F(RemoteSuggestionsProviderImplTest, - LoadInvalidJsonWithExistingSuggestions) { - auto provider = MakeSuggestionsProvider(); + ShouldNotClearOldSuggestionsAfterFetchError) { + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/true, /*set_empty_response=*/true); + auto* mock_fetcher = static_cast<StrictMock<MockRemoteSuggestionsFetcher>*>( + suggestions_fetcher()); - LoadFromJSONString(provider.get(), GetTestJson({GetSuggestion()})); - ASSERT_THAT(provider->GetSuggestionsForTesting(articles_category()), - SizeIs(1)); - ASSERT_EQ("OK", suggestions_fetcher()->GetLastStatusForDebugging()); + std::vector<FetchedCategory> fetched_categories; + fetched_categories.push_back(FetchedCategory( + articles_category(), + BuildRemoteCategoryInfo(base::UTF8ToUTF16("title"), + /*allow_fetching_more_results=*/true))); + fetched_categories[0].suggestions.push_back( + CreateTestRemoteSuggestion(base::StringPrintf("http://abc.com/"))); + RemoteSuggestionsFetcher::SnippetsAvailableCallback snippets_callback; + EXPECT_CALL(*mock_fetcher, FetchSnippets(_, _)) + .WillOnce(MoveSecondArgumentPointeeTo(&snippets_callback)); + FetchSuggestions(provider.get(), /*interactive_request=*/false, + RemoteSuggestionsProvider::FetchStatusCallback()); + std::move(snippets_callback) + .Run(Status(StatusCode::SUCCESS, "success message"), + std::move(fetched_categories)); - LoadFromJSONString(provider.get(), GetTestJson({GetInvalidSuggestion()})); - EXPECT_THAT(suggestions_fetcher()->GetLastStatusForDebugging(), - StartsWith("Received invalid JSON")); + ASSERT_THAT( + provider->GetSuggestionsForTesting(articles_category()), + ElementsAre(Pointee(Property(&RemoteSuggestion::id, "http://abc.com/")))); + + EXPECT_CALL(*mock_fetcher, FetchSnippets(_, _)) + .WillOnce(MoveSecondArgumentPointeeTo(&snippets_callback)); + FetchSuggestions(provider.get(), /*interactive_request=*/false, + RemoteSuggestionsProvider::FetchStatusCallback()); + std::move(snippets_callback) + .Run(Status(StatusCode::TEMPORARY_ERROR, "Received invalid JSON"), + base::nullopt); // This should not have changed the existing suggestions. - EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), - SizeIs(1)); -} - -TEST_F(RemoteSuggestionsProviderImplTest, LoadIncompleteJson) { - auto provider = MakeSuggestionsProvider(); - - LoadFromJSONString(provider.get(), GetTestJson({GetIncompleteSuggestion()})); - EXPECT_EQ("Invalid / empty list.", - suggestions_fetcher()->GetLastStatusForDebugging()); - EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), - IsEmpty()); -} - -TEST_F(RemoteSuggestionsProviderImplTest, - LoadIncompleteJsonWithExistingSuggestions) { - auto provider = MakeSuggestionsProvider(); - - LoadFromJSONString(provider.get(), GetTestJson({GetSuggestion()})); - ASSERT_THAT(provider->GetSuggestionsForTesting(articles_category()), - SizeIs(1)); - - LoadFromJSONString(provider.get(), GetTestJson({GetIncompleteSuggestion()})); - EXPECT_EQ("Invalid / empty list.", - suggestions_fetcher()->GetLastStatusForDebugging()); - // This should not have changed the existing suggestions. - EXPECT_THAT(provider->GetSuggestionsForTesting(articles_category()), - SizeIs(1)); + EXPECT_THAT( + provider->GetSuggestionsForTesting(articles_category()), + ElementsAre(Pointee(Property(&RemoteSuggestion::id, "http://abc.com/")))); } TEST_F(RemoteSuggestionsProviderImplTest, Dismiss) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); std::string json_str(GetTestJson( {GetSuggestionWithSources("http://site.com", "Source 1", "")})); @@ -1554,7 +1539,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, GetDismissed) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); LoadFromJSONString(provider.get(), GetTestJson({GetSuggestion()})); @@ -1589,7 +1575,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, CreationTimestampParseFail) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); std::string json = GetSuggestionWithTimes(GetDefaultCreationTime(), GetDefaultExpirationTime()); @@ -1603,7 +1590,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, RemoveExpiredDismissedContent) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); std::string json_str1(GetTestJson({GetExpiredSuggestion()})); // Load it. @@ -1637,7 +1625,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, ExpiredContentNotRemoved) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); std::string json_str(GetTestJson({GetExpiredSuggestion()})); @@ -1647,7 +1636,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, TestSingleSource) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); std::string json_str(GetTestJson({GetSuggestionWithSources( "http://source1.com", "Source 1", "http://source1.amp.com")})); @@ -1664,7 +1654,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, TestSingleSourceWithMalformedUrl) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); std::string json_str(GetTestJson({GetSuggestionWithSources( "ceci n'est pas un url", "Source 1", "http://source1.amp.com")})); @@ -1675,7 +1666,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, TestSingleSourceWithMissingData) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); std::string json_str( GetTestJson({GetSuggestionWithSources("http://source1.com", "", "")})); @@ -1686,7 +1678,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, LogNumArticlesHistogram) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); base::HistogramTester tester; LoadFromJSONString(provider.get(), GetTestJson({GetInvalidSuggestion()})); @@ -1743,7 +1736,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, DismissShouldRespectAllKnownUrls) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); const base::Time creation = GetDefaultCreationTime(); const base::Time expiry = GetDefaultExpirationTime(); @@ -1777,7 +1771,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, ImageReturnedWithTheSameId) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); LoadFromJSONString(provider.get(), GetTestJson({GetSuggestion()})); @@ -1802,7 +1797,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, EmptyImageReturnedForNonExistentId) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); // Create a non-empty image so that we can test the image gets updated. gfx::Image image = gfx::test::CreateImage(1, 1); @@ -1823,7 +1819,8 @@ // Testing that the provider is not accessing the database is tricky. // Therefore, we simply put in some data making sure that if the provider asks // the database, it will get a wrong answer. - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); ContentSuggestion::ID unknown_id = MakeArticleID(kSuggestionUrl2); database()->SaveImage(unknown_id.id_within_category(), "some image blob"); @@ -1845,7 +1842,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, ClearHistoryRemovesAllSuggestions) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); std::string first_suggestion = GetSuggestionWithUrl("http://url1.com"); std::string second_suggestion = GetSuggestionWithUrl("http://url2.com"); @@ -1876,7 +1874,8 @@ ShouldKeepArticlesCategoryAvailableAfterClearHistory) { // If the provider marks that category as NOT_PROVIDED, then it won't be shown // at all in the UI and the user cannot load new data :-/. - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); ASSERT_THAT(observer().StatusForCategory(articles_category()), Eq(CategoryStatus::AVAILABLE)); @@ -1888,7 +1887,8 @@ } TEST_F(RemoteSuggestionsProviderImplTest, ShouldClearOrphanedImagesOnRestart) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); LoadFromJSONString(provider.get(), GetTestJson({GetSuggestion()})); ServeImageCallback cb = @@ -1918,7 +1918,8 @@ TEST_F(RemoteSuggestionsProviderImplTest, ShouldHandleMoreThanMaxSuggestionsInResponse) { - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); std::vector<std::string> suggestions; for (int i = 0; i < provider->GetMaxSuggestionCountForTesting() + 1; ++i) { @@ -2374,7 +2375,8 @@ TEST_F(RemoteSuggestionsProviderImplTest, ShouldFetchNormallyWithoutPrefetchedPagesTracker) { EnableKeepingPrefetchedContentSuggestions(); - auto provider = MakeSuggestionsProvider(); + auto provider = MakeSuggestionsProvider( + /*use_mock_suggestions_fetcher=*/false, /*set_empty_response=*/true); LoadFromJSONString(provider.get(), GetTestJson({GetSuggestion()})); EXPECT_THAT(observer().SuggestionsForCategory(articles_category()), SizeIs(1));
diff --git a/content/browser/service_worker/service_worker_context_request_handler.cc b/content/browser/service_worker/service_worker_context_request_handler.cc index 364e41f7..13b4c483 100644 --- a/content/browser/service_worker/service_worker_context_request_handler.cc +++ b/content/browser/service_worker/service_worker_context_request_handler.cc
@@ -21,26 +21,6 @@ namespace content { -namespace { - -bool IsInstalled(const ServiceWorkerVersion* version) { - switch (version->status()) { - case ServiceWorkerVersion::NEW: - case ServiceWorkerVersion::INSTALLING: - return false; - case ServiceWorkerVersion::INSTALLED: - case ServiceWorkerVersion::ACTIVATING: - case ServiceWorkerVersion::ACTIVATED: - return true; - case ServiceWorkerVersion::REDUNDANT: - return false; - } - NOTREACHED(); - return false; -} - -} // namespace - ServiceWorkerContextRequestHandler::ServiceWorkerContextRequestHandler( base::WeakPtr<ServiceWorkerContextCore> context, base::WeakPtr<ServiceWorkerProviderHost> provider_host, @@ -110,7 +90,8 @@ MaybeCreateJobImpl(request, network_delegate, &status); const bool is_main_script = resource_type_ == RESOURCE_TYPE_SERVICE_WORKER; ServiceWorkerMetrics::RecordContextRequestHandlerStatus( - status, IsInstalled(version_.get()), is_main_script); + status, ServiceWorkerVersion::IsInstalled(version_->status()), + is_main_script); if (job) return job; @@ -170,7 +151,7 @@ int resource_id = version_->script_cache_map()->LookupResourceId(request->url()); if (resource_id != kInvalidServiceWorkerResourceId) { - if (IsInstalled(version_.get())) { + if (ServiceWorkerVersion::IsInstalled(version_->status())) { // An installed worker is loading a stored script. if (is_main_script) version_->embedded_worker()->OnURLJobCreatedForMainScript(); @@ -186,7 +167,7 @@ } // An installed worker is importing a non-stored script. - if (IsInstalled(version_.get())) { + if (ServiceWorkerVersion::IsInstalled(version_->status())) { DCHECK(!is_main_script); *out_status = CreateJobStatus::ERROR_UNINSTALLED_SCRIPT_IMPORT; return nullptr;
diff --git a/content/browser/service_worker/service_worker_provider_host.cc b/content/browser/service_worker/service_worker_provider_host.cc index ae8dcdbd..0f5c67b 100644 --- a/content/browser/service_worker/service_worker_provider_host.cc +++ b/content/browser/service_worker/service_worker_provider_host.cc
@@ -100,13 +100,11 @@ context->RemoveProviderHost(process_id, provider_id); } -// Used by a Service Worker for script loading (for all script loading for now, -// but to be used only during installation once script streaming lands). -// For now this is just a proxy loader for the network loader. +// Used by a Service Worker for script loading only during the installation +// time. For now this is just a proxy loader for the network loader. // Eventually this should replace the existing URLRequestJob-based request // interception for script loading, namely ServiceWorkerWriteToCacheJob. -// TODO(kinuko): Implement this. Hook up the existing code in -// ServiceWorkerContextRequestHandler. +// TODO(kinuko): Implement this. class ScriptURLLoader : public mojom::URLLoader, public mojom::URLLoaderClient { public: ScriptURLLoader( @@ -226,6 +224,16 @@ mojom::URLLoaderClientPtr client, const net::MutableNetworkTrafficAnnotationTag& traffic_annotation) override { + if (!ShouldHandleScriptRequest(resource_request)) { + // If the request should not be handled by ScriptURLLoader, just + // fallback to the network. + // TODO(kinuko): Record the reason like what we do with netlog in + // ServiceWorkerContextRequestHandler. + loader_factory_getter_->GetNetworkFactory()->get()->CreateLoaderAndStart( + std::move(request), routing_id, request_id, options, resource_request, + std::move(client), traffic_annotation); + return; + } mojo::MakeStrongAssociatedBinding( base::MakeUnique<ScriptURLLoader>( routing_id, request_id, options, resource_request, @@ -242,6 +250,53 @@ } private: + bool ShouldHandleScriptRequest(const ResourceRequest& resource_request) { + if (!context_ || !provider_host_) + return false; + + // We only use the script cache for main script loading and + // importScripts(), even if a cached script is xhr'd, we don't + // retrieve it from the script cache. + if (resource_request.resource_type != RESOURCE_TYPE_SERVICE_WORKER && + resource_request.resource_type != RESOURCE_TYPE_SCRIPT) { + // TODO: Record bad message, we shouldn't come here for other + // request types. + return false; + } + + scoped_refptr<ServiceWorkerVersion> version = + provider_host_->running_hosted_version(); + + // This could happen if browser-side has set the status to redundant but + // the worker has not yet stopped. The worker is already doomed so just + // reject the request. Handle it specially here because otherwise it'd be + // unclear whether "REDUNDANT" should count as installed or not installed + // when making decisions about how to handle the request and logging UMA. + if (!version || version->status() == ServiceWorkerVersion::REDUNDANT) + return false; + + // TODO: Make sure we don't handle the redirected request. + + // For installed worker we fallback to the network for now. + // TODO: Make sure we don't come here for installed worker once + // script streaming is enabled. + if (ServiceWorkerVersion::IsInstalled(version->status())) + return false; + + // TODO: Make sure we come here only for new / unknown scripts + // once script streaming manager in the renderer side stops sending + // resource requests for the known script URLs, i.e. add DCHECK for + // version->script_cache_map()->LookupResourceId(url) == + // kInvalidServiceWorkerResourceId. + // + // Currently this could be false for the installing worker that imports + // the same script twice (e.g. importScripts('dupe.js'); + // importScripts('dupe.js');). + + // Request should be served by ScriptURLLoader. + return true; + } + base::WeakPtr<ServiceWorkerContextCore> context_; base::WeakPtr<ServiceWorkerProviderHost> provider_host_; base::WeakPtr<storage::BlobStorageContext> blob_storage_context_;
diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc index cf2c4e0..8767d26 100644 --- a/content/browser/service_worker/service_worker_version.cc +++ b/content/browser/service_worker/service_worker_version.cc
@@ -135,21 +135,6 @@ *time = base::TimeTicks(); } -bool IsInstalled(ServiceWorkerVersion::Status status) { - switch (status) { - case ServiceWorkerVersion::NEW: - case ServiceWorkerVersion::INSTALLING: - case ServiceWorkerVersion::REDUNDANT: - return false; - case ServiceWorkerVersion::INSTALLED: - case ServiceWorkerVersion::ACTIVATING: - case ServiceWorkerVersion::ACTIVATED: - return true; - } - NOTREACHED() << "Unexpected status: " << status; - return false; -} - std::string VersionStatusToString(ServiceWorkerVersion::Status status) { switch (status) { case ServiceWorkerVersion::NEW: @@ -1076,6 +1061,21 @@ provider_host_by_uuid.second->CountFeature(feature); } +bool ServiceWorkerVersion::IsInstalled(ServiceWorkerVersion::Status status) { + switch (status) { + case ServiceWorkerVersion::NEW: + case ServiceWorkerVersion::INSTALLING: + case ServiceWorkerVersion::REDUNDANT: + return false; + case ServiceWorkerVersion::INSTALLED: + case ServiceWorkerVersion::ACTIVATING: + case ServiceWorkerVersion::ACTIVATED: + return true; + } + NOTREACHED() << "Unexpected status: " << status; + return false; +} + void ServiceWorkerVersion::OnOpenNewTab(int request_id, const GURL& url) { OnOpenWindow(request_id, url, WindowOpenDisposition::NEW_FOREGROUND_TAB); }
diff --git a/content/browser/service_worker/service_worker_version.h b/content/browser/service_worker/service_worker_version.h index 5ed49bf..9e03f59 100644 --- a/content/browser/service_worker/service_worker_version.h +++ b/content/browser/service_worker/service_worker_version.h
@@ -403,6 +403,8 @@ } const std::set<uint32_t>& used_features() const { return used_features_; } + static bool IsInstalled(ServiceWorkerVersion::Status status); + private: friend class base::RefCounted<ServiceWorkerVersion>; friend class ServiceWorkerReadFromCacheJobTest;
diff --git a/content/child/blink_platform_impl.cc b/content/child/blink_platform_impl.cc index 5a261cae..71bf19c 100644 --- a/content/child/blink_platform_impl.cc +++ b/content/child/blink_platform_impl.cc
@@ -44,6 +44,7 @@ #include "content/child/notifications/notification_manager.h" #include "content/child/push_messaging/push_provider.h" #include "content/child/thread_safe_sender.h" +#include "content/child/web_data_consumer_handle_impl.h" #include "content/child/web_url_loader_impl.h" #include "content/child/web_url_request_util.h" #include "content/child/worker_thread_registry.h" @@ -394,6 +395,12 @@ BlinkPlatformImpl::~BlinkPlatformImpl() { } +std::unique_ptr<blink::WebDataConsumerHandle> +BlinkPlatformImpl::CreateDataConsumerHandle( + mojo::ScopedDataPipeConsumerHandle handle) { + return base::MakeUnique<WebDataConsumerHandleImpl>(std::move(handle)); +} + WebString BlinkPlatformImpl::UserAgent() { return blink::WebString::FromUTF8(GetContentClient()->GetUserAgent()); }
diff --git a/content/child/blink_platform_impl.h b/content/child/blink_platform_impl.h index 43a4aa5..f103da92 100644 --- a/content/child/blink_platform_impl.h +++ b/content/child/blink_platform_impl.h
@@ -78,6 +78,8 @@ size_t MaxDecodedImageBytes() override; uint32_t GetUniqueIdForProcess() override; + std::unique_ptr<blink::WebDataConsumerHandle> CreateDataConsumerHandle( + mojo::ScopedDataPipeConsumerHandle handle) override; blink::WebString UserAgent() override; blink::WebURLError CancelledError(const blink::WebURL& url) const override; std::unique_ptr<blink::WebThread> CreateThread(const char* name) override;
diff --git a/content/renderer/service_worker/service_worker_context_client.cc b/content/renderer/service_worker/service_worker_context_client.cc index 5dab871f..768e9232 100644 --- a/content/renderer/service_worker/service_worker_context_client.cc +++ b/content/renderer/service_worker/service_worker_context_client.cc
@@ -75,6 +75,8 @@ #include "third_party/WebKit/public/web/modules/serviceworker/WebServiceWorkerContextClient.h" #include "third_party/WebKit/public/web/modules/serviceworker/WebServiceWorkerContextProxy.h" +using blink::WebURLRequest; + namespace content { namespace { @@ -93,21 +95,30 @@ // Blink calls this method for each request starting with the main script, // we tag them with the provider id. - void WillSendRequest(blink::WebURLRequest& request) override { + void WillSendRequest(WebURLRequest& request) override { std::unique_ptr<RequestExtraData> extra_data(new RequestExtraData); extra_data->set_service_worker_provider_id(provider_->provider_id()); extra_data->set_originated_from_service_worker(true); // Service workers are only available in secure contexts, so all requests // are initiated in a secure context. extra_data->set_initiated_in_secure_context(true); - extra_data->set_url_loader_factory_override( - provider_->script_loader_factory()); + if (IsScriptRequest(request)) { + extra_data->set_url_loader_factory_override( + provider_->script_loader_factory()); + } request.SetExtraData(extra_data.release()); } int GetProviderID() const override { return provider_->provider_id(); } private: + static bool IsScriptRequest(const WebURLRequest& request) { + auto request_context = request.GetRequestContext(); + return request_context == WebURLRequest::kRequestContextServiceWorker || + request_context == WebURLRequest::kRequestContextScript || + request_context == WebURLRequest::kRequestContextImport; + } + std::unique_ptr<ServiceWorkerNetworkProvider> provider_; }; @@ -139,31 +150,28 @@ return SERVICE_WORKER_ERROR_FAILED; } -blink::WebURLRequest::FetchRequestMode GetBlinkFetchRequestMode( +WebURLRequest::FetchRequestMode GetBlinkFetchRequestMode( FetchRequestMode mode) { - return static_cast<blink::WebURLRequest::FetchRequestMode>(mode); + return static_cast<WebURLRequest::FetchRequestMode>(mode); } -blink::WebURLRequest::FetchCredentialsMode GetBlinkFetchCredentialsMode( +WebURLRequest::FetchCredentialsMode GetBlinkFetchCredentialsMode( FetchCredentialsMode credentials_mode) { - return static_cast<blink::WebURLRequest::FetchCredentialsMode>( - credentials_mode); + return static_cast<WebURLRequest::FetchCredentialsMode>(credentials_mode); } -blink::WebURLRequest::FetchRedirectMode GetBlinkFetchRedirectMode( +WebURLRequest::FetchRedirectMode GetBlinkFetchRedirectMode( FetchRedirectMode redirect_mode) { - return static_cast<blink::WebURLRequest::FetchRedirectMode>(redirect_mode); + return static_cast<WebURLRequest::FetchRedirectMode>(redirect_mode); } -blink::WebURLRequest::RequestContext GetBlinkRequestContext( +WebURLRequest::RequestContext GetBlinkRequestContext( RequestContextType request_context_type) { - return static_cast<blink::WebURLRequest::RequestContext>( - request_context_type); + return static_cast<WebURLRequest::RequestContext>(request_context_type); } -blink::WebURLRequest::FrameType GetBlinkFrameType( - RequestContextFrameType frame_type) { - return static_cast<blink::WebURLRequest::FrameType>(frame_type); +WebURLRequest::FrameType GetBlinkFrameType(RequestContextFrameType frame_type) { + return static_cast<WebURLRequest::FrameType>(frame_type); } blink::WebServiceWorkerClientInfo
diff --git a/google_apis/gaia/account_tracker.cc b/google_apis/gaia/account_tracker.cc index 6978ffa..978fb93 100644 --- a/google_apis/gaia/account_tracker.cc +++ b/google_apis/gaia/account_tracker.cc
@@ -139,18 +139,6 @@ } } -void AccountTracker::NotifyAccountAdded(const AccountState& account) { - DCHECK(!account.ids.gaia.empty()); - for (auto& observer : observer_list_) - observer.OnAccountAdded(account.ids); -} - -void AccountTracker::NotifyAccountRemoved(const AccountState& account) { - DCHECK(!account.ids.gaia.empty()); - for (auto& observer : observer_list_) - observer.OnAccountRemoved(account.ids); -} - void AccountTracker::NotifySignInChanged(const AccountState& account) { // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460 is // fixed. @@ -207,7 +195,6 @@ AccountState& account = accounts_[account_key]; if (!account.ids.gaia.empty()) { UpdateSignInState(account_key, false); - NotifyAccountRemoved(account); } accounts_.erase(account_key); } @@ -268,7 +255,6 @@ AccountState& account = accounts_[account_key]; account.ids.gaia = gaia_id; - NotifyAccountAdded(account); if (account.is_signed_in) NotifySignInChanged(account);
diff --git a/google_apis/gaia/account_tracker.h b/google_apis/gaia/account_tracker.h index 7ff169fa..19aa28f 100644 --- a/google_apis/gaia/account_tracker.h +++ b/google_apis/gaia/account_tracker.h
@@ -49,8 +49,6 @@ class Observer { public: - virtual void OnAccountAdded(const AccountIds& ids) = 0; - virtual void OnAccountRemoved(const AccountIds& ids) = 0; virtual void OnAccountSignInChanged(const AccountIds& ids, bool is_signed_in) = 0; }; @@ -92,8 +90,6 @@ bool is_signed_in; }; - void NotifyAccountAdded(const AccountState& account); - void NotifyAccountRemoved(const AccountState& account); void NotifySignInChanged(const AccountState& account); void UpdateSignInState(const std::string& account_key, bool is_signed_in);
diff --git a/google_apis/gaia/account_tracker_unittest.cc b/google_apis/gaia/account_tracker_unittest.cc index 97d5356d..c4451c18 100644 --- a/google_apis/gaia/account_tracker_unittest.cc +++ b/google_apis/gaia/account_tracker_unittest.cc
@@ -23,8 +23,6 @@ const char kPrimaryAccountKey[] = "primary_account@example.com"; enum TrackingEventType { - ADDED, - REMOVED, SIGN_IN, SIGN_OUT }; @@ -56,12 +54,6 @@ std::string ToString() const { const char * typestr = "INVALID"; switch (type_) { - case ADDED: - typestr = "ADD"; - break; - case REMOVED: - typestr = "REM"; - break; case SIGN_IN: typestr = " IN"; break; @@ -136,8 +128,6 @@ void SortEventsByUser(); // AccountTracker::Observer implementation - void OnAccountAdded(const AccountIds& ids) override; - void OnAccountRemoved(const AccountIds& ids) override; void OnAccountSignInChanged(const AccountIds& ids, bool is_signed_in) override; @@ -148,14 +138,6 @@ std::vector<TrackingEvent> events_; }; -void AccountTrackerObserver::OnAccountAdded(const AccountIds& ids) { - events_.push_back(TrackingEvent(ADDED, ids.email, ids.gaia)); -} - -void AccountTrackerObserver::OnAccountRemoved(const AccountIds& ids) { - events_.push_back(TrackingEvent(REMOVED, ids.email, ids.gaia)); -} - void AccountTrackerObserver::OnAccountSignInChanged(const AccountIds& ids, bool is_signed_in) { events_.push_back( @@ -397,8 +379,7 @@ ReturnOAuthUrlFetchSuccess(kPrimaryAccountKey); EXPECT_TRUE( - observer()->CheckEvents(TrackingEvent(ADDED, kPrimaryAccountKey), - TrackingEvent(SIGN_IN, kPrimaryAccountKey))); + observer()->CheckEvents(TrackingEvent(SIGN_IN, kPrimaryAccountKey))); } TEST_F(IdentityAccountTrackerTest, PrimaryTokenAvailableThenLogin) { @@ -408,8 +389,7 @@ NotifyLogin(kPrimaryAccountKey); ReturnOAuthUrlFetchSuccess(kPrimaryAccountKey); EXPECT_TRUE( - observer()->CheckEvents(TrackingEvent(ADDED, kPrimaryAccountKey), - TrackingEvent(SIGN_IN, kPrimaryAccountKey))); + observer()->CheckEvents(TrackingEvent(SIGN_IN, kPrimaryAccountKey))); } TEST_F(IdentityAccountTrackerTest, PrimaryTokenAvailableAndRevokedThenLogin) { @@ -419,11 +399,10 @@ NotifyLogin(kPrimaryAccountKey); ReturnOAuthUrlFetchSuccess(kPrimaryAccountKey); EXPECT_TRUE( - observer()->CheckEvents(TrackingEvent(ADDED, kPrimaryAccountKey), - TrackingEvent(SIGN_IN, kPrimaryAccountKey))); + observer()->CheckEvents(TrackingEvent(SIGN_IN, kPrimaryAccountKey))); } -TEST_F(IdentityAccountTrackerTest, PrimaryRevokeThenLogout) { +TEST_F(IdentityAccountTrackerTest, PrimaryRevoke) { NotifyLogin(kPrimaryAccountKey); NotifyTokenAvailable(kPrimaryAccountKey); ReturnOAuthUrlFetchSuccess(kPrimaryAccountKey); @@ -432,10 +411,6 @@ NotifyTokenRevoked(kPrimaryAccountKey); EXPECT_TRUE( observer()->CheckEvents(TrackingEvent(SIGN_OUT, kPrimaryAccountKey))); - - NotifyLogout(); - EXPECT_TRUE( - observer()->CheckEvents(TrackingEvent(REMOVED, kPrimaryAccountKey))); } TEST_F(IdentityAccountTrackerTest, PrimaryRevokeThenLogin) { @@ -469,8 +444,7 @@ NotifyLogout(); EXPECT_TRUE( - observer()->CheckEvents(TrackingEvent(SIGN_OUT, kPrimaryAccountKey), - TrackingEvent(REMOVED, kPrimaryAccountKey))); + observer()->CheckEvents(TrackingEvent(SIGN_OUT, kPrimaryAccountKey))); NotifyTokenRevoked(kPrimaryAccountKey); EXPECT_TRUE(observer()->CheckEvents()); @@ -487,7 +461,6 @@ NotifyTokenAvailable(kPrimaryAccountKey); ReturnOAuthUrlFetchSuccess(kPrimaryAccountKey); EXPECT_TRUE(observer()->CheckEvents( - TrackingEvent(ADDED, kPrimaryAccountKey), TrackingEvent(SIGN_IN, kPrimaryAccountKey))); } @@ -501,7 +474,6 @@ ReturnOAuthUrlFetchSuccess("user@example.com"); EXPECT_TRUE(observer()->CheckEvents( - TrackingEvent(ADDED, "user@example.com"), TrackingEvent(SIGN_IN, "user@example.com"))); } @@ -519,7 +491,6 @@ ReturnOAuthUrlFetchSuccess("user@example.com"); NotifyTokenRevoked("user@example.com"); EXPECT_TRUE(observer()->CheckEvents( - TrackingEvent(ADDED, "user@example.com"), TrackingEvent(SIGN_IN, "user@example.com"), TrackingEvent(SIGN_OUT, "user@example.com"))); @@ -538,7 +509,6 @@ NotifyTokenAvailable("user@example.com"); ReturnOAuthUrlFetchSuccess("user@example.com"); EXPECT_TRUE(observer()->CheckEvents( - TrackingEvent(ADDED, "user@example.com"), TrackingEvent(SIGN_IN, "user@example.com"))); } @@ -549,7 +519,6 @@ ReturnOAuthUrlFetchSuccess("user@example.com"); NotifyTokenRevoked("user@example.com"); EXPECT_TRUE(observer()->CheckEvents( - TrackingEvent(ADDED, "user@example.com"), TrackingEvent(SIGN_IN, "user@example.com"), TrackingEvent(SIGN_OUT, "user@example.com"))); @@ -563,7 +532,6 @@ NotifyTokenAvailable("user@example.com"); ReturnOAuthUrlFetchSuccess("user@example.com"); EXPECT_TRUE(observer()->CheckEvents( - TrackingEvent(ADDED, "user@example.com"), TrackingEvent(SIGN_IN, "user@example.com"))); NotifyTokenAvailable("user@example.com"); @@ -576,13 +544,11 @@ NotifyTokenAvailable("alpha@example.com"); ReturnOAuthUrlFetchSuccess("alpha@example.com"); EXPECT_TRUE(observer()->CheckEvents( - TrackingEvent(ADDED, "alpha@example.com"), TrackingEvent(SIGN_IN, "alpha@example.com"))); NotifyTokenAvailable("beta@example.com"); ReturnOAuthUrlFetchSuccess("beta@example.com"); EXPECT_TRUE(observer()->CheckEvents( - TrackingEvent(ADDED, "beta@example.com"), TrackingEvent(SIGN_IN, "beta@example.com"))); NotifyTokenRevoked("alpha@example.com"); @@ -604,7 +570,6 @@ NotifyTokenAvailable("user@example.com"); ReturnOAuthUrlFetchSuccess("user@example.com"); EXPECT_TRUE(observer()->CheckEvents( - TrackingEvent(ADDED, "user@example.com"), TrackingEvent(SIGN_IN, "user@example.com"))); } @@ -618,20 +583,15 @@ observer()->SortEventsByUser(); EXPECT_TRUE(observer()->CheckEvents( - TrackingEvent(ADDED, "alpha@example.com"), TrackingEvent(SIGN_IN, "alpha@example.com"), - TrackingEvent(ADDED, "beta@example.com"), TrackingEvent(SIGN_IN, "beta@example.com"))); NotifyLogout(); observer()->SortEventsByUser(); - EXPECT_TRUE(observer()->CheckEvents( - TrackingEvent(SIGN_OUT, "alpha@example.com"), - TrackingEvent(REMOVED, "alpha@example.com"), - TrackingEvent(SIGN_OUT, "beta@example.com"), - TrackingEvent(REMOVED, "beta@example.com"), - TrackingEvent(SIGN_OUT, kPrimaryAccountKey), - TrackingEvent(REMOVED, kPrimaryAccountKey))); + EXPECT_TRUE( + observer()->CheckEvents(TrackingEvent(SIGN_OUT, "alpha@example.com"), + TrackingEvent(SIGN_OUT, "beta@example.com"), + TrackingEvent(SIGN_OUT, kPrimaryAccountKey))); // No events fire at all while profile is signed out. NotifyTokenRevoked("alpha@example.com"); @@ -646,11 +606,8 @@ ReturnOAuthUrlFetchSuccess(kPrimaryAccountKey); observer()->SortEventsByUser(); EXPECT_TRUE(observer()->CheckEvents( - TrackingEvent(ADDED, "beta@example.com"), TrackingEvent(SIGN_IN, "beta@example.com"), - TrackingEvent(ADDED, "gamma@example.com"), TrackingEvent(SIGN_IN, "gamma@example.com"), - TrackingEvent(ADDED, kPrimaryAccountKey), TrackingEvent(SIGN_IN, kPrimaryAccountKey))); // Revoking the primary token does not affect other accounts. @@ -686,9 +643,7 @@ observer()->SortEventsByUser(); EXPECT_TRUE( observer()->CheckEvents(TrackingEvent(SIGN_OUT, kPrimaryAccountKey), - TrackingEvent(REMOVED, kPrimaryAccountKey), - TrackingEvent(SIGN_OUT, "user@example.com"), - TrackingEvent(REMOVED, "user@example.com"))); + TrackingEvent(SIGN_OUT, "user@example.com"))); } TEST_F(IdentityAccountTrackerTest, MultiRevokePrimaryDoesNotRemoveAllAccounts) {
diff --git a/headless/BUILD.gn b/headless/BUILD.gn index b7822f4..30ddfcf 100644 --- a/headless/BUILD.gn +++ b/headless/BUILD.gn
@@ -307,6 +307,8 @@ "public/util/navigation_request.h", "public/util/testing/generic_url_request_mocks.cc", "public/util/testing/generic_url_request_mocks.h", + "public/util/throttled_dispatcher.cc", + "public/util/throttled_dispatcher.h", "public/util/url_fetcher.cc", "public/util/url_fetcher.h", "public/util/url_request_dispatcher.h", @@ -471,6 +473,7 @@ "public/util/generic_url_request_job_test.cc", "public/util/testing/fake_managed_dispatch_url_request_job.cc", "public/util/testing/fake_managed_dispatch_url_request_job.h", + "public/util/throttled_dispatcher_test.cc", ] if (!is_component_build) {
diff --git a/headless/public/util/expedited_dispatcher.h b/headless/public/util/expedited_dispatcher.h index 80579d2d..0a4f1d11 100644 --- a/headless/public/util/expedited_dispatcher.h +++ b/headless/public/util/expedited_dispatcher.h
@@ -33,7 +33,7 @@ void NavigationRequested( std::unique_ptr<NavigationRequest> navigation_request) override; - private: + protected: scoped_refptr<base::SingleThreadTaskRunner> io_thread_task_runner_; DISALLOW_COPY_AND_ASSIGN(ExpeditedDispatcher);
diff --git a/headless/public/util/throttled_dispatcher.cc b/headless/public/util/throttled_dispatcher.cc new file mode 100644 index 0000000..48efca7 --- /dev/null +++ b/headless/public/util/throttled_dispatcher.cc
@@ -0,0 +1,55 @@ +// Copyright 2017 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 "headless/public/util/throttled_dispatcher.h" + +#include <utility> + +#include "base/bind.h" +#include "base/synchronization/lock.h" +#include "headless/public/util/managed_dispatch_url_request_job.h" + +namespace headless { + +ThrottledDispatcher::ThrottledDispatcher( + scoped_refptr<base::SingleThreadTaskRunner> io_thread_task_runner) + : ExpeditedDispatcher(std::move(io_thread_task_runner)), + requests_paused_(false) {} + +ThrottledDispatcher::~ThrottledDispatcher() {} + +void ThrottledDispatcher::PauseRequests() { + base::AutoLock lock(lock_); + requests_paused_ = true; +} + +void ThrottledDispatcher::ResumeRequests() { + base::AutoLock lock(lock_); + requests_paused_ = false; + for (ManagedDispatchURLRequestJob* job : paused_jobs_) { + io_thread_task_runner_->PostTask( + FROM_HERE, base::Bind(&ManagedDispatchURLRequestJob::OnHeadersComplete, + base::Unretained(job))); + } + paused_jobs_.clear(); +} + +void ThrottledDispatcher::DataReady(ManagedDispatchURLRequestJob* job) { + base::AutoLock lock(lock_); + if (requests_paused_) { + paused_jobs_.push_back(job); + } else { + io_thread_task_runner_->PostTask( + FROM_HERE, base::Bind(&ManagedDispatchURLRequestJob::OnHeadersComplete, + base::Unretained(job))); + } +} + +void ThrottledDispatcher::JobDeleted(ManagedDispatchURLRequestJob* job) { + base::AutoLock lock(lock_); + paused_jobs_.erase(std::remove(paused_jobs_.begin(), paused_jobs_.end(), job), + paused_jobs_.end()); +} + +} // namespace headless
diff --git a/headless/public/util/throttled_dispatcher.h b/headless/public/util/throttled_dispatcher.h new file mode 100644 index 0000000..152dbd75a --- /dev/null +++ b/headless/public/util/throttled_dispatcher.h
@@ -0,0 +1,41 @@ +// Copyright 2017 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 HEADLESS_PUBLIC_UTIL_THROTTLED_DISPATCHER_H_ +#define HEADLESS_PUBLIC_UTIL_THROTTLED_DISPATCHER_H_ + +#include <vector> + +#include "headless/public/util/expedited_dispatcher.h" + +namespace headless { + +class ManagedDispatchURLRequestJob; + +// Similar to ExpeditedDispatcher except network fetches can be paused. +class HEADLESS_EXPORT ThrottledDispatcher : public ExpeditedDispatcher { + public: + explicit ThrottledDispatcher( + scoped_refptr<base::SingleThreadTaskRunner> io_thread_task_runner); + ~ThrottledDispatcher() override; + + void PauseRequests(); + void ResumeRequests(); + + // UrlRequestDispatcher implementation: + void DataReady(ManagedDispatchURLRequestJob* job) override; + void JobDeleted(ManagedDispatchURLRequestJob* job) override; + + private: + // Protects all members below. + base::Lock lock_; + bool requests_paused_; + std::vector<ManagedDispatchURLRequestJob*> paused_jobs_; + + DISALLOW_COPY_AND_ASSIGN(ThrottledDispatcher); +}; + +} // namespace headless + +#endif // HEADLESS_PUBLIC_UTIL_EXPEDITED_DISPATCHER_H_
diff --git a/headless/public/util/throttled_dispatcher_test.cc b/headless/public/util/throttled_dispatcher_test.cc new file mode 100644 index 0000000..5e3ba3f --- /dev/null +++ b/headless/public/util/throttled_dispatcher_test.cc
@@ -0,0 +1,121 @@ +// Copyright 2017 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 "headless/public/util/throttled_dispatcher.h" + +#include <memory> +#include <string> +#include <vector> + +#include "base/memory/ptr_util.h" +#include "base/message_loop/message_loop.h" +#include "base/run_loop.h" +#include "base/single_thread_task_runner.h" +#include "headless/public/util/navigation_request.h" +#include "headless/public/util/testing/fake_managed_dispatch_url_request_job.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +using testing::ElementsAre; + +namespace headless { + +class ThrottledDispatcherTest : public ::testing::Test { + protected: + ThrottledDispatcherTest() {} + ~ThrottledDispatcherTest() override {} + + void SetUp() override { + throttled_dispatcher_.reset(new ThrottledDispatcher(loop_.task_runner())); + } + + base::MessageLoop loop_; + std::unique_ptr<ThrottledDispatcher> throttled_dispatcher_; +}; + +TEST_F(ThrottledDispatcherTest, DispatchDataReadyInReverseOrder) { + std::vector<std::string> notifications; + std::unique_ptr<FakeManagedDispatchURLRequestJob> job1( + new FakeManagedDispatchURLRequestJob(throttled_dispatcher_.get(), 1, + ¬ifications)); + std::unique_ptr<FakeManagedDispatchURLRequestJob> job2( + new FakeManagedDispatchURLRequestJob(throttled_dispatcher_.get(), 2, + ¬ifications)); + std::unique_ptr<FakeManagedDispatchURLRequestJob> job3( + new FakeManagedDispatchURLRequestJob(throttled_dispatcher_.get(), 3, + ¬ifications)); + std::unique_ptr<FakeManagedDispatchURLRequestJob> job4( + new FakeManagedDispatchURLRequestJob(throttled_dispatcher_.get(), 4, + ¬ifications)); + job4->DispatchHeadersComplete(); + job3->DispatchHeadersComplete(); + job2->DispatchHeadersComplete(); + job1->DispatchHeadersComplete(); + + EXPECT_TRUE(notifications.empty()); + + base::RunLoop().RunUntilIdle(); + EXPECT_THAT(notifications, + ElementsAre("id: 4 OK", "id: 3 OK", "id: 2 OK", "id: 1 OK")); +} + +TEST_F(ThrottledDispatcherTest, ErrorsAndDataReadyDispatchedInCallOrder) { + std::vector<std::string> notifications; + std::unique_ptr<FakeManagedDispatchURLRequestJob> job1( + new FakeManagedDispatchURLRequestJob(throttled_dispatcher_.get(), 1, + ¬ifications)); + std::unique_ptr<FakeManagedDispatchURLRequestJob> job2( + new FakeManagedDispatchURLRequestJob(throttled_dispatcher_.get(), 2, + ¬ifications)); + std::unique_ptr<FakeManagedDispatchURLRequestJob> job3( + new FakeManagedDispatchURLRequestJob(throttled_dispatcher_.get(), 3, + ¬ifications)); + std::unique_ptr<FakeManagedDispatchURLRequestJob> job4( + new FakeManagedDispatchURLRequestJob(throttled_dispatcher_.get(), 4, + ¬ifications)); + job4->DispatchHeadersComplete(); + job3->DispatchStartError(static_cast<net::Error>(-123)); + job2->DispatchHeadersComplete(); + job1->DispatchHeadersComplete(); + + EXPECT_TRUE(notifications.empty()); + + base::RunLoop().RunUntilIdle(); + EXPECT_THAT(notifications, ElementsAre("id: 4 OK", "id: 3 err: -123", + "id: 2 OK", "id: 1 OK")); +} + +TEST_F(ThrottledDispatcherTest, Throttling) { + std::vector<std::string> notifications; + std::unique_ptr<FakeManagedDispatchURLRequestJob> job1( + new FakeManagedDispatchURLRequestJob(throttled_dispatcher_.get(), 1, + ¬ifications)); + std::unique_ptr<FakeManagedDispatchURLRequestJob> job2( + new FakeManagedDispatchURLRequestJob(throttled_dispatcher_.get(), 2, + ¬ifications)); + std::unique_ptr<FakeManagedDispatchURLRequestJob> job3( + new FakeManagedDispatchURLRequestJob(throttled_dispatcher_.get(), 3, + ¬ifications)); + std::unique_ptr<FakeManagedDispatchURLRequestJob> job4( + new FakeManagedDispatchURLRequestJob(throttled_dispatcher_.get(), 4, + ¬ifications)); + + throttled_dispatcher_->PauseRequests(); + + job4->DispatchHeadersComplete(); + job3->DispatchStartError(static_cast<net::Error>(-123)); + job2->DispatchHeadersComplete(); + job1->DispatchHeadersComplete(); + + base::RunLoop().RunUntilIdle(); + EXPECT_THAT(notifications, ElementsAre("id: 3 err: -123")); + + throttled_dispatcher_->ResumeRequests(); + base::RunLoop().RunUntilIdle(); + + EXPECT_THAT(notifications, ElementsAre("id: 3 err: -123", "id: 4 OK", + "id: 2 OK", "id: 1 OK")); +} + +} // namespace headless
diff --git a/ios/chrome/app/BUILD.gn b/ios/chrome/app/BUILD.gn index 8e7d5be..142e159 100644 --- a/ios/chrome/app/BUILD.gn +++ b/ios/chrome/app/BUILD.gn
@@ -183,7 +183,6 @@ "//ios/chrome/browser/memory", "//ios/chrome/browser/metrics", "//ios/chrome/browser/metrics:metrics_internal", - "//ios/chrome/browser/native_app_launcher:native_app_launcher_internal", "//ios/chrome/browser/net", "//ios/chrome/browser/omaha", "//ios/chrome/browser/prefs",
diff --git a/ios/chrome/browser/browsing_data/BUILD.gn b/ios/chrome/browser/browsing_data/BUILD.gn index ee16ec9d..fce3d9f 100644 --- a/ios/chrome/browser/browsing_data/BUILD.gn +++ b/ios/chrome/browser/browsing_data/BUILD.gn
@@ -92,13 +92,10 @@ "//components/signin/ios/browser", "//ios/chrome/browser:browser_internal", "//ios/chrome/browser/browser_state", - "//ios/chrome/browser/native_app_launcher:native_app_launcher_internal", "//ios/chrome/browser/sessions:serialisation", "//ios/chrome/browser/signin", "//ios/chrome/browser/snapshots", "//ios/chrome/browser/ui:ui_internal", - "//ios/public/provider/chrome/browser", - "//ios/public/provider/chrome/browser/native_app_launcher", "//ios/web", "//net", ]
diff --git a/ios/chrome/browser/browsing_data/browsing_data_removal_controller.mm b/ios/chrome/browser/browsing_data/browsing_data_removal_controller.mm index f13a5eb..115c6ff 100644 --- a/ios/chrome/browser/browsing_data/browsing_data_removal_controller.mm +++ b/ios/chrome/browser/browsing_data/browsing_data_removal_controller.mm
@@ -24,9 +24,6 @@ #include "ios/chrome/browser/signin/account_consistency_service_factory.h" #import "ios/chrome/browser/snapshots/snapshots_util.h" #import "ios/chrome/browser/ui/browser_view_controller.h" -#include "ios/public/provider/chrome/browser/chrome_browser_provider.h" -#import "ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h" -#import "ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h" #include "ios/web/public/web_thread.h" #import "ios/web/public/web_view_creation_util.h" #import "ios/web/web_state/ui/wk_web_view_configuration_provider.h" @@ -46,8 +43,6 @@ } @interface BrowsingDataRemovalController () -// Removes data used by the Google App Launcher. -- (void)removeGALData; // Removes browsing data that is created by web views associated with // |browserState|. |mask| is obtained from // IOSChromeBrowsingDataRemover::RemoveDataMask. |deleteBegin| defines the begin @@ -236,13 +231,6 @@ ClearIOSSnapshots(); } - // TODO(crbug.com/227636): Support multiple profile. - // Google App Launcher data is tied to the normal profile. - if (mask & IOSChromeBrowsingDataRemover::REMOVE_GOOGLE_APP_LAUNCHER_DATA && - !browserState->IsOffTheRecord()) { - [self removeGALData]; - } - if (browserState->IsOffTheRecord()) { // In incognito, only data removal for all time is currently supported. DCHECK_EQ(base::Time(), deleteBegin); @@ -256,15 +244,6 @@ completionHandler:browsingDataCleared]; } -- (void)removeGALData { - [ios::GetChromeBrowserProvider()->GetNativeAppWhitelistManager() - filteredAppsUsingBlock:^BOOL(const id<NativeAppMetadata> app, - BOOL* stop) { - [app resetInfobarHistory]; - return NO; - }]; -} - - (void)removeWebViewCreatedBrowsingDataFromBrowserState: (ios::ChromeBrowserState*)browserState mask:(int)mask
diff --git a/ios/chrome/browser/browsing_data/ios_chrome_browsing_data_remover.h b/ios/chrome/browser/browsing_data/ios_chrome_browsing_data_remover.h index 79f33eba..643c1db 100644 --- a/ios/chrome/browser/browsing_data/ios_chrome_browsing_data_remover.h +++ b/ios/chrome/browser/browsing_data/ios_chrome_browsing_data_remover.h
@@ -48,9 +48,8 @@ REMOVE_PASSWORDS = 1 << 8, REMOVE_WEBSQL = 1 << 9, REMOVE_CHANNEL_IDS = 1 << 10, - REMOVE_GOOGLE_APP_LAUNCHER_DATA = 1 << 11, - REMOVE_CACHE_STORAGE = 1 << 12, - REMOVE_VISITED_LINKS = 1 << 13, + REMOVE_CACHE_STORAGE = 1 << 11, + REMOVE_VISITED_LINKS = 1 << 12, // "Site data" includes cookies, appcache, file systems, indexedDBs, local // storage, webSQL, service workers, cache storage, plugin data, and web app
diff --git a/ios/chrome/browser/native_app_launcher/BUILD.gn b/ios/chrome/browser/native_app_launcher/BUILD.gn index c502fc3..48f7e41 100644 --- a/ios/chrome/browser/native_app_launcher/BUILD.gn +++ b/ios/chrome/browser/native_app_launcher/BUILD.gn
@@ -48,65 +48,3 @@ "//url", ] } - -source_set("native_app_launcher_internal") { - configs += [ "//build/config/compiler:enable_arc" ] - sources = [ - "native_app_navigation_controller.h", - "native_app_navigation_controller.mm", - "native_app_navigation_util.h", - "native_app_navigation_util.mm", - ] - deps = [ - ":native_app_launcher", - "//base", - "//components/image_fetcher/ios", - "//components/infobars/core", - "//ios/chrome/browser", - "//ios/chrome/browser/infobars", - "//ios/chrome/browser/net", - "//ios/chrome/browser/store_kit", - "//ios/chrome/browser/tabs", - "//ios/chrome/browser/web:web", - "//ios/public/provider/chrome/browser", - "//ios/public/provider/chrome/browser/native_app_launcher", - "//ios/public/provider/chrome/browser/signin", - "//ios/public/provider/chrome/browser/ui", - "//ios/web", - "//net", - "//ui/base", - "//url", - ] - libs = [ "StoreKit.framework" ] -} - -source_set("unit_tests_internal") { - configs += [ "//build/config/compiler:enable_arc" ] - testonly = true - sources = [ - "native_app_navigation_controller_unittest.mm", - "native_app_navigation_util_unittest.mm", - ] - deps = [ - ":native_app_launcher", - ":native_app_launcher_internal", - "//base", - "//base/test:test_support", - "//components/infobars/core", - "//ios/chrome/browser", - "//ios/chrome/browser/infobars:infobars", - "//ios/chrome/browser/web:test_support", - "//ios/chrome/test:test_support", - "//ios/public/provider/chrome/browser", - "//ios/public/provider/chrome/browser:test_support", - "//ios/public/provider/chrome/browser/native_app_launcher:test_support", - "//ios/public/provider/chrome/browser/signin:test_support", - "//ios/web", - "//ios/web/public/test/fakes", - "//net:test_support", - "//testing/gmock", - "//testing/gtest", - "//ui/base", - "//url", - ] -}
diff --git a/ios/chrome/browser/native_app_launcher/native_app_navigation_controller.h b/ios/chrome/browser/native_app_launcher/native_app_navigation_controller.h deleted file mode 100644 index 9ddac65..0000000 --- a/ios/chrome/browser/native_app_launcher/native_app_navigation_controller.h +++ /dev/null
@@ -1,29 +0,0 @@ -// Copyright 2012 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 IOS_CHROME_BROWSER_NATIVE_APP_LAUNCHER_NATIVE_APP_NAVIGATION_CONTROLLER_H_ -#define IOS_CHROME_BROWSER_NATIVE_APP_LAUNCHER_NATIVE_APP_NAVIGATION_CONTROLLER_H_ - -#import <Foundation/Foundation.h> - -namespace web { -class WebState; -} // namespace web - -// NativeAppNavigationController brings up a GAL Infobar if the webpage directs -// it to do so and there are no other circumstances that would suppress its -// display. -@interface NativeAppNavigationController : NSObject - -- (instancetype)initWithWebState:(web::WebState*)webState - NS_DESIGNATED_INITIALIZER; - -- (instancetype)init NS_UNAVAILABLE; - -// Copies the list of applications possibly being installed and register to be -// notified of their installation. -- (void)copyStateFrom:(NativeAppNavigationController*)controller; -@end - -#endif // IOS_CHROME_BROWSER_NATIVE_APP_LAUNCHER_NATIVE_APP_NAVIGATION_CONTROLLER_H_
diff --git a/ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm b/ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm deleted file mode 100644 index e9fac9d..0000000 --- a/ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm +++ /dev/null
@@ -1,245 +0,0 @@ -// Copyright 2012 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. - -#import "ios/chrome/browser/native_app_launcher/native_app_navigation_controller.h" - -#import <StoreKit/StoreKit.h> - -#include "base/memory/ptr_util.h" -#include "base/metrics/user_metrics.h" -#include "base/metrics/user_metrics_action.h" -#include "base/threading/sequenced_worker_pool.h" -#include "components/image_fetcher/ios/ios_image_data_fetcher_wrapper.h" -#include "components/infobars/core/infobar_manager.h" -#include "ios/chrome/browser/infobars/infobar_manager_impl.h" -#import "ios/chrome/browser/installation_notifier.h" -#include "ios/chrome/browser/native_app_launcher/native_app_infobar_delegate.h" -#import "ios/chrome/browser/native_app_launcher/native_app_navigation_controller_protocol.h" -#include "ios/chrome/browser/native_app_launcher/native_app_navigation_util.h" -#import "ios/chrome/browser/open_url_util.h" -#import "ios/chrome/browser/store_kit/store_kit_tab_helper.h" -#import "ios/chrome/browser/tabs/legacy_tab_helper.h" -#import "ios/chrome/browser/tabs/tab.h" -#include "ios/public/provider/chrome/browser/chrome_browser_provider.h" -#import "ios/public/provider/chrome/browser/native_app_launcher/native_app_metadata.h" -#import "ios/public/provider/chrome/browser/native_app_launcher/native_app_types.h" -#import "ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h" -#include "ios/web/public/browser_state.h" -#include "ios/web/public/web_state/web_state.h" -#import "ios/web/public/web_state/web_state_observer_bridge.h" -#include "ios/web/public/web_thread.h" -#import "net/base/mac/url_conversions.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -using base::UserMetricsAction; - -@interface NativeAppNavigationController ()< - CRWWebStateObserver, - NativeAppNavigationControllerProtocol> -// Shows a native app infobar by looking at the page's URL and by checking -// wheter that infobar should be bypassed or not. -- (void)showInfoBarIfNecessary; - -// Returns a pointer to the NSMutableSet of |_appsPossiblyBeingInstalled| -- (NSMutableSet*)appsPossiblyBeingInstalled; - -// Records what type of infobar was opened. -- (void)recordInfobarDisplayedOfType:(NativeAppControllerType)type - onLinkNavigation:(BOOL)isLinkNavigation; - -@end - -@implementation NativeAppNavigationController { - // WebState provides access to the *TabHelper objects. - web::WebState* _webState; - // ImageFetcher needed to fetch the icons. - std::unique_ptr<image_fetcher::IOSImageDataFetcherWrapper> _imageFetcher; - id<NativeAppMetadata> _metadata; - // A set of appIds encoded as NSStrings. - NSMutableSet* _appsPossiblyBeingInstalled; - // Allows this class to subscribe for CRWWebStateObserver callbacks. - std::unique_ptr<web::WebStateObserverBridge> _webStateObserver; -} - -// Designated initializer. Use this instead of -init. -- (instancetype)initWithWebState:(web::WebState*)webState { - self = [super init]; - if (self) { - DCHECK(webState); - _webState = webState; - _imageFetcher = base::MakeUnique<image_fetcher::IOSImageDataFetcherWrapper>( - _webState->GetBrowserState()->GetRequestContext(), - web::WebThread::GetBlockingPool()); - _appsPossiblyBeingInstalled = [[NSMutableSet alloc] init]; - _webStateObserver = - base::MakeUnique<web::WebStateObserverBridge>(webState, self); - } - return self; -} - -- (void)copyStateFrom:(NativeAppNavigationController*)controller { - DCHECK(controller); - _appsPossiblyBeingInstalled = [[NSMutableSet alloc] - initWithSet:[controller appsPossiblyBeingInstalled]]; - for (NSString* appIdString in _appsPossiblyBeingInstalled) { - DCHECK([appIdString isKindOfClass:[NSString class]]); - NSURL* appURL = - [ios::GetChromeBrowserProvider()->GetNativeAppWhitelistManager() - schemeForAppId:appIdString]; - [[InstallationNotifier sharedInstance] - registerForInstallationNotifications:self - withSelector:@selector(appDidInstall:) - forScheme:[appURL scheme]]; - } - [self showInfoBarIfNecessary]; -} - -- (void)dealloc { - [[InstallationNotifier sharedInstance] unregisterForNotifications:self]; -} - -- (NSMutableSet*)appsPossiblyBeingInstalled { - return _appsPossiblyBeingInstalled; -} - -- (void)showInfoBarIfNecessary { - // Find a potential matching native app. - GURL pageURL = _webState->GetLastCommittedURL(); - _metadata = [ios::GetChromeBrowserProvider()->GetNativeAppWhitelistManager() - nativeAppForURL:pageURL]; - if (!_metadata || [_metadata shouldBypassInfoBars]) - return; - - // Select the infobar type. - NativeAppControllerType type; - bool isLinkNavigation = native_app_launcher::IsLinkNavigation(_webState); - if ([_metadata canOpenURL:pageURL]) { // App is installed. - type = isLinkNavigation && ![_metadata shouldAutoOpenLinks] - ? NATIVE_APP_OPEN_POLICY_CONTROLLER - : NATIVE_APP_LAUNCHER_CONTROLLER; - } else { // App is not installed. - // Check if the user already opened the store for this app. - if ([_appsPossiblyBeingInstalled containsObject:[_metadata appId]]) - return; - type = NATIVE_APP_INSTALLER_CONTROLLER; - } - // Inform the metadata that an infobar of |type| will be shown so that metrics - // and ignored behavior can be handled. - [_metadata willBeShownInInfobarOfType:type]; - // Display the proper infobar. - infobars::InfoBarManager* infoBarManager = - InfoBarManagerImpl::FromWebState(_webState); - NativeAppInfoBarDelegate::Create(infoBarManager, self, pageURL, type); - [self recordInfobarDisplayedOfType:type onLinkNavigation:isLinkNavigation]; -} - -- (void)recordInfobarDisplayedOfType:(NativeAppControllerType)type - onLinkNavigation:(BOOL)isLinkNavigation { - switch (type) { - case NATIVE_APP_INSTALLER_CONTROLLER: - base::RecordAction( - isLinkNavigation - ? UserMetricsAction("MobileGALInstallInfoBarLinkNavigation") - : UserMetricsAction("MobileGALInstallInfoBarDirectNavigation")); - break; - case NATIVE_APP_LAUNCHER_CONTROLLER: - base::RecordAction(UserMetricsAction("MobileGALLaunchInfoBar")); - break; - case NATIVE_APP_OPEN_POLICY_CONTROLLER: - base::RecordAction(UserMetricsAction("MobileGALOpenPolicyInfoBar")); - break; - default: - NOTREACHED(); - break; - } -} - -#pragma mark - NativeAppNavigationControllerProtocol methods - -- (NSString*)appId { - return [_metadata appId]; -} - -- (NSString*)appName { - return [_metadata appName]; -} - -- (void)fetchSmallIconWithCompletionBlock:(void (^)(UIImage*))block { - [_metadata fetchSmallIconWithImageFetcher:_imageFetcher.get() - completionBlock:block]; -} - -- (void)openStore { - // Register to get a notification when the app is installed. - [[InstallationNotifier sharedInstance] - registerForInstallationNotifications:self - withSelector:@selector(appDidInstall:) - forScheme:[_metadata anyScheme]]; - NSString* appIdString = [self appId]; - // Defensively early return if native app metadata returns an nil string for - // appId which can cause subsequent -addObject: to throw exceptions. - if (![appIdString length]) - return; - DCHECK(![_appsPossiblyBeingInstalled containsObject:appIdString]); - [_appsPossiblyBeingInstalled addObject:appIdString]; - StoreKitTabHelper* tabHelper = StoreKitTabHelper::FromWebState(_webState); - if (tabHelper) - tabHelper->OpenAppStore(appIdString); -} - -- (void)launchApp:(const GURL&)URL { - // TODO(crbug.com/353957): Pass the ChromeIdentity to - // -launchURLWithURL:identity: - GURL launchURL([_metadata launchURLWithURL:URL identity:nil]); - if (launchURL.is_valid()) { - OpenUrlWithCompletionHandler(net::NSURLWithGURL(launchURL), nil); - } -} - -- (void)updateMetadataWithUserAction:(NativeAppActionType)userAction { - [_metadata updateWithUserAction:userAction]; -} - -#pragma mark - CRWWebStateObserver methods - -- (void)webState:(web::WebState*)webState didLoadPageWithSuccess:(BOOL)success { - Tab* tab = LegacyTabHelper::GetTabForWebState(_webState); - if (success && ![tab isPrerenderTab]) - [self showInfoBarIfNecessary]; -} - -- (void)webStateDestroyed:(web::WebState*)webState { - _webState = nullptr; - _webStateObserver.reset(); -} - -#pragma mark - Private methods - -- (void)appDidInstall:(NSNotification*)notification { - [self removeAppFromNotification:notification]; - [self showInfoBarIfNecessary]; -} - -- (void)removeAppFromNotification:(NSNotification*)notification { - DCHECK([[notification object] isKindOfClass:[InstallationNotifier class]]); - NSString* schemeOfInstalledApp = [notification name]; - __block NSString* appIDToRemove = nil; - [_appsPossiblyBeingInstalled - enumerateObjectsUsingBlock:^(id appID, BOOL* stop) { - NSURL* appURL = - [ios::GetChromeBrowserProvider()->GetNativeAppWhitelistManager() - schemeForAppId:appID]; - if ([[appURL scheme] isEqualToString:schemeOfInstalledApp]) { - appIDToRemove = appID; - *stop = YES; - } - }]; - DCHECK(appIDToRemove); - [_appsPossiblyBeingInstalled removeObject:appIDToRemove]; -} - -@end
diff --git a/ios/chrome/browser/native_app_launcher/native_app_navigation_controller_unittest.mm b/ios/chrome/browser/native_app_launcher/native_app_navigation_controller_unittest.mm deleted file mode 100644 index 0607c780..0000000 --- a/ios/chrome/browser/native_app_launcher/native_app_navigation_controller_unittest.mm +++ /dev/null
@@ -1,207 +0,0 @@ -// Copyright 2012 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 <memory> - -#include "base/bind.h" -#include "base/memory/ptr_util.h" -#include "base/metrics/user_metrics.h" -#include "base/strings/utf_string_conversions.h" -#include "ios/chrome/browser/infobars/infobar_manager_impl.h" -#import "ios/chrome/browser/installation_notifier.h" -#include "ios/chrome/browser/native_app_launcher/native_app_infobar_delegate.h" -#import "ios/chrome/browser/native_app_launcher/native_app_navigation_controller.h" -#import "ios/chrome/browser/web/chrome_web_test.h" -#include "ios/chrome/test/ios_chrome_scoped_testing_chrome_browser_provider.h" -#import "ios/public/provider/chrome/browser/native_app_launcher/fake_native_app_metadata.h" -#import "ios/public/provider/chrome/browser/native_app_launcher/fake_native_app_whitelist_manager.h" -#include "ios/public/provider/chrome/browser/test_chrome_browser_provider.h" -#import "testing/gtest_mac.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -@interface NativeAppNavigationController (Testing) -- (void)recordInfobarDisplayedOfType:(NativeAppControllerType)type - onLinkNavigation:(BOOL)isLinkNavigation; -- (NSMutableSet*)appsPossiblyBeingInstalled; -- (void)removeAppFromNotification:(NSNotification*)notification; -@end - -namespace { - -class FakeChromeBrowserProvider : public ios::TestChromeBrowserProvider { - public: - FakeChromeBrowserProvider(FakeNativeAppWhitelistManager* fake_manager) { - manager_ = fake_manager; - } - ~FakeChromeBrowserProvider() override {} - - id<NativeAppWhitelistManager> GetNativeAppWhitelistManager() const override { - return manager_; - } - - private: - id<NativeAppWhitelistManager> manager_; -}; - -class NativeAppNavigationControllerTest : public ChromeWebTest { - protected: - void SetUp() override { - ChromeWebTest::SetUp(); - controller_ = - [[NativeAppNavigationController alloc] initWithWebState:web_state()]; - - action_callback_ = - base::Bind(&NativeAppNavigationControllerTest::OnUserAction, - base::Unretained(this)); - base::AddActionCallback(action_callback_); - - handler_called_counter_ = 0; - } - - void TearDown() override { - base::RemoveActionCallback(action_callback_); - ChromeWebTest::TearDown(); - } - - void SetExpectedActionName(const std::string& action_name) { - expected_action_name_.reset(new std::string(action_name)); - } - - void OnUserAction(const std::string& action_name) { - EXPECT_EQ(*expected_action_name_, action_name); - handler_called_counter_++; - } - - void ExpectHandlerCalledAndReset(int number_of_calls) { - EXPECT_EQ(number_of_calls, handler_called_counter_); - handler_called_counter_ = 0; - } - - NativeAppNavigationController* controller_; - - // The callback to invoke when an action is recorded. - base::ActionCallback action_callback_; - std::unique_ptr<std::string> expected_action_name_; - int handler_called_counter_; -}; - -TEST_F(NativeAppNavigationControllerTest, TestConstructor) { - EXPECT_TRUE(controller_); -} - -TEST_F(NativeAppNavigationControllerTest, TestUMA) { - SetExpectedActionName("MobileGALInstallInfoBarLinkNavigation"); - [controller_ recordInfobarDisplayedOfType:NATIVE_APP_INSTALLER_CONTROLLER - onLinkNavigation:YES]; - ExpectHandlerCalledAndReset(1); - - SetExpectedActionName("MobileGALInstallInfoBarDirectNavigation"); - [controller_ recordInfobarDisplayedOfType:NATIVE_APP_INSTALLER_CONTROLLER - onLinkNavigation:NO]; - ExpectHandlerCalledAndReset(1); - - SetExpectedActionName("MobileGALLaunchInfoBar"); - [controller_ recordInfobarDisplayedOfType:NATIVE_APP_LAUNCHER_CONTROLLER - onLinkNavigation:YES]; - ExpectHandlerCalledAndReset(1); - [controller_ recordInfobarDisplayedOfType:NATIVE_APP_LAUNCHER_CONTROLLER - onLinkNavigation:NO]; - ExpectHandlerCalledAndReset(1); - - SetExpectedActionName("MobileGALOpenPolicyInfoBar"); - [controller_ recordInfobarDisplayedOfType:NATIVE_APP_OPEN_POLICY_CONTROLLER - onLinkNavigation:YES]; - ExpectHandlerCalledAndReset(1); - [controller_ recordInfobarDisplayedOfType:NATIVE_APP_OPEN_POLICY_CONTROLLER - onLinkNavigation:NO]; - ExpectHandlerCalledAndReset(1); -} - -// Tests presenting NativeAppInfoBar after page is loaded. -TEST_F(NativeAppNavigationControllerTest, NativeAppInfoBar) { - SetExpectedActionName("MobileGALInstallInfoBarDirectNavigation"); - InfoBarManagerImpl::CreateForWebState(web_state()); - - // Set up fake metadata. - FakeNativeAppWhitelistManager* fakeManager = - [[FakeNativeAppWhitelistManager alloc] init]; - IOSChromeScopedTestingChromeBrowserProvider provider( - base::MakeUnique<FakeChromeBrowserProvider>(fakeManager)); - FakeNativeAppMetadata* metadata = [[FakeNativeAppMetadata alloc] init]; - fakeManager.metadata = metadata; - metadata.appName = @"App"; - metadata.appId = @"App-ID"; - - // Load the page to trigger infobar presentation. - LoadHtml(@"<html><body></body></html>", GURL("http://test.com")); - - // Verify that infobar was presented - auto* infobar_manager = InfoBarManagerImpl::FromWebState(web_state()); - ASSERT_EQ(1U, infobar_manager->infobar_count()); - infobars::InfoBar* infobar = infobar_manager->infobar_at(0); - auto* delegate = infobar->delegate()->AsNativeAppInfoBarDelegate(); - ASSERT_TRUE(delegate); - - // Verify infobar appearance. - EXPECT_EQ("Open this page in the App app?", - base::UTF16ToUTF8(delegate->GetInstallText())); - EXPECT_EQ("Open this page in the App app?", - base::UTF16ToUTF8(delegate->GetLaunchText())); - EXPECT_EQ("Open in App", base::UTF16ToUTF8(delegate->GetOpenPolicyText())); - EXPECT_EQ("Just once", base::UTF16ToUTF8(delegate->GetOpenOnceText())); - EXPECT_EQ("Always", base::UTF16ToUTF8(delegate->GetOpenAlwaysText())); - EXPECT_NSEQ(@"App-ID", delegate->GetAppId()); -} - -TEST_F(NativeAppNavigationControllerTest, - TestRemovingAppFromListAfterInstallation) { - NSString* const kMapsAppName = @"Maps"; - NSString* const kMapsAppId = @"1"; - NSString* const kYoutubeAppName = @"Youtube"; - NSString* const kYoutubeAppId = @"2"; - - InstallationNotifier* installationNotifier = - [[InstallationNotifier alloc] init]; - - FakeNativeAppWhitelistManager* fakeManager = - [[FakeNativeAppWhitelistManager alloc] init]; - IOSChromeScopedTestingChromeBrowserProvider provider( - base::MakeUnique<FakeChromeBrowserProvider>(fakeManager)); - - FakeNativeAppMetadata* metadataMaps = [[FakeNativeAppMetadata alloc] init]; - [metadataMaps setAppName:kMapsAppName]; - [metadataMaps setAppId:kMapsAppId]; - ASSERT_TRUE(metadataMaps); - NSString* appIdMaps = [metadataMaps appId]; - NSNotification* notificationMaps = - [NSNotification notificationWithName:kMapsAppName - object:installationNotifier]; - - FakeNativeAppMetadata* metadataYouTube = [[FakeNativeAppMetadata alloc] init]; - [metadataYouTube setAppName:kYoutubeAppName]; - [metadataYouTube setAppId:kYoutubeAppId]; - - ASSERT_TRUE(metadataYouTube); - NSString* appIdYouTube = [metadataYouTube appId]; - NSNotification* notificationYouTube = - [NSNotification notificationWithName:kYoutubeAppName - object:installationNotifier]; - - DCHECK([[controller_ appsPossiblyBeingInstalled] count] == 0); - [[controller_ appsPossiblyBeingInstalled] addObject:appIdMaps]; - DCHECK([[controller_ appsPossiblyBeingInstalled] count] == 1); - [[controller_ appsPossiblyBeingInstalled] addObject:appIdYouTube]; - DCHECK([[controller_ appsPossiblyBeingInstalled] count] == 2); - [fakeManager setAppScheme:kMapsAppName]; - [controller_ removeAppFromNotification:notificationMaps]; - DCHECK([[controller_ appsPossiblyBeingInstalled] count] == 1); - [fakeManager setAppScheme:kYoutubeAppName]; - [controller_ removeAppFromNotification:notificationYouTube]; - DCHECK([[controller_ appsPossiblyBeingInstalled] count] == 0); -} - -} // namespace
diff --git a/ios/chrome/browser/native_app_launcher/native_app_navigation_util.h b/ios/chrome/browser/native_app_launcher/native_app_navigation_util.h deleted file mode 100644 index 36c4fbf..0000000 --- a/ios/chrome/browser/native_app_launcher/native_app_navigation_util.h +++ /dev/null
@@ -1,22 +0,0 @@ -// Copyright 2017 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 IOS_CHROME_BROWSER_NATIVE_APP_LAUNCHER_NATIVE_APP_NAVIGATION_UTIL_H_ -#define IOS_CHROME_BROWSER_NATIVE_APP_LAUNCHER_NATIVE_APP_NAVIGATION_UTIL_H_ - -namespace web { -class WebState; -} // namespace web - -namespace native_app_launcher { - -// Returns whether the current state is arrived at via a "link navigation" in -// the sense of Native App Launcher, i.e. a navigation caused by an explicit -// user action in the rectangle of the web content area. |web_state| must not -// be nullptr. -bool IsLinkNavigation(web::WebState* web_state); - -} // namespace native_app_launcher - -#endif // IOS_CHROME_BROWSER_NATIVE_APP_LAUNCHER_NATIVE_APP_NAVIGATION_UTIL_H_
diff --git a/ios/chrome/browser/native_app_launcher/native_app_navigation_util.mm b/ios/chrome/browser/native_app_launcher/native_app_navigation_util.mm deleted file mode 100644 index 1e2298e..0000000 --- a/ios/chrome/browser/native_app_launcher/native_app_navigation_util.mm +++ /dev/null
@@ -1,29 +0,0 @@ -// Copyright 2017 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 "ios/chrome/browser/native_app_launcher/native_app_navigation_util.h" - -#import "ios/chrome/browser/web/navigation_manager_util.h" -#import "ios/web/public/navigation_item.h" -#import "ios/web/public/web_state/web_state.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -namespace native_app_launcher { - -bool IsLinkNavigation(web::WebState* web_state) { - DCHECK(web_state); - web::NavigationItem* item = - GetLastCommittedNonRedirectedItem(web_state->GetNavigationManager()); - if (!item) - return false; - ui::PageTransition transition = item->GetTransitionType(); - return PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_LINK) || - PageTransitionCoreTypeIs(transition, - ui::PAGE_TRANSITION_AUTO_BOOKMARK); -} - -} // namespace native_app_launcher
diff --git a/ios/chrome/browser/native_app_launcher/native_app_navigation_util_unittest.mm b/ios/chrome/browser/native_app_launcher/native_app_navigation_util_unittest.mm deleted file mode 100644 index a651a2fb..0000000 --- a/ios/chrome/browser/native_app_launcher/native_app_navigation_util_unittest.mm +++ /dev/null
@@ -1,90 +0,0 @@ -// Copyright 2017 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 "ios/chrome/browser/native_app_launcher/native_app_navigation_util.h" - -#include "base/memory/ptr_util.h" -#import "ios/web/public/test/fakes/test_navigation_manager.h" -#import "ios/web/public/test/fakes/test_web_state.h" -#include "testing/platform_test.h" -#include "ui/base/page_transition_types.h" -#include "url/gurl.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -// Tests the implementation of IsLinkNavigation(). The function being tested -// uses public NavigationManager interfaces and can be tested by using -// TestNavigationManager that implements the same interface. -class NativeAppNavigationUtilsTest : public PlatformTest { - protected: - void SetUp() override { - PlatformTest::SetUp(); - std::unique_ptr<web::TestNavigationManager> test_navigation_manager = - base::MakeUnique<web::TestNavigationManager>(); - test_navigation_manager_ = test_navigation_manager.get(); - test_web_state_.SetNavigationManager(std::move(test_navigation_manager)); - } - - web::WebState* web_state() { return &test_web_state_; } - - // Adds a navigation item of |transition| type to current WebState. - void AddItem(const std::string& url_spec, ui::PageTransition transition) { - test_navigation_manager_->AddItem(GURL(url_spec), transition); - } - - private: - web::TestNavigationManager* test_navigation_manager_; - web::TestWebState test_web_state_; -}; - -// Tests that default state is not a link click. -TEST_F(NativeAppNavigationUtilsTest, TestEmpty) { - EXPECT_FALSE(native_app_launcher::IsLinkNavigation(web_state())); -} - -// URL typed by user is not a link click. -TEST_F(NativeAppNavigationUtilsTest, TestTypedUrl) { - AddItem("http://foo.com/page0", ui::PAGE_TRANSITION_TYPED); - EXPECT_FALSE(native_app_launcher::IsLinkNavigation(web_state())); -} - -// Transition state shows that user navigated via a link click. -TEST_F(NativeAppNavigationUtilsTest, TestLinkClicked) { - AddItem("http://foo.com/page0", ui::PAGE_TRANSITION_LINK); - EXPECT_TRUE(native_app_launcher::IsLinkNavigation(web_state())); -} - -// Transition state shows that user navigated through clicking on a bookmark -// is considered *not* a link click. -TEST_F(NativeAppNavigationUtilsTest, TestAutoBookmark) { - AddItem("http://foo.com/page0", ui::PAGE_TRANSITION_AUTO_BOOKMARK); - EXPECT_TRUE(native_app_launcher::IsLinkNavigation(web_state())); -} - -// When there are redirects along the way, redirects are skipped until the -// first non-redirect entry. -TEST_F(NativeAppNavigationUtilsTest, TestHasTypedUrlWithRedirect) { - AddItem("http://foo.com/page0", ui::PAGE_TRANSITION_TYPED); - AddItem("http://foo.com/page1", ui::PAGE_TRANSITION_CLIENT_REDIRECT); - EXPECT_FALSE(native_app_launcher::IsLinkNavigation(web_state())); -} - -// When there are multiple redirects along the way, redirects are skipped until -// the first non-redirect entry. -TEST_F(NativeAppNavigationUtilsTest, TestHasLinkClickedWithRedirect) { - AddItem("http://foo.com/page0", ui::PAGE_TRANSITION_LINK); - AddItem("http://bar.com/page1", ui::PAGE_TRANSITION_CLIENT_REDIRECT); - AddItem("http://zap.com/page2", ui::PAGE_TRANSITION_CLIENT_REDIRECT); - EXPECT_TRUE(native_app_launcher::IsLinkNavigation(web_state())); -} - -// The first non-redirect entry is tested. Earlier redirects do not matter. -TEST_F(NativeAppNavigationUtilsTest, TestTypedUrlWithRedirectEarlier) { - AddItem("http://foo.com/page0", ui::PAGE_TRANSITION_LINK); - AddItem("http://bar.com/page1", ui::PAGE_TRANSITION_CLIENT_REDIRECT); - AddItem("http://blah.com/page2", ui::PAGE_TRANSITION_TYPED); - EXPECT_FALSE(native_app_launcher::IsLinkNavigation(web_state())); -}
diff --git a/ios/chrome/browser/tabs/BUILD.gn b/ios/chrome/browser/tabs/BUILD.gn index 2259699..4f4262a 100644 --- a/ios/chrome/browser/tabs/BUILD.gn +++ b/ios/chrome/browser/tabs/BUILD.gn
@@ -92,7 +92,6 @@ "//ios/chrome/browser/infobars", "//ios/chrome/browser/metrics", "//ios/chrome/browser/metrics:metrics_internal", - "//ios/chrome/browser/native_app_launcher:native_app_launcher_internal", "//ios/chrome/browser/passwords", "//ios/chrome/browser/passwords:passwords_internal", "//ios/chrome/browser/reading_list",
diff --git a/ios/chrome/browser/ui/BUILD.gn b/ios/chrome/browser/ui/BUILD.gn index 809cd7ad..9c1a36a 100644 --- a/ios/chrome/browser/ui/BUILD.gn +++ b/ios/chrome/browser/ui/BUILD.gn
@@ -258,7 +258,6 @@ "//ios/chrome/browser/geolocation:geolocation_internal", "//ios/chrome/browser/infobars", "//ios/chrome/browser/metrics:metrics_internal", - "//ios/chrome/browser/native_app_launcher:native_app_launcher_internal", "//ios/chrome/browser/net", "//ios/chrome/browser/passwords", "//ios/chrome/browser/prefs", @@ -380,7 +379,6 @@ "//ios/chrome/browser/geolocation:geolocation_internal", "//ios/chrome/browser/infobars", "//ios/chrome/browser/metrics:metrics_internal", - "//ios/chrome/browser/native_app_launcher:native_app_launcher_internal", "//ios/chrome/browser/net", "//ios/chrome/browser/passwords", "//ios/chrome/browser/prefs",
diff --git a/ios/chrome/browser/ui/downloads/BUILD.gn b/ios/chrome/browser/ui/downloads/BUILD.gn index b0bd380..380116b 100644 --- a/ios/chrome/browser/ui/downloads/BUILD.gn +++ b/ios/chrome/browser/ui/downloads/BUILD.gn
@@ -47,7 +47,6 @@ "//ios/chrome/app/strings", "//ios/chrome/browser", "//ios/chrome/browser/native_app_launcher", - "//ios/chrome/browser/native_app_launcher:native_app_launcher_internal", "//ios/chrome/browser/store_kit", "//ios/chrome/browser/ui", "//ios/chrome/browser/ui/alert_coordinator",
diff --git a/ios/chrome/browser/ui/settings/BUILD.gn b/ios/chrome/browser/ui/settings/BUILD.gn index 8638120..3dd812c5 100644 --- a/ios/chrome/browser/ui/settings/BUILD.gn +++ b/ios/chrome/browser/ui/settings/BUILD.gn
@@ -142,7 +142,6 @@ "//ios/chrome/browser/browsing_data", "//ios/chrome/browser/content_settings", "//ios/chrome/browser/history", - "//ios/chrome/browser/native_app_launcher:native_app_launcher_internal", "//ios/chrome/browser/passwords", "//ios/chrome/browser/physical_web", "//ios/chrome/browser/prefs",
diff --git a/ios/chrome/browser/ui/settings/clear_browsing_data_collection_view_controller.mm b/ios/chrome/browser/ui/settings/clear_browsing_data_collection_view_controller.mm index 8de8c2d..006b73a1 100644 --- a/ios/chrome/browser/ui/settings/clear_browsing_data_collection_view_controller.mm +++ b/ios/chrome/browser/ui/settings/clear_browsing_data_collection_view_controller.mm
@@ -273,13 +273,10 @@ // Data types section. [model addSectionWithIdentifier:SectionIdentifierDataTypes]; - int clearBrowsingHistoryMask = - IOSChromeBrowsingDataRemover::REMOVE_HISTORY | - IOSChromeBrowsingDataRemover::REMOVE_GOOGLE_APP_LAUNCHER_DATA; CollectionViewItem* browsingHistoryItem = [self clearDataItemWithType:ItemTypeDataTypeBrowsingHistory titleID:IDS_IOS_CLEAR_BROWSING_HISTORY - mask:clearBrowsingHistoryMask + mask:IOSChromeBrowsingDataRemover::REMOVE_HISTORY prefName:browsing_data::prefs::kDeleteBrowsingHistory]; [model addItem:browsingHistoryItem toSectionWithIdentifier:SectionIdentifierDataTypes]; @@ -726,9 +723,7 @@ if (!model) return; - if (data_mask & - (IOSChromeBrowsingDataRemover::REMOVE_HISTORY | - IOSChromeBrowsingDataRemover::REMOVE_GOOGLE_APP_LAUNCHER_DATA)) { + if (data_mask & IOSChromeBrowsingDataRemover::REMOVE_HISTORY) { NSIndexPath* indexPath = [self.collectionViewModel indexPathForItemType:ItemTypeDataTypeBrowsingHistory sectionIdentifier:SectionIdentifierDataTypes];
diff --git a/ios/chrome/test/BUILD.gn b/ios/chrome/test/BUILD.gn index 3ec9015..bd39f8e 100644 --- a/ios/chrome/test/BUILD.gn +++ b/ios/chrome/test/BUILD.gn
@@ -143,7 +143,6 @@ "//ios/chrome/browser/metrics:unit_tests", "//ios/chrome/browser/metrics:unit_tests_internal", "//ios/chrome/browser/native_app_launcher:unit_tests", - "//ios/chrome/browser/native_app_launcher:unit_tests_internal", "//ios/chrome/browser/net:unit_tests", "//ios/chrome/browser/omaha:unit_tests", "//ios/chrome/browser/passwords:unit_tests",
diff --git a/services/identity/identity_manager.cc b/services/identity/identity_manager.cc index e5156e25..37a452c 100644 --- a/services/identity/identity_manager.cc +++ b/services/identity/identity_manager.cc
@@ -7,6 +7,7 @@ #include <utility> #include "base/time/time.h" +#include "components/signin/core/browser/account_tracker_service.h" #include "components/signin/core/browser/signin_manager_base.h" #include "google_apis/gaia/google_service_auth_error.h" #include "mojo/public/cpp/bindings/strong_binding.h" @@ -57,24 +58,39 @@ // static void IdentityManager::Create(mojom::IdentityManagerRequest request, + AccountTrackerService* account_tracker, SigninManagerBase* signin_manager, ProfileOAuth2TokenService* token_service) { - mojo::MakeStrongBinding( - base::MakeUnique<IdentityManager>(signin_manager, token_service), - std::move(request)); + mojo::MakeStrongBinding(base::MakeUnique<IdentityManager>( + account_tracker, signin_manager, token_service), + std::move(request)); } -IdentityManager::IdentityManager(SigninManagerBase* signin_manager, +IdentityManager::IdentityManager(AccountTrackerService* account_tracker, + SigninManagerBase* signin_manager, ProfileOAuth2TokenService* token_service) - : signin_manager_(signin_manager), token_service_(token_service) {} + : account_tracker_(account_tracker), + signin_manager_(signin_manager), + token_service_(token_service) {} IdentityManager::~IdentityManager() {} void IdentityManager::GetPrimaryAccountInfo( GetPrimaryAccountInfoCallback callback) { + // It's annoying that this can't be trivially implemented in terms of + // GetAccountInfoFromGaiaId(), but there's no SigninManagerBase method that + // directly returns the authenticated GAIA ID. We can of course get it from + // the AccountInfo but once we have the ACcountInfo we ... have the + // AccountInfo. std::move(callback).Run(signin_manager_->GetAuthenticatedAccountInfo()); } +void IdentityManager::GetAccountInfoFromGaiaId( + const std::string& gaia_id, + GetAccountInfoFromGaiaIdCallback callback) { + std::move(callback).Run(account_tracker_->FindAccountInfoByGaiaId(gaia_id)); +} + void IdentityManager::GetAccessToken(const std::string& account_id, const ScopeSet& scopes, const std::string& consumer_id,
diff --git a/services/identity/identity_manager.h b/services/identity/identity_manager.h index d0cfe3c..1c767c01 100644 --- a/services/identity/identity_manager.h +++ b/services/identity/identity_manager.h
@@ -10,6 +10,7 @@ #include "services/identity/public/cpp/scope_set.h" #include "services/identity/public/interfaces/identity_manager.mojom.h" +class AccountTrackerService; class SigninManagerBase; namespace identity { @@ -17,10 +18,12 @@ class IdentityManager : public mojom::IdentityManager { public: static void Create(mojom::IdentityManagerRequest request, + AccountTrackerService* account_tracker, SigninManagerBase* signin_manager, ProfileOAuth2TokenService* token_service); - IdentityManager(SigninManagerBase* signin_manager, + IdentityManager(AccountTrackerService* account_tracker, + SigninManagerBase* signin_manager, ProfileOAuth2TokenService* token_service); ~IdentityManager() override; @@ -61,6 +64,9 @@ // mojom::IdentityManager: void GetPrimaryAccountInfo(GetPrimaryAccountInfoCallback callback) override; + void GetAccountInfoFromGaiaId( + const std::string& gaia_id, + GetAccountInfoFromGaiaIdCallback callback) override; void GetAccessToken(const std::string& account_id, const ScopeSet& scopes, const std::string& consumer_id, @@ -69,6 +75,7 @@ // Deletes |request|. void AccessTokenRequestCompleted(AccessTokenRequest* request); + AccountTrackerService* account_tracker_; SigninManagerBase* signin_manager_; ProfileOAuth2TokenService* token_service_;
diff --git a/services/identity/identity_manager_unittest.cc b/services/identity/identity_manager_unittest.cc index 8f44365..493a0b6 100644 --- a/services/identity/identity_manager_unittest.cc +++ b/services/identity/identity_manager_unittest.cc
@@ -31,9 +31,11 @@ public service_manager::mojom::ServiceFactory { public: ServiceTestClient(service_manager::test::ServiceTest* test, + AccountTrackerService* account_tracker, SigninManagerBase* signin_manager, ProfileOAuth2TokenService* token_service) : service_manager::test::ServiceTestClient(test), + account_tracker_(account_tracker), signin_manager_(signin_manager), token_service_(token_service) { registry_.AddInterface<service_manager::mojom::ServiceFactory>( @@ -52,7 +54,8 @@ const std::string& name) override { if (name == mojom::kServiceName) { identity_service_context_.reset(new service_manager::ServiceContext( - base::MakeUnique<IdentityService>(signin_manager_, token_service_), + base::MakeUnique<IdentityService>(account_tracker_, signin_manager_, + token_service_), std::move(request))); } } @@ -63,6 +66,7 @@ } private: + AccountTrackerService* account_tracker_; SigninManagerBase* signin_manager_; ProfileOAuth2TokenService* token_service_; service_manager::BinderRegistry registry_; @@ -91,6 +95,13 @@ quit_closure.Run(); } + void OnReceivedAccountInfoFromGaiaId( + base::Closure quit_closure, + const base::Optional<AccountInfo>& account_info) { + account_info_from_gaia_id_ = account_info; + quit_closure.Run(); + } + void OnReceivedAccessToken(base::Closure quit_closure, const base::Optional<std::string>& access_token, base::Time expiration_time, @@ -109,15 +120,17 @@ // service_manager::test::ServiceTest: std::unique_ptr<service_manager::Service> CreateService() override { - return base::MakeUnique<ServiceTestClient>(this, &signin_manager_, - &token_service_); + return base::MakeUnique<ServiceTestClient>( + this, &account_tracker_, &signin_manager_, &token_service_); } mojom::IdentityManagerPtr identity_manager_; base::Optional<AccountInfo> primary_account_info_; + base::Optional<AccountInfo> account_info_from_gaia_id_; base::Optional<std::string> access_token_; GoogleServiceAuthError access_token_error_; + AccountTrackerService* account_tracker() { return &account_tracker_; } SigninManagerBase* signin_manager() { return &signin_manager_; } FakeProfileOAuth2TokenService* token_service() { return &token_service_; } @@ -131,8 +144,8 @@ DISALLOW_COPY_AND_ASSIGN(IdentityManagerTest); }; -// Check that the primary account ID is null if not signed in. -TEST_F(IdentityManagerTest, GetPrimaryAccountNotSignedIn) { +// Check that the primary account info is null if not signed in. +TEST_F(IdentityManagerTest, GetPrimaryAccountInfoNotSignedIn) { base::RunLoop run_loop; identity_manager_->GetPrimaryAccountInfo( base::Bind(&IdentityManagerTest::OnReceivedPrimaryAccountInfo, @@ -141,8 +154,8 @@ EXPECT_FALSE(primary_account_info_); } -// Check that the primary account ID has expected values if signed in. -TEST_F(IdentityManagerTest, GetPrimaryAccountSignedIn) { +// Check that the primary account info has expected values if signed in. +TEST_F(IdentityManagerTest, GetPrimaryAccountInfoSignedIn) { signin_manager()->SetAuthenticatedAccountInfo(kTestGaiaId, kTestEmail); base::RunLoop run_loop; identity_manager_->GetPrimaryAccountInfo( @@ -154,6 +167,33 @@ EXPECT_EQ(kTestEmail, primary_account_info_->email); } +// Check that the account info for a given GAIA ID is null if that GAIA ID is +// unknown. +TEST_F(IdentityManagerTest, GetAccountInfoForUnknownGaiaID) { + base::RunLoop run_loop; + identity_manager_->GetAccountInfoFromGaiaId( + kTestGaiaId, + base::Bind(&IdentityManagerTest::OnReceivedAccountInfoFromGaiaId, + base::Unretained(this), run_loop.QuitClosure())); + run_loop.Run(); + EXPECT_FALSE(account_info_from_gaia_id_); +} + +// Check that the account info for a given GAIA ID has expected values if that +// GAIA ID is known. +TEST_F(IdentityManagerTest, GetAccountInfoForKnownGaiaId) { + account_tracker()->SeedAccountInfo(kTestGaiaId, kTestEmail); + base::RunLoop run_loop; + identity_manager_->GetAccountInfoFromGaiaId( + kTestGaiaId, + base::Bind(&IdentityManagerTest::OnReceivedAccountInfoFromGaiaId, + base::Unretained(this), run_loop.QuitClosure())); + run_loop.Run(); + EXPECT_TRUE(account_info_from_gaia_id_); + EXPECT_EQ(kTestGaiaId, account_info_from_gaia_id_->gaia); + EXPECT_EQ(kTestEmail, account_info_from_gaia_id_->email); +} + // Check that the expected error is received if requesting an access token when // not signed in. TEST_F(IdentityManagerTest, GetAccessTokenNotSignedIn) {
diff --git a/services/identity/identity_service.cc b/services/identity/identity_service.cc index b5f9df7b..4043f538 100644 --- a/services/identity/identity_service.cc +++ b/services/identity/identity_service.cc
@@ -9,9 +9,12 @@ namespace identity { -IdentityService::IdentityService(SigninManagerBase* signin_manager, +IdentityService::IdentityService(AccountTrackerService* account_tracker, + SigninManagerBase* signin_manager, ProfileOAuth2TokenService* token_service) - : signin_manager_(signin_manager), token_service_(token_service) { + : account_tracker_(account_tracker), + signin_manager_(signin_manager), + token_service_(token_service) { registry_.AddInterface<mojom::IdentityManager>( base::Bind(&IdentityService::Create, base::Unretained(this))); } @@ -30,7 +33,8 @@ void IdentityService::Create(const service_manager::BindSourceInfo& source_info, mojom::IdentityManagerRequest request) { - IdentityManager::Create(std::move(request), signin_manager_, token_service_); + IdentityManager::Create(std::move(request), account_tracker_, signin_manager_, + token_service_); } } // namespace identity
diff --git a/services/identity/identity_service.h b/services/identity/identity_service.h index 472a911..1aa513d 100644 --- a/services/identity/identity_service.h +++ b/services/identity/identity_service.h
@@ -9,6 +9,7 @@ #include "services/service_manager/public/cpp/binder_registry.h" #include "services/service_manager/public/cpp/service.h" +class AccountTrackerService; class SigninManagerBase; class ProfileOAuth2TokenService; @@ -16,7 +17,8 @@ class IdentityService : public service_manager::Service { public: - IdentityService(SigninManagerBase* signin_manager, + IdentityService(AccountTrackerService* account_tracker, + SigninManagerBase* signin_manager, ProfileOAuth2TokenService* token_service); ~IdentityService() override; @@ -30,6 +32,7 @@ void Create(const service_manager::BindSourceInfo& source_info, mojom::IdentityManagerRequest request); + AccountTrackerService* account_tracker_; SigninManagerBase* signin_manager_; ProfileOAuth2TokenService* token_service_;
diff --git a/services/identity/public/interfaces/identity_manager.mojom b/services/identity/public/interfaces/identity_manager.mojom index ef497bcf..c7a7ca4 100644 --- a/services/identity/public/interfaces/identity_manager.mojom +++ b/services/identity/public/interfaces/identity_manager.mojom
@@ -16,6 +16,10 @@ // are not signed in). GetPrimaryAccountInfo() => (AccountInfo? account_info); + // Returns the AccountInfo for the user's Google account corresponding to + // |gaia_id|, or null if the user has such corresponding account. + GetAccountInfoFromGaiaId(string gaia_id) => (AccountInfo? account_info); + // Returns an access token with the requested scopes for the given // |account_id|, or null if it is not possible to obtain such a token (e.g., // because the user is not signed in with that account). |expiration_time|
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index 42dea08..6a7a611 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -1509,6 +1509,11 @@ crbug.com/553838 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/writing-modes-3/text-combine-upright-compression-006.html [ Failure ] crbug.com/553838 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/writing-modes-3/text-combine-upright-compression-006a.html [ Failure ] crbug.com/553838 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/writing-modes-3/text-combine-upright-compression-007.html [ Failure ] +crbug.com/736319 [ Linux Mac ] external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/writing-modes-3/text-combine-upright-compression-001.html [ Failure Pass ] +crbug.com/736319 [ Linux ] external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/writing-modes-3/text-combine-upright-compression-002.html [ Failure Pass ] +crbug.com/736319 [ Linux ] external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/writing-modes-3/text-combine-upright-compression-003.html [ Failure Pass ] +crbug.com/736319 [ Linux Mac ] external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/writing-modes-3/text-combine-upright-compression-004.html [ Failure Pass ] +crbug.com/736319 [ Linux ] external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/writing-modes-3/text-combine-upright-compression-005.html [ Failure Pass ] # These need a rebaseline due crbug.com/504745 on Windows when they are activated again. crbug.com/521124 crbug.com/410145 [ Win7 ] fast/css/font-weight-1.html [ Pass Failure ] @@ -2656,7 +2661,7 @@ crbug.com/708499 crbug.com/724817 fast/compositor-wheel-scroll-latching/touchpad-scroll-impl-to-main.html [ Pass Timeout Crash ] crbug.com/708499 crbug.com/724817 virtual/wheelscrolllatching/fast/compositor-wheel-scroll-latching/touchpad-scroll-impl-to-main.html [ Pass Timeout Crash ] -crbug.com/706091 [ Linux ] virtual/wheelscrolllatching/fast/events/wheel/wheel-scroll-latching-on-scrollbar.html [ Failure Pass ] +crbug.com/706091 virtual/wheelscrolllatching/fast/events/wheel/wheel-scroll-latching-on-scrollbar.html [ Failure Pass ] # Sheriff failures 2017-04-24 crbug.com/714862 [ Android ] tables/mozilla/collapsing_borders/bug41262-3.html [ Failure ] @@ -2834,63 +2839,128 @@ crbug.com/734762 [ Mac Debug ] inspector-protocol/timeline/page-frames.js [ Failure ] -crbug.com/736255 [ Mac ] external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/writing-modes-3/text-combine-upright-compression-001.html [ Skip ] +crbug.com/736255 [ Mac ] external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-baseline-multi-item-vert-001b.html [ Skip ] # These tests crash, fail or are flaky on Mac 10.9 crbug.com/736257 [ Mac10.9 ] editing/selection/modify_move/move_left_right_character_in_mixed_bidi.html [ Skip ] crbug.com/736257 [ Mac10.9 ] external/wpt/css/css-writing-modes-3/bidi-isolate-override-007.html [ Skip ] crbug.com/736257 [ Mac10.9 ] fast/text-autosizing/tables/css-table-single-cell-lots-of-text.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text-autosizing/tables/lots-of-text-many-cells.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text-autosizing/tables/narrow-percentage-width.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text-autosizing/tables/nested-table-wrapping.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text-autosizing/tables/nested-tables.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text-autosizing/tables/table-cell-inflation.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text-autosizing/tables/table-for-layout.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text-autosizing/tables/wide-percentage-width.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text-autosizing/tables/wide-specified-width.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/atsui-kerning-and-ligatures.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/atsui-partial-selection.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/atsui-pointtooffset-calls-cg.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/atsui-rtl-override-selection.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/basic/002.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/basic/003.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/basic/005.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/basic/008.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/basic/011.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/basic/012.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/basic/013.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/basic/generic-family-changes.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/bidi-img-alt-text.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/break-word.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/capitalize-boundaries.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/capitalize-empty-generated-string.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/capitalize-preserve-nbsp.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/complex-text-rtl-selection-repaint.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/delete-hard-break-character.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/ellipsis-at-edge-of-ltr-text-in-rtl-flow.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/ellipsis-at-edge-of-rtl-text-in-ltr-flow.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/ellipsis-ltr-text-in-ltr-flow-underline-composition.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/ellipsis-ltr-text-in-ltr-flow-underline.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/ellipsis-ltr-text-in-rtl-flow-leading-space.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/ellipsis-ltr-text-in-rtl-flow-underline-composition.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/ellipsis-ltr-text-in-rtl-flow.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/ellipsis-mixed-text-in-ltr-flow-underline-2.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/ellipsis-rtl-text-in-ltr-flow-underline-composition.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/ellipsis-rtl-text-in-ltr-flow-underline.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/ellipsis-rtl-text-in-ltr-flow.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/ellipsis-rtl-text-in-rtl-flow-underline.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/emoji-web-font.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/emphasis-complex.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/fallback-for-custom-font.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/firstline/001.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/font-fallback.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/font-features/caps-native-synthesis.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/font-initial.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/font-stretch.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/font-weight-variant.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/font-weight.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/international/001.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/international/bidi-AN-after-L.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/international/bidi-CS-after-AN.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/international/bidi-L2-run-reordering.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/international/bidi-LDB-2-CSS.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/international/bidi-LDB-2-HTML.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/international/bidi-LDB-2-formatting-characters.html [ Skip ] +crbug.com/736257 [ Mac10.9 ] fast/text/international/bidi-european-terminators.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/international/bidi-explicit-embedding.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/international/bidi-ignored-for-first-child-inline.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/international/bidi-layout-across-linebreak.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/international/bidi-linebreak-002.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/international/bidi-linebreak-003.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/international/bidi-listbox-atsui.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/international/bidi-neutral-directionality-paragraph-start.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/international/bidi-neutral-run.html [ Skip ] +crbug.com/736257 [ Mac10.9 ] fast/text/international/danda-space.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/international/hebrew-vowels.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/international/hindi-spacing.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/international/mixed-directionality-selection.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/international/rtl-caret.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/international/rtl-white-space-pre-wrap.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/international/thai-baht-space.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/international/unicode-bidi-plaintext.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/justified-selection-at-edge.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/justified-selection.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/justify-ideograph-complex.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/justify-ideograph-simple.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/justify-ideograph-vertical.html [ Skip ] crbug.com/736257 [ Mac10.9 ] fast/text/midword-break-after-breakable-char.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/midword-break-hang.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/place-ellipsis-in-inline-block-adjacent-float.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/place-ellipsis-in-inline-blocks-align-center.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/place-ellipsis-in-inline-blocks-align-right.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/place-mixed-ellipsis-in-inline-blocks-align-left-2.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/place-mixed-ellipsis-in-inline-blocks-align-right-2.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/place-mixed-ellipsis-in-inline-blocks.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/place-rtl-ellipsis-in-inline-blocks-2.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/place-rtl-ellipsis-in-inline-blocks-align-left.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/reset-emptyRun.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/selection-rect-line-height-too-big.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/selection-rect-line-height-too-small.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/shadow-translucent-fill.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/should-use-atsui.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/small-caps-turkish.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/softHyphen.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/text-stroke-with-border.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/vertical-rl-rtl-linebreak.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/vertical-surrogate-pair.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/wbr-styled.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/whitespace/001.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/whitespace/004.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/whitespace/005.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/whitespace/006.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/whitespace/008.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/whitespace/010.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/whitespace/011.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/whitespace/012.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/whitespace/016.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/whitespace/021.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/whitespace/026.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/whitespace/027.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/whitespace/028.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/whitespace/030.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] fast/text/whitespace/pre-wrap-line-test.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/whitespace/span-in-word-space-causes-overflow.html [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] fast/text/word-break.html [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] ietestcenter/css3/text/textshadow-003.htm [ Failure Pass ] +crbug.com/736257 [ Mac10.9 ] ietestcenter/css3/text/textshadow-004.htm [ Failure Pass ] crbug.com/736257 [ Mac10.9 ] inspector/elements/bidi-dom-tree.html [ Skip ]
diff --git a/third_party/WebKit/Source/core/css/SelectorFilter.h b/third_party/WebKit/Source/core/css/SelectorFilter.h index c9653da..8c3f7d8b 100644 --- a/third_party/WebKit/Source/core/css/SelectorFilter.h +++ b/third_party/WebKit/Source/core/css/SelectorFilter.h
@@ -40,7 +40,7 @@ class CSSSelector; -class SelectorFilter { +class CORE_EXPORT SelectorFilter { WTF_MAKE_NONCOPYABLE(SelectorFilter); DISALLOW_NEW();
diff --git a/third_party/WebKit/Source/core/dom/ContainerNode.cpp b/third_party/WebKit/Source/core/dom/ContainerNode.cpp index 8d530f01..5c177c7 100644 --- a/third_party/WebKit/Source/core/dom/ContainerNode.cpp +++ b/third_party/WebKit/Source/core/dom/ContainerNode.cpp
@@ -905,9 +905,13 @@ } DISABLE_CFI_PERF -void ContainerNode::AttachLayoutTree(const AttachContext& context) { +void ContainerNode::AttachLayoutTree(AttachContext& context) { AttachContext children_context(context); children_context.resolved_style = nullptr; + bool clear_previous_in_flow = !!GetLayoutObject(); + if (clear_previous_in_flow) + children_context.previous_in_flow = nullptr; + children_context.use_previous_in_flow = true; for (Node* child = firstChild(); child; child = child->nextSibling()) { #if DCHECK_IS_ON() @@ -918,6 +922,9 @@ child->AttachLayoutTree(children_context); } + if (children_context.previous_in_flow && !clear_previous_in_flow) + context.previous_in_flow = children_context.previous_in_flow; + ClearChildNeedsStyleRecalc(); ClearChildNeedsReattachLayoutTree(); Node::AttachLayoutTree(context);
diff --git a/third_party/WebKit/Source/core/dom/ContainerNode.h b/third_party/WebKit/Source/core/dom/ContainerNode.h index e7d515b..84123c2 100644 --- a/third_party/WebKit/Source/core/dom/ContainerNode.h +++ b/third_party/WebKit/Source/core/dom/ContainerNode.h
@@ -143,7 +143,7 @@ void CloneChildNodes(ContainerNode* clone); - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; void DetachLayoutTree(const AttachContext& = AttachContext()) override; LayoutRect BoundingBox() const final; void SetFocused(bool, WebFocusType) override;
diff --git a/third_party/WebKit/Source/core/dom/Document.cpp b/third_party/WebKit/Source/core/dom/Document.cpp index 96f33e2..5f661d84 100644 --- a/third_party/WebKit/Source/core/dom/Document.cpp +++ b/third_party/WebKit/Source/core/dom/Document.cpp
@@ -2534,7 +2534,8 @@ layout_view_->Compositor()->SetNeedsCompositingUpdate( kCompositingUpdateAfterCompositingInputChange); - ContainerNode::AttachLayoutTree(); + AttachContext context; + ContainerNode::AttachLayoutTree(context); // The TextAutosizer can't update layout view info while the Document is // detached, so update now in case anything changed.
diff --git a/third_party/WebKit/Source/core/dom/Document.h b/third_party/WebKit/Source/core/dom/Document.h index d7ba059..731c536 100644 --- a/third_party/WebKit/Source/core/dom/Document.h +++ b/third_party/WebKit/Source/core/dom/Document.h
@@ -547,9 +547,7 @@ void Initialize(); virtual void Shutdown(); - void AttachLayoutTree(const AttachContext& = AttachContext()) override { - NOTREACHED(); - } + void AttachLayoutTree(AttachContext&) override { NOTREACHED(); } void DetachLayoutTree(const AttachContext& = AttachContext()) override { NOTREACHED(); }
diff --git a/third_party/WebKit/Source/core/dom/Element.cpp b/third_party/WebKit/Source/core/dom/Element.cpp index 1dc4c6f..6468370 100644 --- a/third_party/WebKit/Source/core/dom/Element.cpp +++ b/third_party/WebKit/Source/core/dom/Element.cpp
@@ -1785,7 +1785,7 @@ GetDocument().GetFrame()->GetEventHandler().ElementRemoved(this); } -void Element::AttachLayoutTree(const AttachContext& context) { +void Element::AttachLayoutTree(AttachContext& context) { DCHECK(GetDocument().InStyleRecalc()); // We've already been through detach when doing an attach, but we might @@ -3456,7 +3456,9 @@ if (pseudo_id == kPseudoIdBackdrop) GetDocument().AddToTopLayer(element, this); element->InsertedInto(this); - element->AttachLayoutTree(); + + AttachContext context; + element->AttachLayoutTree(context); probe::pseudoElementCreated(element);
diff --git a/third_party/WebKit/Source/core/dom/Element.h b/third_party/WebKit/Source/core/dom/Element.h index 1ba12344..c9a79e5 100644 --- a/third_party/WebKit/Source/core/dom/Element.h +++ b/third_party/WebKit/Source/core/dom/Element.h
@@ -432,7 +432,7 @@ virtual void CopyNonAttributePropertiesFromElement(const Element&) {} - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; void DetachLayoutTree(const AttachContext& = AttachContext()) override; virtual LayoutObject* CreateLayoutObject(const ComputedStyle&);
diff --git a/third_party/WebKit/Source/core/dom/FirstLetterPseudoElement.cpp b/third_party/WebKit/Source/core/dom/FirstLetterPseudoElement.cpp index f8fddbc..0a89711 100644 --- a/third_party/WebKit/Source/core/dom/FirstLetterPseudoElement.cpp +++ b/third_party/WebKit/Source/core/dom/FirstLetterPseudoElement.cpp
@@ -239,7 +239,7 @@ remaining_text_layout_object_ = fragment; } -void FirstLetterPseudoElement::AttachLayoutTree(const AttachContext& context) { +void FirstLetterPseudoElement::AttachLayoutTree(AttachContext& context) { PseudoElement::AttachLayoutTree(context); AttachFirstLetterTextLayoutObjects(); }
diff --git a/third_party/WebKit/Source/core/dom/FirstLetterPseudoElement.h b/third_party/WebKit/Source/core/dom/FirstLetterPseudoElement.h index 8296e40..148e0af 100644 --- a/third_party/WebKit/Source/core/dom/FirstLetterPseudoElement.h +++ b/third_party/WebKit/Source/core/dom/FirstLetterPseudoElement.h
@@ -55,7 +55,7 @@ void UpdateTextFragments(); - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; void DetachLayoutTree(const AttachContext& = AttachContext()) override; private:
diff --git a/third_party/WebKit/Source/core/dom/Node.cpp b/third_party/WebKit/Source/core/dom/Node.cpp index 789e0df..8db2bf1 100644 --- a/third_party/WebKit/Source/core/dom/Node.cpp +++ b/third_party/WebKit/Source/core/dom/Node.cpp
@@ -1007,28 +1007,32 @@ return nullptr; } -void Node::ReattachLayoutTree(const AttachContext& context) { - AttachContext reattach_context(context); - reattach_context.performing_reattach = true; +void Node::ReattachLayoutTree(AttachContext& context) { + context.performing_reattach = true; // We only need to detach if the node has already been through // attachLayoutTree(). if (GetStyleChangeType() < kNeedsReattachStyleChange) - DetachLayoutTree(reattach_context); - AttachLayoutTree(reattach_context); + DetachLayoutTree(context); + AttachLayoutTree(context); } -void Node::AttachLayoutTree(const AttachContext&) { +void Node::AttachLayoutTree(AttachContext& context) { DCHECK(GetDocument().InStyleRecalc() || IsDocumentNode()); DCHECK(!GetDocument().Lifecycle().InDetach()); DCHECK(NeedsAttach()); - DCHECK(!GetLayoutObject() || - (GetLayoutObject()->Style() && - (GetLayoutObject()->Parent() || GetLayoutObject()->IsLayoutView()))); + + LayoutObject* layout_object = GetLayoutObject(); + DCHECK(!layout_object || + (layout_object->Style() && + (layout_object->Parent() || layout_object->IsLayoutView()))); ClearNeedsStyleRecalc(); ClearNeedsReattachLayoutTree(); + if (layout_object && !layout_object->IsFloatingOrOutOfFlowPositioned()) + context.previous_in_flow = layout_object; + if (AXObjectCache* cache = GetDocument().AxObjectCache()) cache->UpdateCacheAfterNodeIsAttached(this); }
diff --git a/third_party/WebKit/Source/core/dom/Node.h b/third_party/WebKit/Source/core/dom/Node.h index 6ca2e50c..aacf220 100644 --- a/third_party/WebKit/Source/core/dom/Node.h +++ b/third_party/WebKit/Source/core/dom/Node.h
@@ -625,8 +625,14 @@ struct AttachContext { STACK_ALLOCATED(); ComputedStyle* resolved_style = nullptr; + // Keep track of previously attached in-flow box during attachment so that + // we don't need to backtrack past display:none/contents and out of flow + // objects when we need to do whitespace re-attachment. + LayoutObject* previous_in_flow = nullptr; bool performing_reattach = false; bool clear_invalidation = false; + // True if the previous_in_flow member is up-to-date, even if it is nullptr. + bool use_previous_in_flow = false; AttachContext() {} }; @@ -635,14 +641,18 @@ // applied to the node and creates an appropriate LayoutObject which will be // inserted into the tree (except when the style has display: none). This // makes the node visible in the LocalFrameView. - virtual void AttachLayoutTree(const AttachContext& = AttachContext()); + virtual void AttachLayoutTree(AttachContext&); // Detaches the node from the layout tree, making it invisible in the rendered // view. This method will remove the node's layout object from the layout tree // and delete it. virtual void DetachLayoutTree(const AttachContext& = AttachContext()); - void ReattachLayoutTree(const AttachContext& = AttachContext()); + void ReattachLayoutTree() { + AttachContext context; + ReattachLayoutTree(context); + } + void ReattachLayoutTree(AttachContext&); void LazyReattachIfAttached(); // Returns true if recalcStyle should be called on the object, if there is
diff --git a/third_party/WebKit/Source/core/dom/NodeTest.cpp b/third_party/WebKit/Source/core/dom/NodeTest.cpp index 749e49c..0c248b9 100644 --- a/third_party/WebKit/Source/core/dom/NodeTest.cpp +++ b/third_party/WebKit/Source/core/dom/NodeTest.cpp
@@ -4,11 +4,40 @@ #include "core/dom/Node.h" +#include "bindings/core/v8/V8BindingForCore.h" +#include "core/css/resolver/StyleResolver.h" +#include "core/dom/shadow/ShadowRootInit.h" #include "core/editing/EditingTestBase.h" namespace blink { -class NodeTest : public EditingTestBase {}; +class NodeTest : public EditingTestBase { + protected: + ShadowRoot* AttachShadowTo(Element* element) { + ShadowRootInit shadow_root_init; + shadow_root_init.setMode("open"); + return element->attachShadow( + ToScriptStateForMainWorld(GetDocument().GetFrame()), shadow_root_init, + ASSERT_NO_EXCEPTION); + } + + LayoutObject* ReattachLayoutTreeForNode(Node& node) { + GetDocument().Lifecycle().AdvanceTo(DocumentLifecycle::kInStyleRecalc); + PushSelectorFilterAncestors( + GetDocument().EnsureStyleResolver().GetSelectorFilter(), node); + Node::AttachContext context; + node.ReattachLayoutTree(context); + return context.previous_in_flow; + } + + private: + void PushSelectorFilterAncestors(SelectorFilter& filter, Node& node) { + if (Element* parent = FlatTreeTraversal::ParentElement(node)) { + PushSelectorFilterAncestors(filter, *parent); + filter.PushParent(*parent); + } + } +}; TEST_F(NodeTest, canStartSelection) { const char* body_content = @@ -54,4 +83,182 @@ EXPECT_EQ(Node::kV0NotCustomElement, div->GetV0CustomElementState()); } +TEST_F(NodeTest, AttachContext_PreviousInFlow_TextRoot) { + SetBodyContent("Text"); + Node* root = GetDocument().body()->firstChild(); + LayoutObject* previous_in_flow = ReattachLayoutTreeForNode(*root); + + EXPECT_TRUE(previous_in_flow); + EXPECT_EQ(root->GetLayoutObject(), previous_in_flow); +} + +TEST_F(NodeTest, AttachContext_PreviousInFlow_InlineRoot) { + SetBodyContent("<span id=root>Text <span></span></span>"); + Element* root = GetDocument().getElementById("root"); + LayoutObject* previous_in_flow = ReattachLayoutTreeForNode(*root); + + EXPECT_TRUE(previous_in_flow); + EXPECT_EQ(root->GetLayoutObject(), previous_in_flow); +} + +TEST_F(NodeTest, AttachContext_PreviousInFlow_BlockRoot) { + SetBodyContent("<div id=root>Text <span></span></div>"); + Element* root = GetDocument().getElementById("root"); + LayoutObject* previous_in_flow = ReattachLayoutTreeForNode(*root); + + EXPECT_TRUE(previous_in_flow); + EXPECT_EQ(root->GetLayoutObject(), previous_in_flow); +} + +TEST_F(NodeTest, AttachContext_PreviousInFlow_FloatRoot) { + SetBodyContent("<div id=root style='float:left'><span></span></div>"); + Element* root = GetDocument().getElementById("root"); + LayoutObject* previous_in_flow = ReattachLayoutTreeForNode(*root); + + EXPECT_FALSE(previous_in_flow); +} + +TEST_F(NodeTest, AttachContext_PreviousInFlow_AbsoluteRoot) { + SetBodyContent("<div id=root style='position:absolute'><span></span></div>"); + Element* root = GetDocument().getElementById("root"); + LayoutObject* previous_in_flow = ReattachLayoutTreeForNode(*root); + + EXPECT_FALSE(previous_in_flow); +} + +TEST_F(NodeTest, AttachContext_PreviousInFlow_Text) { + SetBodyContent("<div id=root style='display:contents'>Text</div>"); + Element* root = GetDocument().getElementById("root"); + LayoutObject* previous_in_flow = ReattachLayoutTreeForNode(*root); + + EXPECT_TRUE(previous_in_flow); + EXPECT_EQ(root->firstChild()->GetLayoutObject(), previous_in_flow); +} + +TEST_F(NodeTest, AttachContext_PreviousInFlow_Inline) { + SetBodyContent("<div id=root style='display:contents'><span></span></div>"); + Element* root = GetDocument().getElementById("root"); + LayoutObject* previous_in_flow = ReattachLayoutTreeForNode(*root); + + EXPECT_TRUE(previous_in_flow); + EXPECT_EQ(root->firstChild()->GetLayoutObject(), previous_in_flow); +} + +TEST_F(NodeTest, AttachContext_PreviousInFlow_Block) { + SetBodyContent("<div id=root style='display:contents'><div></div></div>"); + Element* root = GetDocument().getElementById("root"); + LayoutObject* previous_in_flow = ReattachLayoutTreeForNode(*root); + + EXPECT_TRUE(previous_in_flow); + EXPECT_EQ(root->firstChild()->GetLayoutObject(), previous_in_flow); +} + +TEST_F(NodeTest, AttachContext_PreviousInFlow_Float) { + SetBodyContent( + "<style>" + " #root { display:contents }" + " .float { float:left }" + "</style>" + "<div id=root><div class=float></div></div>"); + Element* root = GetDocument().getElementById("root"); + LayoutObject* previous_in_flow = ReattachLayoutTreeForNode(*root); + + EXPECT_FALSE(previous_in_flow); +} + +TEST_F(NodeTest, AttachContext_PreviousInFlow_AbsolutePositioned) { + SetBodyContent( + "<style>" + " #root { display:contents }" + " .abs { position:absolute }" + "</style>" + "<div id=root><div class=abs></div></div>"); + Element* root = GetDocument().getElementById("root"); + LayoutObject* previous_in_flow = ReattachLayoutTreeForNode(*root); + + EXPECT_FALSE(previous_in_flow); +} + +TEST_F(NodeTest, AttachContext_PreviousInFlow_SkipAbsolute) { + SetBodyContent( + "<style>" + " #root { display:contents }" + " .abs { position:absolute }" + "</style>" + "<div id=root>" + "<div class=abs></div><span id=inline></span><div class=abs></div>" + "</div>"); + Element* root = GetDocument().getElementById("root"); + Element* span = GetDocument().getElementById("inline"); + LayoutObject* previous_in_flow = ReattachLayoutTreeForNode(*root); + + EXPECT_TRUE(previous_in_flow); + EXPECT_EQ(span->GetLayoutObject(), previous_in_flow); +} + +TEST_F(NodeTest, AttachContext_PreviousInFlow_SkipFloats) { + SetBodyContent( + "<style>" + " #root { display:contents }" + " .float { float:left }" + "</style>" + "<div id=root>" + "<div class=float></div>" + "<span id=inline></span>" + "<div class=float></div>" + "</div>"); + Element* root = GetDocument().getElementById("root"); + Element* span = GetDocument().getElementById("inline"); + LayoutObject* previous_in_flow = ReattachLayoutTreeForNode(*root); + + EXPECT_TRUE(previous_in_flow); + EXPECT_EQ(span->GetLayoutObject(), previous_in_flow); +} + +TEST_F(NodeTest, AttachContext_PreviousInFlow_InsideDisplayContents) { + SetBodyContent( + "<style>" + " #root, .contents { display:contents }" + " .float { float:left }" + "</style>" + "<div id=root>" + "<span></span><div class=contents><span id=inline></span></div>" + "</div>"); + Element* root = GetDocument().getElementById("root"); + Element* span = GetDocument().getElementById("inline"); + LayoutObject* previous_in_flow = ReattachLayoutTreeForNode(*root); + + EXPECT_TRUE(previous_in_flow); + EXPECT_EQ(span->GetLayoutObject(), previous_in_flow); +} + +TEST_F(NodeTest, AttachContext_PreviousInFlow_Slotted) { + SetBodyContent("<div id=host><span id=inline></span></div>"); + ShadowRoot* shadow_root = + AttachShadowTo(GetDocument().getElementById("host")); + shadow_root->setInnerHTML( + "<div id=root style='display:contents'><span></span><slot></slot></div>"); + GetDocument().View()->UpdateAllLifecyclePhases(); + + Element* root = shadow_root->getElementById("root"); + Element* span = GetDocument().getElementById("inline"); + LayoutObject* previous_in_flow = ReattachLayoutTreeForNode(*root); + + EXPECT_TRUE(previous_in_flow); + EXPECT_EQ(span->GetLayoutObject(), previous_in_flow); +} + +TEST_F(NodeTest, AttachContext_PreviousInFlow_V0Content) { + SetBodyContent("<div id=host><span id=inline></span></div>"); + ShadowRoot* shadow_root = CreateShadowRootForElementWithIDAndSetInnerHTML( + GetDocument(), "host", + "<div id=root style='display:contents'><span></span><content /></div>"); + Element* root = shadow_root->getElementById("root"); + Element* span = GetDocument().getElementById("inline"); + LayoutObject* previous_in_flow = ReattachLayoutTreeForNode(*root); + + EXPECT_TRUE(previous_in_flow); + EXPECT_EQ(span->GetLayoutObject(), previous_in_flow); +} + } // namespace blink
diff --git a/third_party/WebKit/Source/core/dom/PseudoElement.cpp b/third_party/WebKit/Source/core/dom/PseudoElement.cpp index 95892525..7811a2e 100644 --- a/third_party/WebKit/Source/core/dom/PseudoElement.cpp +++ b/third_party/WebKit/Source/core/dom/PseudoElement.cpp
@@ -118,7 +118,7 @@ RemovedFrom(parent); } -void PseudoElement::AttachLayoutTree(const AttachContext& context) { +void PseudoElement::AttachLayoutTree(AttachContext& context) { DCHECK(!GetLayoutObject()); Element::AttachLayoutTree(context);
diff --git a/third_party/WebKit/Source/core/dom/PseudoElement.h b/third_party/WebKit/Source/core/dom/PseudoElement.h index fb2f5d2..8680bc94 100644 --- a/third_party/WebKit/Source/core/dom/PseudoElement.h +++ b/third_party/WebKit/Source/core/dom/PseudoElement.h
@@ -38,7 +38,7 @@ static PseudoElement* Create(Element* parent, PseudoId); PassRefPtr<ComputedStyle> CustomStyleForLayoutObject() override; - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; bool LayoutObjectIsNeeded(const ComputedStyle&) override; bool CanStartSelection() const override { return false; }
diff --git a/third_party/WebKit/Source/core/dom/Text.cpp b/third_party/WebKit/Source/core/dom/Text.cpp index 09e391a0..8edf5a0 100644 --- a/third_party/WebKit/Source/core/dom/Text.cpp +++ b/third_party/WebKit/Source/core/dom/Text.cpp
@@ -351,7 +351,7 @@ return new LayoutText(this, DataImpl()); } -void Text::AttachLayoutTree(const AttachContext& context) { +void Text::AttachLayoutTree(AttachContext& context) { ContainerNode* style_parent = LayoutTreeBuilderTraversal::Parent(*this); LayoutObject* parent_layout_object = LayoutTreeBuilderTraversal::ParentLayoutObject(*this);
diff --git a/third_party/WebKit/Source/core/dom/Text.h b/third_party/WebKit/Source/core/dom/Text.h index 448b0b5..5dbffc36 100644 --- a/third_party/WebKit/Source/core/dom/Text.h +++ b/third_party/WebKit/Source/core/dom/Text.h
@@ -61,7 +61,7 @@ void UpdateTextLayoutObject(unsigned offset_of_replaced_data, unsigned length_of_replaced_data); - void AttachLayoutTree(const AttachContext& = AttachContext()) final; + void AttachLayoutTree(AttachContext&) final; void ReattachLayoutTreeIfNeeded(); bool CanContainRangeEndPoint() const final { return true; }
diff --git a/third_party/WebKit/Source/core/dom/shadow/InsertionPoint.cpp b/third_party/WebKit/Source/core/dom/shadow/InsertionPoint.cpp index 788db882..3e5c3b5 100644 --- a/third_party/WebKit/Source/core/dom/shadow/InsertionPoint.cpp +++ b/third_party/WebKit/Source/core/dom/shadow/InsertionPoint.cpp
@@ -101,15 +101,21 @@ distributed_nodes_.ShrinkToFit(); } -void InsertionPoint::AttachLayoutTree(const AttachContext& context) { +void InsertionPoint::AttachLayoutTree(AttachContext& context) { // We need to attach the distribution here so that they're inserted in the // right order otherwise the n^2 protection inside LayoutTreeBuilder will // cause them to be inserted in the wrong place later. This also lets // distributed nodes benefit from the n^2 protection. + AttachContext children_context(context); + children_context.resolved_style = nullptr; + for (size_t i = 0; i < distributed_nodes_.size(); ++i) { - if (distributed_nodes_.at(i)->NeedsAttach()) - distributed_nodes_.at(i)->AttachLayoutTree(context); + Node* child = distributed_nodes_.at(i); + if (child->NeedsAttach()) + child->AttachLayoutTree(children_context); } + if (children_context.previous_in_flow) + context.previous_in_flow = children_context.previous_in_flow; HTMLElement::AttachLayoutTree(context); }
diff --git a/third_party/WebKit/Source/core/dom/shadow/InsertionPoint.h b/third_party/WebKit/Source/core/dom/shadow/InsertionPoint.h index 2cc200ca..41535b5c 100644 --- a/third_party/WebKit/Source/core/dom/shadow/InsertionPoint.h +++ b/third_party/WebKit/Source/core/dom/shadow/InsertionPoint.h
@@ -56,7 +56,7 @@ virtual bool CanAffectSelector() const { return false; } - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; void DetachLayoutTree(const AttachContext& = AttachContext()) override; void RebuildDistributedChildrenLayoutTrees();
diff --git a/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp b/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp index 4bb2a3a..6ccfb91c 100644 --- a/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp +++ b/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.cpp
@@ -198,7 +198,7 @@ } } -void ShadowRoot::AttachLayoutTree(const AttachContext& context) { +void ShadowRoot::AttachLayoutTree(AttachContext& context) { StyleSharingDepthScope sharing_scope(*this); DocumentFragment::AttachLayoutTree(context); }
diff --git a/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.h b/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.h index fabfaaa9..cc994c1 100644 --- a/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.h +++ b/third_party/WebKit/Source/core/dom/shadow/ShadowRoot.h
@@ -92,7 +92,7 @@ GetType() == ShadowRootType::kClosed; } - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; void DetachLayoutTree(const AttachContext& = AttachContext()) override; InsertionNotificationRequest InsertedInto(ContainerNode*) override;
diff --git a/third_party/WebKit/Source/core/html/HTMLFormControlElement.cpp b/third_party/WebKit/Source/core/html/HTMLFormControlElement.cpp index c23b893..71659a9 100644 --- a/third_party/WebKit/Source/core/html/HTMLFormControlElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLFormControlElement.cpp
@@ -235,7 +235,7 @@ return true; } -void HTMLFormControlElement::AttachLayoutTree(const AttachContext& context) { +void HTMLFormControlElement::AttachLayoutTree(AttachContext& context) { HTMLElement::AttachLayoutTree(context); if (!GetLayoutObject())
diff --git a/third_party/WebKit/Source/core/html/HTMLFormControlElement.h b/third_party/WebKit/Source/core/html/HTMLFormControlElement.h index c5519bd..b41561d 100644 --- a/third_party/WebKit/Source/core/html/HTMLFormControlElement.h +++ b/third_party/WebKit/Source/core/html/HTMLFormControlElement.h
@@ -143,7 +143,7 @@ void ParseAttribute(const AttributeModificationParams&) override; virtual void RequiredAttributeChanged(); virtual void DisabledAttributeChanged(); - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; InsertionNotificationRequest InsertedInto(ContainerNode*) override; void RemovedFrom(ContainerNode*) override; void WillChangeForm() override;
diff --git a/third_party/WebKit/Source/core/html/HTMLFrameElement.cpp b/third_party/WebKit/Source/core/html/HTMLFrameElement.cpp index 502fb03..af8e424 100644 --- a/third_party/WebKit/Source/core/html/HTMLFrameElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLFrameElement.cpp
@@ -52,7 +52,7 @@ return hasAttribute(noresizeAttr); } -void HTMLFrameElement::AttachLayoutTree(const AttachContext& context) { +void HTMLFrameElement::AttachLayoutTree(AttachContext& context) { HTMLFrameElementBase::AttachLayoutTree(context); if (HTMLFrameSetElement* frame_set_element =
diff --git a/third_party/WebKit/Source/core/html/HTMLFrameElement.h b/third_party/WebKit/Source/core/html/HTMLFrameElement.h index 09f44c43..e80e8e5 100644 --- a/third_party/WebKit/Source/core/html/HTMLFrameElement.h +++ b/third_party/WebKit/Source/core/html/HTMLFrameElement.h
@@ -45,7 +45,7 @@ private: explicit HTMLFrameElement(Document&); - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; bool LayoutObjectIsNeeded(const ComputedStyle&) override; LayoutObject* CreateLayoutObject(const ComputedStyle&) override;
diff --git a/third_party/WebKit/Source/core/html/HTMLFrameElementBase.cpp b/third_party/WebKit/Source/core/html/HTMLFrameElementBase.cpp index 1f6289e8..8102e430 100644 --- a/third_party/WebKit/Source/core/html/HTMLFrameElementBase.cpp +++ b/third_party/WebKit/Source/core/html/HTMLFrameElementBase.cpp
@@ -207,7 +207,7 @@ SetNameAndOpenURL(); } -void HTMLFrameElementBase::AttachLayoutTree(const AttachContext& context) { +void HTMLFrameElementBase::AttachLayoutTree(AttachContext& context) { HTMLFrameOwnerElement::AttachLayoutTree(context); if (GetLayoutEmbeddedContent() && ContentFrame())
diff --git a/third_party/WebKit/Source/core/html/HTMLFrameElementBase.h b/third_party/WebKit/Source/core/html/HTMLFrameElementBase.h index e8af00f..537ec6a 100644 --- a/third_party/WebKit/Source/core/html/HTMLFrameElementBase.h +++ b/third_party/WebKit/Source/core/html/HTMLFrameElementBase.h
@@ -49,7 +49,7 @@ InsertionNotificationRequest InsertedInto(ContainerNode*) override; void DidNotifySubtreeInsertionsToDocument() final; - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; void SetScrollingMode(ScrollbarMode); void SetMarginWidth(int);
diff --git a/third_party/WebKit/Source/core/html/HTMLFrameSetElement.cpp b/third_party/WebKit/Source/core/html/HTMLFrameSetElement.cpp index e20b0f7e..1a3df56 100644 --- a/third_party/WebKit/Source/core/html/HTMLFrameSetElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLFrameSetElement.cpp
@@ -230,7 +230,7 @@ return new LayoutFrameSet(this); } -void HTMLFrameSetElement::AttachLayoutTree(const AttachContext& context) { +void HTMLFrameSetElement::AttachLayoutTree(AttachContext& context) { // Inherit default settings from parent frameset // FIXME: This is not dynamic. if (HTMLFrameSetElement* frameset =
diff --git a/third_party/WebKit/Source/core/html/HTMLFrameSetElement.h b/third_party/WebKit/Source/core/html/HTMLFrameSetElement.h index 7375d62..2c540cd 100644 --- a/third_party/WebKit/Source/core/html/HTMLFrameSetElement.h +++ b/third_party/WebKit/Source/core/html/HTMLFrameSetElement.h
@@ -68,7 +68,7 @@ const AtomicString&, MutableStylePropertySet*) override; - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; bool LayoutObjectIsNeeded(const ComputedStyle&) override; LayoutObject* CreateLayoutObject(const ComputedStyle&) override;
diff --git a/third_party/WebKit/Source/core/html/HTMLImageElement.cpp b/third_party/WebKit/Source/core/html/HTMLImageElement.cpp index d3bc4c4..d443ea6 100644 --- a/third_party/WebKit/Source/core/html/HTMLImageElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLImageElement.cpp
@@ -414,7 +414,7 @@ } } -void HTMLImageElement::AttachLayoutTree(const AttachContext& context) { +void HTMLImageElement::AttachLayoutTree(AttachContext& context) { HTMLElement::AttachLayoutTree(context); if (GetLayoutObject() && GetLayoutObject()->IsImage()) { LayoutImage* layout_image = ToLayoutImage(GetLayoutObject());
diff --git a/third_party/WebKit/Source/core/html/HTMLImageElement.h b/third_party/WebKit/Source/core/html/HTMLImageElement.h index c8e0200..6b38f1a 100644 --- a/third_party/WebKit/Source/core/html/HTMLImageElement.h +++ b/third_party/WebKit/Source/core/html/HTMLImageElement.h
@@ -172,7 +172,7 @@ MutableStylePropertySet*) override; void SetLayoutDisposition(LayoutDisposition, bool force_reattach = false); - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; LayoutObject* CreateLayoutObject(const ComputedStyle&) override; bool CanStartSelection() const override { return false; }
diff --git a/third_party/WebKit/Source/core/html/HTMLInputElement.cpp b/third_party/WebKit/Source/core/html/HTMLInputElement.cpp index 56d1871..ddbdf97 100644 --- a/third_party/WebKit/Source/core/html/HTMLInputElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLInputElement.cpp
@@ -839,7 +839,7 @@ return input_type_view_->CreateLayoutObject(style); } -void HTMLInputElement::AttachLayoutTree(const AttachContext& context) { +void HTMLInputElement::AttachLayoutTree(AttachContext& context) { TextControlElement::AttachLayoutTree(context); if (GetLayoutObject()) { input_type_->OnAttachWithLayoutObject();
diff --git a/third_party/WebKit/Source/core/html/HTMLInputElement.h b/third_party/WebKit/Source/core/html/HTMLInputElement.h index 02af52c0..31c6ccc8 100644 --- a/third_party/WebKit/Source/core/html/HTMLInputElement.h +++ b/third_party/WebKit/Source/core/html/HTMLInputElement.h
@@ -346,7 +346,7 @@ void CopyNonAttributePropertiesFromElement(const Element&) final; - void AttachLayoutTree(const AttachContext& = AttachContext()) final; + void AttachLayoutTree(AttachContext&) final; void AppendToFormData(FormData&) final; String ResultForDialogSubmit() final;
diff --git a/third_party/WebKit/Source/core/html/HTMLLIElement.cpp b/third_party/WebKit/Source/core/html/HTMLLIElement.cpp index 5bb7beb..40329761 100644 --- a/third_party/WebKit/Source/core/html/HTMLLIElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLLIElement.cpp
@@ -90,7 +90,7 @@ } } -void HTMLLIElement::AttachLayoutTree(const AttachContext& context) { +void HTMLLIElement::AttachLayoutTree(AttachContext& context) { HTMLElement::AttachLayoutTree(context); if (GetLayoutObject() && GetLayoutObject()->IsListItem()) {
diff --git a/third_party/WebKit/Source/core/html/HTMLLIElement.h b/third_party/WebKit/Source/core/html/HTMLLIElement.h index 2a767c73..5525c1e 100644 --- a/third_party/WebKit/Source/core/html/HTMLLIElement.h +++ b/third_party/WebKit/Source/core/html/HTMLLIElement.h
@@ -42,7 +42,7 @@ const AtomicString&, MutableStylePropertySet*) override; - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; void ParseValue(const AtomicString&); };
diff --git a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp index edda6f9f..3bf780d 100644 --- a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
@@ -716,7 +716,7 @@ } } -void HTMLMediaElement::AttachLayoutTree(const AttachContext& context) { +void HTMLMediaElement::AttachLayoutTree(AttachContext& context) { HTMLElement::AttachLayoutTree(context); if (GetLayoutObject())
diff --git a/third_party/WebKit/Source/core/html/HTMLMediaElement.h b/third_party/WebKit/Source/core/html/HTMLMediaElement.h index 4154c9d..88b604d9 100644 --- a/third_party/WebKit/Source/core/html/HTMLMediaElement.h +++ b/third_party/WebKit/Source/core/html/HTMLMediaElement.h
@@ -340,7 +340,7 @@ void ParseAttribute(const AttributeModificationParams&) override; void FinishParsingChildren() final; bool IsURLAttribute(const Attribute&) const override; - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; InsertionNotificationRequest InsertedInto(ContainerNode*) override; void RemovedFrom(ContainerNode*) override;
diff --git a/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp b/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp index 6f0ec4f..237c1399 100644 --- a/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLOptionElement.cpp
@@ -87,7 +87,7 @@ return element; } -void HTMLOptionElement::AttachLayoutTree(const AttachContext& context) { +void HTMLOptionElement::AttachLayoutTree(AttachContext& context) { AttachContext option_context(context); RefPtr<ComputedStyle> resolved_style; if (!context.resolved_style && ParentComputedStyle()) {
diff --git a/third_party/WebKit/Source/core/html/HTMLOptionElement.h b/third_party/WebKit/Source/core/html/HTMLOptionElement.h index aa6642cc..3b2c8725 100644 --- a/third_party/WebKit/Source/core/html/HTMLOptionElement.h +++ b/third_party/WebKit/Source/core/html/HTMLOptionElement.h
@@ -97,7 +97,7 @@ bool SupportsFocus() const override; bool MatchesDefaultPseudoClass() const override; bool MatchesEnabledPseudoClass() const override; - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; void ParseAttribute(const AttributeModificationParams&) override; InsertionNotificationRequest InsertedInto(ContainerNode*) override; void RemovedFrom(ContainerNode*) override;
diff --git a/third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp b/third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp index 0dd0ce0..b638cda 100644 --- a/third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp
@@ -178,7 +178,7 @@ HTMLFrameOwnerElement::DidMoveToNewDocument(old_document); } -void HTMLPlugInElement::AttachLayoutTree(const AttachContext& context) { +void HTMLPlugInElement::AttachLayoutTree(AttachContext& context) { HTMLFrameOwnerElement::AttachLayoutTree(context); if (!GetLayoutObject() || UseFallbackContent()) { @@ -203,6 +203,9 @@ GetDocument().IncrementLoadEventDelayCount(); GetDocument().LoadPluginsSoon(); } + LayoutObject* layout_object = GetLayoutObject(); + if (layout_object && !layout_object->IsFloatingOrOutOfFlowPositioned()) + context.previous_in_flow = layout_object; } void HTMLPlugInElement::UpdatePlugin() {
diff --git a/third_party/WebKit/Source/core/html/HTMLPlugInElement.h b/third_party/WebKit/Source/core/html/HTMLPlugInElement.h index 0176d2f8..4c40f48 100644 --- a/third_party/WebKit/Source/core/html/HTMLPlugInElement.h +++ b/third_party/WebKit/Source/core/html/HTMLPlugInElement.h
@@ -133,7 +133,7 @@ bool CanStartSelection() const override; bool WillRespondToMouseClickEvents() final; void DefaultEventHandler(Event*) final; - void AttachLayoutTree(const AttachContext& = AttachContext()) final; + void AttachLayoutTree(AttachContext&) final; void DetachLayoutTree(const AttachContext& = AttachContext()) final; void FinishParsingChildren() final;
diff --git a/third_party/WebKit/Source/core/html/HTMLProgressElement.cpp b/third_party/WebKit/Source/core/html/HTMLProgressElement.cpp index bf29c2b..2e2fca48 100644 --- a/third_party/WebKit/Source/core/html/HTMLProgressElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLProgressElement.cpp
@@ -81,7 +81,7 @@ } } -void HTMLProgressElement::AttachLayoutTree(const AttachContext& context) { +void HTMLProgressElement::AttachLayoutTree(AttachContext& context) { LabelableElement::AttachLayoutTree(context); if (LayoutProgressItem layout_item = LayoutProgressItem(GetLayoutProgress())) layout_item.UpdateFromElement();
diff --git a/third_party/WebKit/Source/core/html/HTMLProgressElement.h b/third_party/WebKit/Source/core/html/HTMLProgressElement.h index 2bcb57a..56fc04e 100644 --- a/third_party/WebKit/Source/core/html/HTMLProgressElement.h +++ b/third_party/WebKit/Source/core/html/HTMLProgressElement.h
@@ -62,7 +62,7 @@ void ParseAttribute(const AttributeModificationParams&) override; - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; void DidElementStateChange(); void DidAddUserAgentShadowRoot(ShadowRoot&) override;
diff --git a/third_party/WebKit/Source/core/html/HTMLSlotElement.cpp b/third_party/WebKit/Source/core/html/HTMLSlotElement.cpp index 0f3e1d22..0c0b054 100644 --- a/third_party/WebKit/Source/core/html/HTMLSlotElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLSlotElement.cpp
@@ -160,12 +160,17 @@ return NormalizeSlotName(FastGetAttribute(HTMLNames::nameAttr)); } -void HTMLSlotElement::AttachLayoutTree(const AttachContext& context) { +void HTMLSlotElement::AttachLayoutTree(AttachContext& context) { if (SupportsDistribution()) { + AttachContext children_context(context); + children_context.resolved_style = nullptr; + for (auto& node : distributed_nodes_) { if (node->NeedsAttach()) - node->AttachLayoutTree(context); + node->AttachLayoutTree(children_context); } + if (children_context.previous_in_flow) + context.previous_in_flow = children_context.previous_in_flow; } HTMLElement::AttachLayoutTree(context); }
diff --git a/third_party/WebKit/Source/core/html/HTMLSlotElement.h b/third_party/WebKit/Source/core/html/HTMLSlotElement.h index 0711e87..2120c38 100644 --- a/third_party/WebKit/Source/core/html/HTMLSlotElement.h +++ b/third_party/WebKit/Source/core/html/HTMLSlotElement.h
@@ -73,7 +73,7 @@ void LazyReattachDistributedNodesIfNeeded(); - void AttachLayoutTree(const AttachContext& = AttachContext()) final; + void AttachLayoutTree(AttachContext&) final; void DetachLayoutTree(const AttachContext& = AttachContext()) final; void RebuildDistributedChildrenLayoutTrees();
diff --git a/third_party/WebKit/Source/core/html/HTMLVideoElement.cpp b/third_party/WebKit/Source/core/html/HTMLVideoElement.cpp index 49f93b2..6bc214b 100644 --- a/third_party/WebKit/Source/core/html/HTMLVideoElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLVideoElement.cpp
@@ -132,7 +132,7 @@ return new LayoutVideo(this); } -void HTMLVideoElement::AttachLayoutTree(const AttachContext& context) { +void HTMLVideoElement::AttachLayoutTree(AttachContext& context) { HTMLMediaElement::AttachLayoutTree(context); UpdateDisplayState();
diff --git a/third_party/WebKit/Source/core/html/HTMLVideoElement.h b/third_party/WebKit/Source/core/html/HTMLVideoElement.h index 4969b434..4c4bef42 100644 --- a/third_party/WebKit/Source/core/html/HTMLVideoElement.h +++ b/third_party/WebKit/Source/core/html/HTMLVideoElement.h
@@ -161,7 +161,7 @@ bool LayoutObjectIsNeeded(const ComputedStyle&) override; LayoutObject* CreateLayoutObject(const ComputedStyle&) override; - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; void ParseAttribute(const AttributeModificationParams&) override; bool IsPresentationAttribute(const QualifiedName&) const override; void CollectStyleForPresentationAttribute(const QualifiedName&,
diff --git a/third_party/WebKit/Source/core/loader/BaseFetchContext.cpp b/third_party/WebKit/Source/core/loader/BaseFetchContext.cpp index c1f715ac..e1dc576 100644 --- a/third_party/WebKit/Source/core/loader/BaseFetchContext.cpp +++ b/third_party/WebKit/Source/core/loader/BaseFetchContext.cpp
@@ -161,6 +161,9 @@ SecurityViolationReportingPolicy reporting_policy, FetchParameters::OriginRestriction origin_restriction, ResourceRequest::RedirectStatus redirect_status) const { + if (IsDetached() && !resource_request.GetKeepalive()) + return ResourceRequestBlockedReason::kOther; + if (ShouldBlockRequestByInspector(resource_request)) return ResourceRequestBlockedReason::kInspector;
diff --git a/third_party/WebKit/Source/core/loader/BaseFetchContextTest.cpp b/third_party/WebKit/Source/core/loader/BaseFetchContextTest.cpp index feb1aaa..5d89317 100644 --- a/third_party/WebKit/Source/core/loader/BaseFetchContextTest.cpp +++ b/third_party/WebKit/Source/core/loader/BaseFetchContextTest.cpp
@@ -77,6 +77,9 @@ } const KURL& Url() const override { return execution_context_->Url(); } + SecurityOrigin* GetSecurityOrigin() const override { + return execution_context_->GetSecurityOrigin(); + } const SecurityOrigin* GetParentSecurityOrigin() const override { return nullptr; } @@ -94,8 +97,12 @@ BaseFetchContext::Trace(visitor); } + bool IsDetached() const override { return is_detached_; } + void SetIsDetached(bool is_detached) { is_detached_ = is_detached; } + private: Member<ExecutionContext> execution_context_; + bool is_detached_ = false; }; class BaseFetchContextTest : public ::testing::Test { @@ -108,7 +115,7 @@ } Persistent<ExecutionContext> execution_context_; - Persistent<BaseFetchContext> fetch_context_; + Persistent<MockBaseFetchContext> fetch_context_; }; TEST_F(BaseFetchContextTest, SetIsExternalRequestForPublicContext) { @@ -299,4 +306,37 @@ EXPECT_EQ(2u, policy->violation_reports_sent_.size()); } +TEST_F(BaseFetchContextTest, CanRequestWhenDetached) { + KURL url(KURL(), "http://www.example.com/"); + ResourceRequest request(url); + ResourceRequest keepalive_request(url); + keepalive_request.SetKeepalive(true); + + EXPECT_EQ(ResourceRequestBlockedReason::kNone, + fetch_context_->CanRequest( + Resource::kRaw, request, url, ResourceLoaderOptions(), + SecurityViolationReportingPolicy::kSuppressReporting, + FetchParameters::kNoOriginRestriction)); + + EXPECT_EQ(ResourceRequestBlockedReason::kNone, + fetch_context_->CanRequest( + Resource::kRaw, keepalive_request, url, ResourceLoaderOptions(), + SecurityViolationReportingPolicy::kSuppressReporting, + FetchParameters::kNoOriginRestriction)); + + fetch_context_->SetIsDetached(true); + + EXPECT_EQ(ResourceRequestBlockedReason::kOther, + fetch_context_->CanRequest( + Resource::kRaw, request, url, ResourceLoaderOptions(), + SecurityViolationReportingPolicy::kSuppressReporting, + FetchParameters::kNoOriginRestriction)); + + EXPECT_EQ(ResourceRequestBlockedReason::kNone, + fetch_context_->CanRequest( + Resource::kRaw, keepalive_request, url, ResourceLoaderOptions(), + SecurityViolationReportingPolicy::kSuppressReporting, + FetchParameters::kNoOriginRestriction)); +} + } // namespace blink
diff --git a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp index 0c16946b..34429a9 100644 --- a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp +++ b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
@@ -275,34 +275,56 @@ new_request.SetFetchRequestMode(options_.fetch_request_mode); } - // We assume that ServiceWorker is skipped for sync requests and unsupported - // protocol requests by content/ code. - if (async_ && - request.GetServiceWorkerMode() == - WebURLRequest::ServiceWorkerMode::kAll && - SchemeRegistry::ShouldTreatURLSchemeAsAllowingServiceWorkers( - request.Url().Protocol()) && - loading_context_->GetResourceFetcher()->IsControlledByServiceWorker()) { - if (new_request.GetFetchRequestMode() == - WebURLRequest::kFetchRequestModeCORS || - new_request.GetFetchRequestMode() == - WebURLRequest::kFetchRequestModeCORSWithForcedPreflight) { - fallback_request_for_service_worker_ = ResourceRequest(request); - // m_fallbackRequestForServiceWorker is used when a regular controlling - // service worker doesn't handle a cross origin request. When this happens - // we still want to give foreign fetch a chance to handle the request, so - // only skip the controlling service worker for the fallback request. This - // is currently safe because of http://crbug.com/604084 the - // wasFallbackRequiredByServiceWorker flag is never set when foreign fetch - // handled a request. - fallback_request_for_service_worker_.SetServiceWorkerMode( - WebURLRequest::ServiceWorkerMode::kForeign); - } - LoadRequest(new_request, resource_loader_options_); + // Process the CORS protocol inside the DocumentThreadableLoader for the + // following cases: + // + // - When the request is sync or the protocol is unsupported since we can + // assume that any SW is skipped for such requests by content/ code. + // - When the ServiceWorkerMode is not kAll, the local SW will be skipped. + // The request can still be intercepted by a foreign SW, but we cannot know + // whether such a foreign fetch interception happens or not at this point. + // - If we're not yet controlled by a local SW, then we're sure that this + // request won't be intercepted by a local SW. In case we end up with + // sending a CORS preflight request, the actual request to be sent later + // may be intercepted. This is taken care of in LoadPreflightRequest() by + // setting the ServiceWorkerMode to kNone. + // + // From the above analysis, you can see that the request can never be + // intercepted by a local SW inside this if-block. It's because: + // - the ServiceWorkerMode needs to be kAll, and + // - we're controlled by a SW at this point + // to allow a local SW to intercept the request. Even when the request gets + // issued asnychronously after performing the CORS preflight, it doesn'g get + // intercepted since LoadPreflightRequest() sets the flag to kNone in + // advance. + if (!async_ || + request.GetServiceWorkerMode() != + WebURLRequest::ServiceWorkerMode::kAll || + !SchemeRegistry::ShouldTreatURLSchemeAsAllowingServiceWorkers( + request.Url().Protocol()) || + !loading_context_->GetResourceFetcher()->IsControlledByServiceWorker()) { + DispatchInitialRequest(new_request); return; } - DispatchInitialRequest(new_request); + if (new_request.GetFetchRequestMode() == + WebURLRequest::kFetchRequestModeCORS || + new_request.GetFetchRequestMode() == + WebURLRequest::kFetchRequestModeCORSWithForcedPreflight) { + // Save the request to fallback_request_for_service_worker to use when the + // local SW doesn't handle (call respondWith()) a CORS enabled request. + fallback_request_for_service_worker_ = ResourceRequest(request); + + // We still want to give a foreign SW if any a chance to handle the + // request. So, only skip the controlling local SW for the fallback + // request. This is currently safe because of http://crbug.com/604084. The + // WasFallbackRequiredByServiceWorker() flag is never set when a foreign SW + // handled a request. + fallback_request_for_service_worker_.SetServiceWorkerMode( + WebURLRequest::ServiceWorkerMode::kForeign); + } + + LoadRequest(new_request, resource_loader_options_); } void DocumentThreadableLoader::DispatchInitialRequest( @@ -321,13 +343,39 @@ } void DocumentThreadableLoader::PrepareCrossOriginRequest( - ResourceRequest& request) { + ResourceRequest& request) const { if (GetSecurityOrigin()) request.SetHTTPOrigin(GetSecurityOrigin()); if (override_referrer_) request.SetHTTPReferrer(referrer_after_redirect_); } +void DocumentThreadableLoader::LoadPreflightRequest( + const ResourceRequest& actual_request, + const ResourceLoaderOptions& actual_options) { + ResourceRequest preflight_request = + CreateAccessControlPreflightRequest(actual_request); + + // TODO(tyoshino): Call prepareCrossOriginRequest(preflightRequest) to + // also set the referrer header. + if (GetSecurityOrigin()) + preflight_request.SetHTTPOrigin(GetSecurityOrigin()); + + actual_request_ = actual_request; + actual_options_ = actual_options; + + // Explicitly set the ServiceWorkerMode to None here. Although the page is + // not controlled by a SW at this point, a new SW may be controlling the + // page when this actual request gets sent later. We should not send the + // actual request to the SW. See https://crbug.com/604583. + actual_request_.SetServiceWorkerMode(WebURLRequest::ServiceWorkerMode::kNone); + + // Create a ResourceLoaderOptions for preflight. + ResourceLoaderOptions preflight_options = actual_options; + + LoadRequest(preflight_request, preflight_options); +} + void DocumentThreadableLoader::MakeCrossOriginAccessRequest( const ResourceRequest& request) { DCHECK(options_.fetch_request_mode == WebURLRequest::kFetchRequestModeCORS || @@ -368,74 +416,71 @@ cross_origin_request.RemoveUserAndPassFromURL(); - bool skip_preflight = false; - if (options_.preflight_policy == kPreventPreflight) { - skip_preflight = true; - } else { + // Enforce the CORS preflight for checking the Access-Control-Allow-External + // header. The CORS preflight cache doesn't help for this purpose. + if (request.IsExternalRequest()) { + LoadPreflightRequest(cross_origin_request, cross_origin_options); + return; + } + + if (options_.fetch_request_mode != + WebURLRequest::kFetchRequestModeCORSWithForcedPreflight) { + if (options_.preflight_policy == kPreventPreflight) { + PrepareCrossOriginRequest(cross_origin_request); + LoadRequest(cross_origin_request, cross_origin_options); + return; + } + DCHECK_EQ(options_.preflight_policy, kConsiderPreflight); // We use ContainsOnlyCORSSafelistedOrForbiddenHeaders() here since - // |request| may have been modified in the process of loading (not from the - // user's input). For example, referrer. We need to accept them. For + // |request| may have been modified in the process of loading (not from + // the user's input). For example, referrer. We need to accept them. For // security, we must reject forbidden headers/methods at the point we // accept user's input. Not here. if (FetchUtils::IsCORSSafelistedMethod(request.HttpMethod()) && FetchUtils::ContainsOnlyCORSSafelistedOrForbiddenHeaders( - request.HttpHeaderFields())) - skip_preflight = true; + request.HttpHeaderFields())) { + PrepareCrossOriginRequest(cross_origin_request); + LoadRequest(cross_origin_request, cross_origin_options); + return; + } } - if (!request.IsExternalRequest() && - options_.fetch_request_mode != - WebURLRequest::kFetchRequestModeCORSWithForcedPreflight && - skip_preflight) { - PrepareCrossOriginRequest(cross_origin_request); - LoadRequest(cross_origin_request, cross_origin_options); - return; + // Now, we need to check that the request passes the CORS preflight either by + // issuing a CORS preflight or based on an entry in the CORS preflight cache. + + bool should_ignore_preflight_cache = false; + if (!IsMainThread()) { + // TODO(horo): Currently we don't support the CORS preflight cache on worker + // thread when off-main-thread-fetch is enabled. See + // https://crbug.com/443374. + should_ignore_preflight_cache = true; + } else { + // Prevent use of the CORS preflight cache when instructed by the DevTools + // not to use caches. + probe::shouldForceCORSPreflight(GetDocument(), + &should_ignore_preflight_cache); } - // Explicitly set the ServiceWorkerMode to None here. Although the page is - // not controlled by a SW at this point, a new SW may be controlling the - // page when this request gets sent later. We should not send the actual - // request to the SW. https://crbug.com/604583 - // Similarly we don't want any requests that could involve a CORS preflight - // to get intercepted by a foreign fetch service worker, even if we have the - // result of the preflight cached already. https://crbug.com/674370 - cross_origin_request.SetServiceWorkerMode( - WebURLRequest::ServiceWorkerMode::kNone); - - bool should_force_preflight = request.IsExternalRequest(); - if (!should_force_preflight) - probe::shouldForceCORSPreflight(GetDocument(), &should_force_preflight); - // TODO(horo): Currently we don't support the CORS preflight cache on worker - // thread when off-main-thread-fetch is enabled. https://crbug.com/443374 - bool can_skip_preflight = - IsMainThread() && - CrossOriginPreflightResultCache::Shared().CanSkipPreflight( + if (should_ignore_preflight_cache || + !CrossOriginPreflightResultCache::Shared().CanSkipPreflight( GetSecurityOrigin()->ToString(), cross_origin_request.Url(), cross_origin_request.GetFetchCredentialsMode(), cross_origin_request.HttpMethod(), - cross_origin_request.HttpHeaderFields()); - if (can_skip_preflight && !should_force_preflight) { - PrepareCrossOriginRequest(cross_origin_request); - LoadRequest(cross_origin_request, cross_origin_options); + cross_origin_request.HttpHeaderFields())) { + LoadPreflightRequest(cross_origin_request, cross_origin_options); return; } - ResourceRequest preflight_request = - CreateAccessControlPreflightRequest(cross_origin_request); - // TODO(tyoshino): Call prepareCrossOriginRequest(preflightRequest) to - // also set the referrer header. - if (GetSecurityOrigin()) - preflight_request.SetHTTPOrigin(GetSecurityOrigin()); + // We don't want any requests that could involve a CORS preflight to get + // intercepted by a foreign SW, even if we have the result of the preflight + // cached already. See https://crbug.com/674370. + cross_origin_request.SetServiceWorkerMode( + WebURLRequest::ServiceWorkerMode::kNone); - // Create a ResourceLoaderOptions for preflight. - ResourceLoaderOptions preflight_options = cross_origin_options; - - actual_request_ = cross_origin_request; - actual_options_ = cross_origin_options; - - LoadRequest(preflight_request, preflight_options); + PrepareCrossOriginRequest(cross_origin_request); + LoadRequest(cross_origin_request, cross_origin_options); } DocumentThreadableLoader::~DocumentThreadableLoader() {
diff --git a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h index 364f6c0f..bd097239 100644 --- a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h +++ b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h
@@ -136,9 +136,12 @@ void MakeCrossOriginAccessRequest(const ResourceRequest&); // Loads m_fallbackRequestForServiceWorker. void LoadFallbackRequestForServiceWorker(); - // Loads m_actualRequest. + // Issues a CORS preflight. + void LoadPreflightRequest(const ResourceRequest&, + const ResourceLoaderOptions&); + // Loads actual_request_. void LoadActualRequest(); - // Clears m_actualRequest and reports access control check failure to + // Clears actual_request_ and reports access control check failure to // m_client. void HandlePreflightFailure(const String& url, const String& error_description); @@ -152,7 +155,8 @@ void LoadRequestAsync(const ResourceRequest&, ResourceLoaderOptions); void LoadRequestSync(const ResourceRequest&, ResourceLoaderOptions); - void PrepareCrossOriginRequest(ResourceRequest&); + void PrepareCrossOriginRequest(ResourceRequest&) const; + // This method modifies the ResourceRequest by calling // SetAllowStoredCredentials() on it based on same-origin-ness and the // credentials mode.
diff --git a/third_party/WebKit/Source/core/loader/FrameFetchContext.h b/third_party/WebKit/Source/core/loader/FrameFetchContext.h index 98f51e3..6947cba 100644 --- a/third_party/WebKit/Source/core/loader/FrameFetchContext.h +++ b/third_party/WebKit/Source/core/loader/FrameFetchContext.h
@@ -156,6 +156,8 @@ std::unique_ptr<WebURLLoader> CreateURLLoader( const ResourceRequest&) override; + bool IsDetached() const override { return frozen_state_; } + FetchContext* Detach() override; DECLARE_VIRTUAL_TRACE(); @@ -210,8 +212,6 @@ ClientHintsPreferences GetClientHintsPreferences() const; float GetDevicePixelRatio() const; - bool IsDetached() const { return frozen_state_; } - Member<DocumentLoader> document_loader_; Member<Document> document_;
diff --git a/third_party/WebKit/Source/core/svg/SVGElement.cpp b/third_party/WebKit/Source/core/svg/SVGElement.cpp index 9f1c5fe..409550cc 100644 --- a/third_party/WebKit/Source/core/svg/SVGElement.cpp +++ b/third_party/WebKit/Source/core/svg/SVGElement.cpp
@@ -81,7 +81,7 @@ element->RemoveInstanceMapping(this); } -void SVGElement::AttachLayoutTree(const AttachContext& context) { +void SVGElement::AttachLayoutTree(AttachContext& context) { Element::AttachLayoutTree(context); if (SVGElement* element = CorrespondingElement()) element->MapInstanceToElement(this);
diff --git a/third_party/WebKit/Source/core/svg/SVGElement.h b/third_party/WebKit/Source/core/svg/SVGElement.h index b4005f6..9c48d55e 100644 --- a/third_party/WebKit/Source/core/svg/SVGElement.h +++ b/third_party/WebKit/Source/core/svg/SVGElement.h
@@ -52,7 +52,7 @@ public: ~SVGElement() override; - void AttachLayoutTree(const AttachContext&) override; + void AttachLayoutTree(AttachContext&) override; void DetachLayoutTree(const AttachContext&) override; int tabIndex() const override;
diff --git a/third_party/WebKit/Source/core/svg/SVGImageElement.cpp b/third_party/WebKit/Source/core/svg/SVGImageElement.cpp index 3f15697a..ec2d888 100644 --- a/third_party/WebKit/Source/core/svg/SVGImageElement.cpp +++ b/third_party/WebKit/Source/core/svg/SVGImageElement.cpp
@@ -159,7 +159,7 @@ return !GetImageLoader().HasPendingActivity(); } -void SVGImageElement::AttachLayoutTree(const AttachContext& context) { +void SVGImageElement::AttachLayoutTree(AttachContext& context) { SVGGraphicsElement::AttachLayoutTree(context); if (LayoutSVGImage* image_obj = ToLayoutSVGImage(GetLayoutObject())) {
diff --git a/third_party/WebKit/Source/core/svg/SVGImageElement.h b/third_party/WebKit/Source/core/svg/SVGImageElement.h index 18a375c..2058073 100644 --- a/third_party/WebKit/Source/core/svg/SVGImageElement.h +++ b/third_party/WebKit/Source/core/svg/SVGImageElement.h
@@ -76,7 +76,7 @@ void SvgAttributeChanged(const QualifiedName&) override; - void AttachLayoutTree(const AttachContext& = AttachContext()) override; + void AttachLayoutTree(AttachContext&) override; InsertionNotificationRequest InsertedInto(ContainerNode*) override; LayoutObject* CreateLayoutObject(const ComputedStyle&) override;
diff --git a/third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.cpp b/third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.cpp index 2b03b8f7..0639420 100644 --- a/third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.cpp +++ b/third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.cpp
@@ -13,6 +13,7 @@ #include "core/workers/WorkerThread.h" #include "platform/CrossThreadFunctional.h" #include "platform/RuntimeEnabledFeatures.h" +#include "platform/loader/fetch/ResourceFetcher.h" #include "platform/wtf/Functional.h" namespace blink { @@ -81,6 +82,12 @@ DCHECK(script_controller_); script_controller_->Dispose(); script_controller_.Clear(); + + if (fetch_context_) { + ResourceFetcher* fetcher = fetch_context_->GetResourceFetcher(); + fetcher->StopFetching(); + fetcher->ClearContext(); + } } DEFINE_TRACE(WorkerOrWorkletGlobalScope) {
diff --git a/third_party/WebKit/Source/platform/loader/fetch/FetchContext.h b/third_party/WebKit/Source/platform/loader/fetch/FetchContext.h index 92504a0..fd9aca2f 100644 --- a/third_party/WebKit/Source/platform/loader/fetch/FetchContext.h +++ b/third_party/WebKit/Source/platform/loader/fetch/FetchContext.h
@@ -213,6 +213,8 @@ return nullptr; } + virtual bool IsDetached() const { return false; } + // Called when the underlying context is detached. Note that some // FetchContexts continue working after detached (e.g., for fetch() operations // with "keepalive" specified).
diff --git a/third_party/WebKit/public/platform/Platform.h b/third_party/WebKit/public/platform/Platform.h index 2fdcc52a..96abe0e 100644 --- a/third_party/WebKit/public/platform/Platform.h +++ b/third_party/WebKit/public/platform/Platform.h
@@ -42,6 +42,7 @@ #include "WebAudioDevice.h" #include "WebCommon.h" #include "WebData.h" +#include "WebDataConsumerHandle.h" #include "WebFeaturePolicy.h" #include "WebGamepadListener.h" #include "WebGestureDevice.h" @@ -59,6 +60,7 @@ #include "base/metrics/user_metrics_action.h" #include "cc/resources/shared_bitmap.h" #include "cc/surfaces/frame_sink_id.h" +#include "mojo/public/cpp/system/data_pipe.h" #include "mojo/public/cpp/system/message_pipe.h" namespace device { @@ -334,6 +336,12 @@ // Returns a new WebURLLoader instance. virtual std::unique_ptr<WebURLLoader> CreateURLLoader() { return nullptr; } + // Returns a WebDataConsumerHandle for given a mojo data pipe endpoint. + virtual std::unique_ptr<WebDataConsumerHandle> CreateDataConsumerHandle( + mojo::ScopedDataPipeConsumerHandle handle) { + return nullptr; + } + // May return null. virtual WebPrescientNetworking* PrescientNetworking() { return nullptr; }
diff --git a/tools/android/customtabs_benchmark/BUILD.gn b/tools/android/customtabs_benchmark/BUILD.gn index 01c5ccf..db5962a 100644 --- a/tools/android/customtabs_benchmark/BUILD.gn +++ b/tools/android/customtabs_benchmark/BUILD.gn
@@ -18,6 +18,7 @@ apk_name = "CustomTabsBenchmark" deps = [ ":customtabs_benchmark_apk_resources", + "//third_party/android_tools:android_support_v4_java", "//third_party/custom_tabs_client:custom_tabs_support_java", ] }
diff --git a/tools/android/customtabs_benchmark/java/src/org/chromium/customtabs/test/MainActivity.java b/tools/android/customtabs_benchmark/java/src/org/chromium/customtabs/test/MainActivity.java index a816e72..9eb65171 100644 --- a/tools/android/customtabs_benchmark/java/src/org/chromium/customtabs/test/MainActivity.java +++ b/tools/android/customtabs_benchmark/java/src/org/chromium/customtabs/test/MainActivity.java
@@ -10,6 +10,7 @@ import android.net.Uri; import android.os.Bundle; import android.os.Handler; +import android.os.IBinder; import android.os.Looper; import android.os.SystemClock; import android.support.customtabs.CustomTabsCallback; @@ -17,6 +18,7 @@ import android.support.customtabs.CustomTabsIntent; import android.support.customtabs.CustomTabsServiceConnection; import android.support.customtabs.CustomTabsSession; +import android.support.v4.app.BundleCompat; import android.util.Log; import android.view.View; import android.widget.Button; @@ -46,14 +48,14 @@ private static final String USE_WEBVIEW_KEY = "use_webview"; private static final String WARMUP_KEY = "warmup"; - // Keep in sync with the same constants in CustomTabsConnection. - private static final String DEBUG_OVERRIDE_KEY = - "android.support.customtabs.maylaunchurl.DEBUG_OVERRIDE"; - private static final int NO_OVERRIDE = 0; - private static final int NO_PRERENDERING = 1; - private static final int PREFETCH_ONLY = 2; - // Only for reporting. - private static final int NO_STATE_PREFETCH = 3; + // extraCommand related constants. + private static final String SET_PRERENDER_ON_CELLULAR = "setPrerenderOnCellularForSession"; + private static final String SET_SPECULATION_MODE = "setSpeculationModeForSession"; + private static final String SET_IGNORE_URL_FRAGMENTS_FOR_SESSION = + "setIgnoreUrlFragmentsForSession"; + private static final int NO_SPECULATION = 0; + private static final int PRERENDER = 2; + private static final int HIDDEN_TAB = 3; private final Handler mHandler = new Handler(Looper.getMainLooper()); @@ -217,41 +219,24 @@ private void startCustomTabsBenchmark(Intent intent) { String url = intent.getStringExtra(URL_KEY); if (url == null) url = DEFAULT_URL; + String speculatedUrl = intent.getStringExtra("speculated_url"); + if (speculatedUrl == null) speculatedUrl = url; String packageName = intent.getStringExtra("package_name"); if (packageName == null) packageName = DEFAULT_PACKAGE; boolean warmup = intent.getBooleanExtra("warmup", false); int delayToMayLaunchUrl = intent.getIntExtra("delay_to_may_launch_url", NONE); int delayToLaunchUrl = intent.getIntExtra("delay_to_launch_url", NONE); - - int speculationMode = 0; - String speculationModeValue = intent.getStringExtra("speculation_mode"); - switch (speculationModeValue) { - case "prerender": - speculationMode = NO_OVERRIDE; - break; - case "disabled": - speculationMode = NO_PRERENDERING; - break; - case "speculative_prefetch": - speculationMode = PREFETCH_ONLY; - break; - case "no_state_prefetch": - speculationMode = NO_STATE_PREFETCH; - break; - default: - throw new IllegalArgumentException( - "Invalid prerender mode: " + speculationModeValue); - } - + String speculationMode = intent.getStringExtra("speculation_mode"); + if (speculationMode == null) speculationMode = "prerender"; int timeoutSeconds = intent.getIntExtra("timeout", NONE); - launchCustomTabs(packageName, url, warmup, speculationMode, delayToMayLaunchUrl, - delayToLaunchUrl, timeoutSeconds); + launchCustomTabs(packageName, speculatedUrl, url, warmup, speculationMode, + delayToMayLaunchUrl, delayToLaunchUrl, timeoutSeconds); } private final class CustomCallback extends CustomTabsCallback { private final boolean mWarmup; - private final int mSpeculationMode; + private final String mSpeculationMode; private final int mDelayToMayLaunchUrl; private final int mDelayToLaunchUrl; private long mIntentSentMs = NONE; @@ -259,7 +244,7 @@ private long mPageLoadFinishedMs = NONE; private long mFirstContentfulPaintMs = NONE; - public CustomCallback(boolean warmup, int speculationMode, int delayToMayLaunchUrl, + public CustomCallback(boolean warmup, String speculationMode, int delayToMayLaunchUrl, int delayToLaunchUrl) { mWarmup = warmup; mSpeculationMode = speculationMode; @@ -325,11 +310,50 @@ } } - private void onCustomTabsServiceConnected(CustomTabsClient client, final Uri uri, - final CustomCallback cb, boolean warmup, final int prerenderMode, + private static void forceSpeculationMode( + CustomTabsClient client, IBinder sessionBinder, String speculationMode) { + // The same bundle can be used for all calls, as the commands only look for their own + // arguments in it. + Bundle params = new Bundle(); + BundleCompat.putBinder(params, "session", sessionBinder); + params.putBoolean("ignoreFragments", true); + params.putBoolean("prerender", true); + + int speculationModeValue = 0; + switch (speculationMode) { + case "disabled": + speculationModeValue = NO_SPECULATION; + break; + case "prerender": + speculationModeValue = PRERENDER; + break; + case "hidden_tab": + speculationModeValue = HIDDEN_TAB; + break; + default: + throw new RuntimeException("Invalid speculation mode"); + } + params.putInt("speculationMode", speculationModeValue); + + boolean ok = client.extraCommand(SET_PRERENDER_ON_CELLULAR, params) != null; + if (!ok) throw new RuntimeException("Cannot set cellular prerendering"); + ok = client.extraCommand(SET_IGNORE_URL_FRAGMENTS_FOR_SESSION, params) != null; + if (!ok) throw new RuntimeException("Cannot set ignoreFragments"); + ok = client.extraCommand(SET_SPECULATION_MODE, params) != null; + if (!ok) throw new RuntimeException("Cannot set the speculation mode"); + } + + private void onCustomTabsServiceConnected(CustomTabsClient client, final Uri speculatedUri, + final Uri uri, final CustomCallback cb, boolean warmup, String speculationMode, int delayToMayLaunchUrl, final int delayToLaunchUrl, final int timeoutSeconds) { final CustomTabsSession session = client.newSession(cb); final CustomTabsIntent intent = (new CustomTabsIntent.Builder(session)).build(); + + IBinder sessionBinder = + BundleCompat.getBinder(intent.intent.getExtras(), CustomTabsIntent.EXTRA_SESSION); + assert sessionBinder != null; + forceSpeculationMode(client, sessionBinder, speculationMode); + final Runnable launchRunnable = new Runnable() { @Override public void run() { @@ -341,14 +365,7 @@ Runnable mayLaunchRunnable = new Runnable() { @Override public void run() { - Bundle extras = new Bundle(); - if (prerenderMode == NO_PRERENDERING) { - extras.putInt(DEBUG_OVERRIDE_KEY, NO_PRERENDERING); - } else if (prerenderMode != NO_STATE_PREFETCH) { - extras.putInt(DEBUG_OVERRIDE_KEY, prerenderMode); - } - - session.mayLaunchUrl(uri, extras, null); + session.mayLaunchUrl(speculatedUri, null, null); mHandler.postDelayed(launchRunnable, delayToLaunchUrl); } }; @@ -361,19 +378,20 @@ } } - private void launchCustomTabs(String packageName, String url, final boolean warmup, - final int speculationMode, final int delayToMayLaunchUrl, final int delayToLaunchUrl, - final int timeoutSeconds) { + private void launchCustomTabs(String packageName, String speculatedUrl, String url, + final boolean warmup, final String speculationMode, final int delayToMayLaunchUrl, + final int delayToLaunchUrl, final int timeoutSeconds) { final CustomCallback cb = new CustomCallback(warmup, speculationMode, delayToMayLaunchUrl, delayToLaunchUrl); + final Uri speculatedUri = Uri.parse(speculatedUrl); final Uri uri = Uri.parse(url); CustomTabsClient.bindCustomTabsService( this, packageName, new CustomTabsServiceConnection() { @Override public void onCustomTabsServiceConnected( ComponentName name, final CustomTabsClient client) { - MainActivity.this.onCustomTabsServiceConnected(client, uri, cb, warmup, - speculationMode, delayToMayLaunchUrl, delayToLaunchUrl, + MainActivity.this.onCustomTabsServiceConnected(client, speculatedUri, uri, + cb, warmup, speculationMode, delayToMayLaunchUrl, delayToLaunchUrl, timeoutSeconds); }
diff --git a/tools/android/customtabs_benchmark/scripts/customtabs_benchmark.py b/tools/android/customtabs_benchmark/scripts/customtabs_benchmark.py index 3676ceeb..93d7d0d 100755 --- a/tools/android/customtabs_benchmark/scripts/customtabs_benchmark.py +++ b/tools/android/customtabs_benchmark/scripts/customtabs_benchmark.py
@@ -198,7 +198,7 @@ import numpy as np data = np.genfromtxt(filename, delimiter=',', skip_header=1) result = np.array(np.zeros(len(data)), - dtype=[('warmup', bool), ('speculation_mode', np.int32), + dtype=[('warmup', bool), ('speculation_mode', str), ('delay_to_may_launch_url', np.int32), ('delay_to_launch_url', np.int32), ('commit', np.int32), ('plt', np.int32), @@ -223,10 +223,9 @@ parser.add_option('--warmup', help='Call warmup.', default=False, action='store_true') parser.add_option('--speculation_mode', default='prerender', - help='The speculation mode (prerender, disabled, ' + help='The speculation mode (prerender, ' 'speculative_prefetch or no_state_prefetch).', - choices=['prerender', 'disabled', 'speculative_prefetch', - 'no_state_prefetch']) + choices=['disabled', 'prerender', 'hidden_tab']) parser.add_option('--delay_to_may_launch_url', help='Delay before calling mayLaunchUrl() in ms.', type='int', default=1000)