diff --git a/DEPS b/DEPS index e83fa9c..a8a5c92 100644 --- a/DEPS +++ b/DEPS
@@ -36,7 +36,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling Skia # and whatever else without interference from each other. - 'skia_revision': 'dad1aef9a8bcdcb2ef68d674035ba076467beef2', + 'skia_revision': '03591a762c0e3f541141d570625c2dab16deacba', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other.
diff --git a/chrome/browser/android/webapk/webapk.proto b/chrome/browser/android/webapk/webapk.proto index 773a3fde..094202d5 100644 --- a/chrome/browser/android/webapk/webapk.proto +++ b/chrome/browser/android/webapk/webapk.proto
@@ -8,35 +8,32 @@ package webapk; -// Creates a WebAPK on the server and returns URL to download WebAPK from Google -// Play. -message CreateWebApkRequest { - optional WebApk webapk = 1; -} - -// Response to CreateWebApkRequest. -message CreateWebApkResponse { +// Response to "create" request. +message WebApkResponse { // Package name to install WebAPK at. - optional string webapk_package_name = 1; + optional string package_name = 1; // URL to download WebAPK. - optional string signed_download_url = 2; + optional string signed_download_url = 3; + + reserved 2, 4; } +// Sent as part of request to create a WebAPK. message WebApk { // The URL of the Web App Manifest. - optional string manifest_url = 2; + optional string manifest_url = 3; // Chrome's package name. - optional string requester_application_package = 4; + optional string requester_application_package = 5; // Chrome's version. - optional string requester_application_version = 5; + optional string requester_application_version = 6; // The Web App Manifest. - optional WebAppManifest manifest = 6; + optional WebAppManifest manifest = 7; - reserved 1, 3; + reserved 1, 2, 4; } // Contains data from the Web App Manifest. @@ -60,7 +57,7 @@ // Murmur2 hash of the icon's bytes. There should not be any transformations // applied to the icon's bytes prior to taking the Murmur2 hash. - optional uint64 hash = 5; + optional string hash = 5; // Actual bytes of the image. This image may be re-encoded from the original // image and may not match the murmur2 hash field above.
diff --git a/chrome/browser/android/webapk/webapk_installer.cc b/chrome/browser/android/webapk/webapk_installer.cc index d07f5a5..79a12d0 100644 --- a/chrome/browser/android/webapk/webapk_installer.cc +++ b/chrome/browser/android/webapk/webapk_installer.cc
@@ -12,6 +12,7 @@ #include "base/command_line.h" #include "base/files/file_path.h" #include "base/files/file_util.h" +#include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" @@ -40,12 +41,13 @@ // The seed to use the murmur2 hash of the app icon. const uint32_t kMurmur2HashSeed = 0; -// The number of milliseconds to wait for the WebAPK download URL from the -// WebAPK server. -const int kWebApkDownloadUrlTimeoutMs = 4000; +// The default number of milliseconds to wait for the WebAPK download URL from +// the WebAPK server. +const int kWebApkDownloadUrlTimeoutMs = 60000; -// The number of milliseconds to wait for the WebAPK download to complete. -const int kDownloadTimeoutMs = 20000; +// The default number of milliseconds to wait for the WebAPK download to +// complete. +const int kDownloadTimeoutMs = 60000; // Returns the scope from |info| if it is specified. Otherwise, returns the // default scope. @@ -56,10 +58,12 @@ } // Computes a murmur2 hash of |bitmap|'s PNG encoded bytes. -uint64_t ComputeBitmapHash(const SkBitmap& bitmap) { +std::string ComputeBitmapHash(const SkBitmap& bitmap) { std::vector<unsigned char> png_bytes; gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, false, &png_bytes); - return MurmurHash64B(&png_bytes.front(), png_bytes.size(), kMurmur2HashSeed); + uint64_t hash = + MurmurHash64B(&png_bytes.front(), png_bytes.size(), kMurmur2HashSeed); + return base::Uint64ToString(hash); } // Converts a color from the format specified in content::Manifest to a CSS @@ -82,6 +86,8 @@ const SkBitmap& shortcut_icon) : shortcut_info_(shortcut_info), shortcut_icon_(shortcut_icon), + webapk_download_url_timeout_ms_(kWebApkDownloadUrlTimeoutMs), + download_timeout_ms_(kDownloadTimeoutMs), io_weak_ptr_factory_(this) { base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); server_url_ = @@ -115,6 +121,11 @@ SendCreateWebApkRequest(); } +void WebApkInstaller::SetTimeoutMs(int timeout_ms) { + webapk_download_url_timeout_ms_ = timeout_ms; + download_timeout_ms_ = timeout_ms; +} + bool WebApkInstaller::StartDownloadedWebApkInstall( JNIEnv* env, const base::android::ScopedJavaLocalRef<jstring>& java_file_path, @@ -136,20 +147,19 @@ std::string response_string; source->GetResponseAsString(&response_string); - std::unique_ptr<webapk::CreateWebApkResponse> response( - new webapk::CreateWebApkResponse); + std::unique_ptr<webapk::WebApkResponse> response( + new webapk::WebApkResponse); if (!response->ParseFromString(response_string)) { OnFailure(); return; } - if (response->signed_download_url().empty() || - response->webapk_package_name().empty()) { + GURL signed_download_url(response->signed_download_url()); + if (!signed_download_url.is_valid() || response->package_name().empty()) { OnFailure(); return; } - OnGotWebApkDownloadUrl(response->signed_download_url(), - response->webapk_package_name()); + OnGotWebApkDownloadUrl(signed_download_url, response->package_name()); } void WebApkInstaller::InitializeRequestContextGetterOnUIThread( @@ -167,11 +177,10 @@ void WebApkInstaller::SendCreateWebApkRequest() { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); - std::unique_ptr<webapk::CreateWebApkRequest> request = - BuildCreateWebApkRequest(); + std::unique_ptr<webapk::WebApk> webapk_proto = BuildWebApkProto(); - timer_.Start(FROM_HERE, - base::TimeDelta::FromMilliseconds(kWebApkDownloadUrlTimeoutMs), + timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds( + webapk_download_url_timeout_ms_), base::Bind(&WebApkInstaller::OnTimeout, io_weak_ptr_factory_.GetWeakPtr())); @@ -179,12 +188,12 @@ net::URLFetcher::Create(server_url_, net::URLFetcher::POST, this); url_fetcher_->SetRequestContext(request_context_getter_); std::string serialized_request; - request->SerializeToString(&serialized_request); + webapk_proto->SerializeToString(&serialized_request); url_fetcher_->SetUploadData(kProtoMimeType, serialized_request); url_fetcher_->Start(); } -void WebApkInstaller::OnGotWebApkDownloadUrl(const std::string& download_url, +void WebApkInstaller::OnGotWebApkDownloadUrl(const GURL& download_url, const std::string& package_name) { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); @@ -194,13 +203,14 @@ // directory. // TODO(pkotwicz): Figure out when downloaded WebAPK should be deleted. - timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(kDownloadTimeoutMs), + timer_.Start(FROM_HERE, + base::TimeDelta::FromMilliseconds(download_timeout_ms_), base::Bind(&WebApkInstaller::OnTimeout, io_weak_ptr_factory_.GetWeakPtr())); base::FilePath output_path = output_dir.AppendASCII(package_name); downloader_.reset(new FileDownloader( - GURL(download_url), output_path, true, request_context_getter_, + download_url, output_path, true, request_context_getter_, base::Bind(&WebApkInstaller::OnWebApkDownloaded, io_weak_ptr_factory_.GetWeakPtr(), output_path, package_name))); @@ -231,12 +241,8 @@ OnFailure(); } -std::unique_ptr<webapk::CreateWebApkRequest> -WebApkInstaller::BuildCreateWebApkRequest() { - std::unique_ptr<webapk::CreateWebApkRequest> request( - new webapk::CreateWebApkRequest); - - webapk::WebApk* webapk = request->mutable_webapk(); +std::unique_ptr<webapk::WebApk> WebApkInstaller::BuildWebApkProto() { + std::unique_ptr<webapk::WebApk> webapk(new webapk::WebApk); webapk->set_manifest_url(shortcut_info_.manifest_url.spec()); webapk->set_requester_application_package( base::android::BuildInfo::GetInstance()->package_name()); @@ -263,7 +269,7 @@ gfx::PNGCodec::EncodeBGRASkBitmap(shortcut_icon_, false, &png_bytes); image->set_image_data(&png_bytes.front(), png_bytes.size()); - return request; + return webapk; } void WebApkInstaller::OnTimeout() {
diff --git a/chrome/browser/android/webapk/webapk_installer.h b/chrome/browser/android/webapk/webapk_installer.h index c1ba56a..0aac26e 100644 --- a/chrome/browser/android/webapk/webapk_installer.h +++ b/chrome/browser/android/webapk/webapk_installer.h
@@ -32,7 +32,7 @@ } namespace webapk { -class CreateWebApkRequest; +class WebApk; } // Talks to Chrome WebAPK server and Google Play to generate a WebAPK on the @@ -56,6 +56,9 @@ net::URLRequestContextGetter* request_context_getter, const FinishCallback& callback); + // Sets the timeout for the server requests. + void SetTimeoutMs(int timeout_ms); + protected: // Starts installation of the downloaded WebAPK. Returns whether the install // could be started. The installation may still fail if true is returned. @@ -80,7 +83,7 @@ // Called with the URL of generated WebAPK and the package name that the // WebAPK should be installed at. - void OnGotWebApkDownloadUrl(const std::string& download_url, + void OnGotWebApkDownloadUrl(const GURL& download_url, const std::string& package_name); // Called once the WebAPK has been downloaded. Installs the WebAPK if the @@ -91,8 +94,8 @@ const std::string& package_name, FileDownloader::Result result); - // Populates webapk::CreateWebApkRequest and returns it. - std::unique_ptr<webapk::CreateWebApkRequest> BuildCreateWebApkRequest(); + // Populates webapk::WebApk and returns it. + std::unique_ptr<webapk::WebApk> BuildWebApkProto(); // Called when the request to the WebAPK server times out or when the WebAPK // download times out. @@ -131,6 +134,13 @@ // WebAPK server URL. GURL server_url_; + // The number of milliseconds to wait for the WebAPK download URL from the + // WebAPK server. + int webapk_download_url_timeout_ms_; + + // The number of milliseconds to wait for the WebAPK download to complete. + int download_timeout_ms_; + // Used to get |weak_ptr_| on the IO thread. base::WeakPtrFactory<WebApkInstaller> io_weak_ptr_factory_;
diff --git a/chrome/browser/android/webapk/webapk_installer_unittest.cc b/chrome/browser/android/webapk/webapk_installer_unittest.cc index 97258cba..cfc004f 100644 --- a/chrome/browser/android/webapk/webapk_installer_unittest.cc +++ b/chrome/browser/android/webapk/webapk_installer_unittest.cc
@@ -78,6 +78,7 @@ // WebApkInstaller owns itself. WebApkInstaller* installer = new TestWebApkInstaller(info, SkBitmap()); + installer->SetTimeoutMs(20); installer->InstallAsyncWithURLRequestContextGetter( url_request_context_getter_.get(), base::Bind(&WebApkInstallerRunner::OnCompleted, @@ -108,13 +109,13 @@ DISALLOW_COPY_AND_ASSIGN(WebApkInstallerRunner); }; -// Builds a webapk::CreateWebApkResponse with |download_url| as the WebAPK -// download URL. -std::unique_ptr<net::test_server::HttpResponse> BuildValidCreateWebApkResponse( +// Builds a webapk::WebApkResponse with |download_url| as the WebAPK download +// URL. +std::unique_ptr<net::test_server::HttpResponse> BuildValidWebApkResponse( const GURL& download_url) { - std::unique_ptr<webapk::CreateWebApkResponse> response_proto( - new webapk::CreateWebApkResponse); - response_proto->set_webapk_package_name(kDownloadedWebApkPackageName); + std::unique_ptr<webapk::WebApkResponse> response_proto( + new webapk::WebApkResponse); + response_proto->set_package_name(kDownloadedWebApkPackageName); response_proto->set_signed_download_url(download_url.spec()); std::string response_content; response_proto->SerializeToString(&response_content); @@ -131,7 +132,7 @@ class WebApkInstallerTest : public ::testing::Test { public: typedef base::Callback<std::unique_ptr<net::test_server::HttpResponse>(void)> - CreateWebApkResponseBuilder; + WebApkResponseBuilder; WebApkInstallerTest() : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP) {} @@ -155,10 +156,9 @@ } // Sets the function that should be used to build the response to the - // webapk::CreateWebApkRequest. - void SetCreateWebApkResponseBuilder( - const CreateWebApkResponseBuilder& builder) { - create_webapk_response_builder_ = builder; + // WebAPK creation request. + void SetWebApkResponseBuilder(const WebApkResponseBuilder& builder) { + webapk_response_builder_ = builder; } net::test_server::EmbeddedTestServer* test_server() { return &test_server_; } @@ -169,22 +169,22 @@ GURL server_url = test_server_.GetURL(kServerUrl); SetWebApkServerUrl(server_url); GURL download_url = test_server_.GetURL(kDownloadUrl); - SetCreateWebApkResponseBuilder( - base::Bind(&BuildValidCreateWebApkResponse, download_url)); + SetWebApkResponseBuilder( + base::Bind(&BuildValidWebApkResponse, download_url)); } std::unique_ptr<net::test_server::HttpResponse> HandleCreateWebApkRequest( const net::test_server::HttpRequest& request) { return (request.relative_url == kServerUrl) - ? create_webapk_response_builder_.Run() + ? webapk_response_builder_.Run() : std::unique_ptr<net::test_server::HttpResponse>(); } content::TestBrowserThreadBundle thread_bundle_; net::EmbeddedTestServer test_server_; - // Builds response to the webapk::CreateWebApkRequest. - CreateWebApkResponseBuilder create_webapk_response_builder_; + // Builds response to the WebAPK creation request. + WebApkResponseBuilder webapk_response_builder_; DISALLOW_COPY_AND_ASSIGN(WebApkInstallerTest); }; @@ -196,7 +196,7 @@ EXPECT_TRUE(runner.success()); } -// Test that installation fails if the CreateWebApkRequest times out. +// Test that installation fails if the WebAPK creation request times out. TEST_F(WebApkInstallerTest, CreateWebApkRequestTimesOut) { GURL server_url = test_server()->GetURL("/slow?1000"); SetWebApkServerUrl(server_url); @@ -209,8 +209,7 @@ // Test that installation fails if the WebAPK download times out. TEST_F(WebApkInstallerTest, WebApkDownloadTimesOut) { GURL download_url = test_server()->GetURL("/slow?1000"); - SetCreateWebApkResponseBuilder( - base::Bind(&BuildValidCreateWebApkResponse, download_url)); + SetWebApkResponseBuilder(base::Bind(&BuildValidWebApkResponse, download_url)); WebApkInstallerRunner runner; runner.Run(); @@ -220,8 +219,7 @@ // Test that installation fails if the WebAPK download fails. TEST_F(WebApkInstallerTest, WebApkDownloadFails) { GURL download_url = test_server()->GetURL("/nocontent"); - SetCreateWebApkResponseBuilder( - base::Bind(&BuildValidCreateWebApkResponse, download_url)); + SetWebApkResponseBuilder(base::Bind(&BuildValidWebApkResponse, download_url)); WebApkInstallerRunner runner; runner.Run(); @@ -230,10 +228,9 @@ namespace { -// Returns an HttpResponse which cannot be parsed as a -// webapk::CreateWebApkResponse. +// Returns an HttpResponse which cannot be parsed as a webapk::WebApkResponse. std::unique_ptr<net::test_server::HttpResponse> -BuildUnparsableCreateWebApkResponse() { +BuildUnparsableWebApkResponse() { std::unique_ptr<net::test_server::BasicHttpResponse> response( new net::test_server::BasicHttpResponse()); response->set_code(net::HTTP_OK); @@ -243,11 +240,10 @@ } // anonymous namespace -// Test that an HTTP response which cannot be parsed as -// a webapk::CreateWebApkResponse is handled properly. +// Test that an HTTP response which cannot be parsed as a webapk::WebApkResponse +// is handled properly. TEST_F(WebApkInstallerTest, UnparsableCreateWebApkResponse) { - SetCreateWebApkResponseBuilder( - base::Bind(&BuildUnparsableCreateWebApkResponse)); + SetWebApkResponseBuilder(base::Bind(&BuildUnparsableWebApkResponse)); WebApkInstallerRunner runner; runner.Run();
diff --git a/components/offline_pages/offline_page_item.h b/components/offline_pages/offline_page_item.h index 3521b69f..1757a46 100644 --- a/components/offline_pages/offline_page_item.h +++ b/components/offline_pages/offline_page_item.h
@@ -84,6 +84,8 @@ base::Time expiration_time; // Number of times that the offline archive has been accessed. int access_count; + // The title of the page at the time it was saved. + base::string16 title; // Flags about the state and behavior of the offline page. Flags flags; };
diff --git a/components/offline_pages/offline_page_metadata_store_impl_unittest.cc b/components/offline_pages/offline_page_metadata_store_impl_unittest.cc index f698d01..bfb8bc5 100644 --- a/components/offline_pages/offline_page_metadata_store_impl_unittest.cc +++ b/components/offline_pages/offline_page_metadata_store_impl_unittest.cc
@@ -39,7 +39,7 @@ // Build a store with outdated schema to simulate the upgrading process. // TODO(romax): move it to sql_unittests. -void BuildTestStoreWithOutdatedSchema(const base::FilePath& file) { +void BuildTestStoreWithSchemaFromM52(const base::FilePath& file) { sql::Connection connection; ASSERT_TRUE( connection.Open(file.Append(FILE_PATH_LITERAL("OfflinePages.db")))); @@ -83,24 +83,68 @@ connection.DoesColumnExist(OFFLINE_PAGES_TABLE_V1, "expiration_time")); } +void BuildTestStoreWithSchemaFromM53(const base::FilePath& file) { + sql::Connection connection; + ASSERT_TRUE( + connection.Open(file.Append(FILE_PATH_LITERAL("OfflinePages.db")))); + ASSERT_TRUE(connection.is_open()); + ASSERT_TRUE(connection.BeginTransaction()); + ASSERT_TRUE(connection.Execute("CREATE TABLE " OFFLINE_PAGES_TABLE_V1 + "(offline_id INTEGER PRIMARY KEY NOT NULL, " + "creation_time INTEGER NOT NULL, " + "file_size INTEGER NOT NULL, " + "version INTEGER NOT NULL, " + "last_access_time INTEGER NOT NULL, " + "access_count INTEGER NOT NULL, " + "status INTEGER NOT NULL DEFAULT 0, " + "user_initiated INTEGER, " + "expiration_time INTEGER NOT NULL DEFAULT 0, " + "client_namespace VARCHAR NOT NULL, " + "client_id VARCHAR NOT NULL, " + "online_url VARCHAR NOT NULL, " + "offline_url VARCHAR NOT NULL DEFAULT '', " + "file_path VARCHAR NOT NULL " + ")")); + ASSERT_TRUE(connection.CommitTransaction()); + sql::Statement statement(connection.GetUniqueStatement( + "INSERT INTO " OFFLINE_PAGES_TABLE_V1 + "(offline_id, creation_time, file_size, version, " + "last_access_time, access_count, client_namespace, " + "client_id, online_url, file_path, expiration_time) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)")); + statement.BindInt64(0, kOfflineId); + statement.BindInt(1, 0); + statement.BindInt64(2, kFileSize); + statement.BindInt(3, 0); + statement.BindInt(4, 0); + statement.BindInt(5, 1); + statement.BindCString(6, kTestClientNamespace); + statement.BindString(7, kTestClientId2.id); + statement.BindCString(8, kTestURL); + statement.BindString(9, base::FilePath(kFilePath).MaybeAsASCII()); + statement.BindInt64(10, base::Time::Now().ToInternalValue()); + ASSERT_TRUE(statement.Run()); + ASSERT_TRUE(connection.DoesTableExist(OFFLINE_PAGES_TABLE_V1)); + ASSERT_FALSE(connection.DoesColumnExist(OFFLINE_PAGES_TABLE_V1, "title")); +} + class OfflinePageMetadataStoreFactory { public: - virtual OfflinePageMetadataStore* BuildStore(const base::FilePath& file) = 0; - virtual OfflinePageMetadataStore* BuildStoreV1( - const base::FilePath& file) = 0; -}; - -class OfflinePageMetadataStoreSQLFactory - : public OfflinePageMetadataStoreFactory { - public: - OfflinePageMetadataStore* BuildStore(const base::FilePath& file) override { + OfflinePageMetadataStore* BuildStore(const base::FilePath& file) { OfflinePageMetadataStoreSQL* store = new OfflinePageMetadataStoreSQL( base::ThreadTaskRunnerHandle::Get(), file); return store; } - OfflinePageMetadataStore* BuildStoreV1(const base::FilePath& file) override { - BuildTestStoreWithOutdatedSchema(file); + OfflinePageMetadataStore* BuildStoreM52(const base::FilePath& file) { + BuildTestStoreWithSchemaFromM52(file); + OfflinePageMetadataStoreSQL* store = new OfflinePageMetadataStoreSQL( + base::ThreadTaskRunnerHandle::Get(), file); + return store; + } + + OfflinePageMetadataStore* BuildStoreM53(const base::FilePath& file) { + BuildTestStoreWithSchemaFromM53(file); OfflinePageMetadataStoreSQL* store = new OfflinePageMetadataStoreSQL( base::ThreadTaskRunnerHandle::Get(), file); return store; @@ -110,10 +154,10 @@ enum CalledCallback { NONE, LOAD, ADD, REMOVE, RESET }; enum Status { STATUS_NONE, STATUS_TRUE, STATUS_FALSE }; -class OfflinePageMetadataStoreTestBase : public testing::Test { +class OfflinePageMetadataStoreTest : public testing::Test { public: - OfflinePageMetadataStoreTestBase(); - ~OfflinePageMetadataStoreTestBase() override; + OfflinePageMetadataStoreTest(); + ~OfflinePageMetadataStoreTest() override; void TearDown() override { // Wait for all the pieces of the store to delete itself properly. @@ -121,6 +165,9 @@ } std::unique_ptr<OfflinePageMetadataStore> BuildStore(); + std::unique_ptr<OfflinePageMetadataStore> BuildStoreWithSchemaFromM52(); + std::unique_ptr<OfflinePageMetadataStore> BuildStoreWithSchemaFromM53(); + void PumpLoop(); void LoadCallback(OfflinePageMetadataStore::LoadStatus load_status, @@ -129,17 +176,22 @@ void ClearResults(); + OfflinePageItem CheckThatStoreHasOneItem(); + void CheckThatOfflinePageCanBeSaved( + std::unique_ptr<OfflinePageMetadataStore> store); + protected: CalledCallback last_called_callback_; Status last_status_; std::vector<OfflinePageItem> offline_pages_; + OfflinePageMetadataStoreFactory factory_; base::ScopedTempDir temp_directory_; scoped_refptr<base::TestSimpleTaskRunner> task_runner_; base::ThreadTaskRunnerHandle task_runner_handle_; }; -OfflinePageMetadataStoreTestBase::OfflinePageMetadataStoreTestBase() +OfflinePageMetadataStoreTest::OfflinePageMetadataStoreTest() : last_called_callback_(NONE), last_status_(STATUS_NONE), task_runner_(new base::TestSimpleTaskRunner), @@ -147,13 +199,13 @@ EXPECT_TRUE(temp_directory_.CreateUniqueTempDir()); } -OfflinePageMetadataStoreTestBase::~OfflinePageMetadataStoreTestBase() {} +OfflinePageMetadataStoreTest::~OfflinePageMetadataStoreTest() {} -void OfflinePageMetadataStoreTestBase::PumpLoop() { +void OfflinePageMetadataStoreTest::PumpLoop() { task_runner_->RunUntilIdle(); } -void OfflinePageMetadataStoreTestBase::LoadCallback( +void OfflinePageMetadataStoreTest::LoadCallback( OfflinePageMetadataStore::LoadStatus load_status, const std::vector<OfflinePageItem>& offline_pages) { last_called_callback_ = LOAD; @@ -162,207 +214,197 @@ offline_pages_.swap(const_cast<std::vector<OfflinePageItem>&>(offline_pages)); } -void OfflinePageMetadataStoreTestBase::UpdateCallback( +void OfflinePageMetadataStoreTest::UpdateCallback( CalledCallback called_callback, bool status) { last_called_callback_ = called_callback; last_status_ = status ? STATUS_TRUE : STATUS_FALSE; } -void OfflinePageMetadataStoreTestBase::ClearResults() { +void OfflinePageMetadataStoreTest::ClearResults() { last_called_callback_ = NONE; last_status_ = STATUS_NONE; offline_pages_.clear(); } -template <typename T> -class OfflinePageMetadataStoreTest : public OfflinePageMetadataStoreTestBase { - public: - std::unique_ptr<OfflinePageMetadataStore> BuildStore(); - std::unique_ptr<OfflinePageMetadataStore> - BuildStoreWithUpgradeFromOutdatedSchema(); +OfflinePageItem OfflinePageMetadataStoreTest::CheckThatStoreHasOneItem() { + EXPECT_EQ(LOAD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); + EXPECT_EQ(1U, offline_pages_.size()); - protected: - T factory_; -}; + return offline_pages_[0]; +} -template <typename T> +void OfflinePageMetadataStoreTest::CheckThatOfflinePageCanBeSaved( + std::unique_ptr<OfflinePageMetadataStore> store) { + size_t store_size = offline_pages_.size(); + OfflinePageItem offline_page(GURL(kTestURL), 1234LL, kTestClientId1, + base::FilePath(kFilePath), kFileSize); + offline_page.title = base::UTF8ToUTF16("a title"); + base::Time expiration_time = base::Time::Now(); + offline_page.expiration_time = expiration_time; + + store->AddOrUpdateOfflinePage( + offline_page, base::Bind(&OfflinePageMetadataStoreTest::UpdateCallback, + base::Unretained(this), ADD)); + PumpLoop(); + EXPECT_EQ(ADD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); + ClearResults(); + + // Close the store first to ensure file lock is removed. + store.reset(); + store = BuildStore(); + PumpLoop(); + + EXPECT_EQ(LOAD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); + ASSERT_EQ(store_size + 1, offline_pages_.size()); + if (store_size > 0 && + offline_pages_[0].offline_id != offline_page.offline_id) { + std::swap(offline_pages_[0], offline_pages_[1]); + } + EXPECT_EQ(offline_page.url, offline_pages_[0].url); + EXPECT_EQ(offline_page.offline_id, offline_pages_[0].offline_id); + EXPECT_EQ(offline_page.file_path, offline_pages_[0].file_path); + EXPECT_EQ(offline_page.last_access_time, offline_pages_[0].last_access_time); + EXPECT_EQ(offline_page.expiration_time, offline_pages_[0].expiration_time); + EXPECT_EQ(offline_page.title, offline_pages_[0].title); + EXPECT_EQ(offline_page.client_id, offline_pages_[0].client_id); + EXPECT_EQ(1234LL, offline_pages_[0].offline_id); + EXPECT_EQ(kFileSize, offline_pages_[0].file_size); + EXPECT_EQ(kTestClientId1, offline_pages_[0].client_id); +} + std::unique_ptr<OfflinePageMetadataStore> -OfflinePageMetadataStoreTest<T>::BuildStore() { +OfflinePageMetadataStoreTest::BuildStore() { std::unique_ptr<OfflinePageMetadataStore> store( factory_.BuildStore(temp_directory_.path())); - store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback, + store->Load(base::Bind(&OfflinePageMetadataStoreTest::LoadCallback, base::Unretained(this))); PumpLoop(); return store; } -template <typename T> std::unique_ptr<OfflinePageMetadataStore> -OfflinePageMetadataStoreTest<T>::BuildStoreWithUpgradeFromOutdatedSchema() { +OfflinePageMetadataStoreTest::BuildStoreWithSchemaFromM52() { std::unique_ptr<OfflinePageMetadataStore> store( - factory_.BuildStoreV1(temp_directory_.path())); - store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback, + factory_.BuildStoreM52(temp_directory_.path())); + store->Load(base::Bind(&OfflinePageMetadataStoreTest::LoadCallback, base::Unretained(this))); PumpLoop(); return store; } -typedef testing::Types<OfflinePageMetadataStoreSQLFactory> MyTypes; -TYPED_TEST_CASE(OfflinePageMetadataStoreTest, MyTypes); +std::unique_ptr<OfflinePageMetadataStore> +OfflinePageMetadataStoreTest::BuildStoreWithSchemaFromM53() { + std::unique_ptr<OfflinePageMetadataStore> store( + factory_.BuildStoreM53(temp_directory_.path())); + store->Load(base::Bind(&OfflinePageMetadataStoreTest::LoadCallback, + base::Unretained(this))); + PumpLoop(); + return store; +} // Loads empty store and makes sure that there are no offline pages stored in // it. -TYPED_TEST(OfflinePageMetadataStoreTest, LoadEmptyStore) { - std::unique_ptr<OfflinePageMetadataStore> store(this->BuildStore()); - EXPECT_EQ(LOAD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - EXPECT_EQ(0U, this->offline_pages_.size()); +TEST_F(OfflinePageMetadataStoreTest, LoadEmptyStore) { + std::unique_ptr<OfflinePageMetadataStore> store(BuildStore()); + EXPECT_EQ(LOAD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); + EXPECT_EQ(0U, offline_pages_.size()); } // Loads a store which has an outdated schema. // This test case would crash if it's not handling correctly when we're loading // old version stores. // TODO(romax): Move this to sql_unittest. -TYPED_TEST(OfflinePageMetadataStoreTest, LoadPreviousVersionStore) { +TEST_F(OfflinePageMetadataStoreTest, LoadVersion52Store) { std::unique_ptr<OfflinePageMetadataStore> store( - this->BuildStoreWithUpgradeFromOutdatedSchema()); - EXPECT_EQ(LOAD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - EXPECT_EQ(1U, this->offline_pages_.size()); - OfflinePageItem offline_page(GURL(kTestURL), 1234LL, kTestClientId1, - base::FilePath(kFilePath), kFileSize); - base::Time expiration_time = base::Time::Now(); - offline_page.expiration_time = expiration_time; - store->AddOrUpdateOfflinePage( - offline_page, - base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback, - base::Unretained(this), ADD)); - this->PumpLoop(); - EXPECT_EQ(ADD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - this->ClearResults(); + BuildStoreWithSchemaFromM52()); - // Close the store first to ensure file lock is removed. - store.reset(); - store = this->BuildStore(); - this->PumpLoop(); + CheckThatStoreHasOneItem(); + CheckThatOfflinePageCanBeSaved(std::move(store)); +} - EXPECT_EQ(LOAD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - ASSERT_EQ(2U, this->offline_pages_.size()); - if (this->offline_pages_[0].offline_id != offline_page.offline_id) { - std::swap(this->offline_pages_[0], this->offline_pages_[1]); - } - EXPECT_EQ(offline_page.url, this->offline_pages_[0].url); - EXPECT_EQ(offline_page.offline_id, this->offline_pages_[0].offline_id); - EXPECT_EQ(offline_page.file_path, this->offline_pages_[0].file_path); - EXPECT_EQ(offline_page.last_access_time, - this->offline_pages_[0].last_access_time); - EXPECT_EQ(offline_page.expiration_time, - this->offline_pages_[0].expiration_time); - EXPECT_EQ(offline_page.client_id, this->offline_pages_[0].client_id); - EXPECT_EQ(kOfflineId, this->offline_pages_[1].offline_id); - EXPECT_EQ(kFileSize, this->offline_pages_[1].file_size); - EXPECT_EQ(kTestClientId2, this->offline_pages_[1].client_id); +// Loads a store which has an outdated schema. +// This test case would crash if it's not handling correctly when we're loading +// old version stores. +// TODO(romax): Move this to sql_unittest. +TEST_F(OfflinePageMetadataStoreTest, LoadVersion53Store) { + std::unique_ptr<OfflinePageMetadataStore> store( + BuildStoreWithSchemaFromM53()); + + OfflinePageItem item = CheckThatStoreHasOneItem(); + // We should have a valid expiration time after upgrade. + EXPECT_NE(base::Time::FromInternalValue(0), + offline_pages_[0].expiration_time); + + CheckThatOfflinePageCanBeSaved(std::move(store)); } // Adds metadata of an offline page into a store and then opens the store // again to make sure that stored metadata survives store restarts. -TYPED_TEST(OfflinePageMetadataStoreTest, AddOfflinePage) { - std::unique_ptr<OfflinePageMetadataStore> store(this->BuildStore()); - OfflinePageItem offline_page(GURL(kTestURL), 1234LL, kTestClientId1, - base::FilePath(kFilePath), kFileSize); - store->AddOrUpdateOfflinePage( - offline_page, - base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback, - base::Unretained(this), ADD)); - this->PumpLoop(); - EXPECT_EQ(ADD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - - this->ClearResults(); - - // Close the store first to ensure file lock is removed. - store.reset(); - store = this->BuildStore(); - this->PumpLoop(); - - EXPECT_EQ(LOAD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - ASSERT_EQ(1U, this->offline_pages_.size()); - EXPECT_EQ(offline_page.url, this->offline_pages_[0].url); - EXPECT_EQ(offline_page.offline_id, this->offline_pages_[0].offline_id); - EXPECT_EQ(offline_page.version, this->offline_pages_[0].version); - EXPECT_EQ(offline_page.file_path, this->offline_pages_[0].file_path); - EXPECT_EQ(offline_page.file_size, this->offline_pages_[0].file_size); - EXPECT_EQ(offline_page.creation_time, this->offline_pages_[0].creation_time); - EXPECT_EQ(offline_page.last_access_time, - this->offline_pages_[0].last_access_time); - EXPECT_EQ(offline_page.expiration_time, - this->offline_pages_[0].expiration_time); - EXPECT_EQ(offline_page.access_count, this->offline_pages_[0].access_count); - EXPECT_EQ(offline_page.client_id, this->offline_pages_[0].client_id); +TEST_F(OfflinePageMetadataStoreTest, AddOfflinePage) { + CheckThatOfflinePageCanBeSaved(BuildStore()); } // Tests removing offline page metadata from the store, for which it first adds // metadata of an offline page. -TYPED_TEST(OfflinePageMetadataStoreTest, RemoveOfflinePage) { - std::unique_ptr<OfflinePageMetadataStore> store(this->BuildStore()); +TEST_F(OfflinePageMetadataStoreTest, RemoveOfflinePage) { + std::unique_ptr<OfflinePageMetadataStore> store(BuildStore()); // Add an offline page. OfflinePageItem offline_page(GURL(kTestURL), 1234LL, kTestClientId1, base::FilePath(kFilePath), kFileSize); store->AddOrUpdateOfflinePage( - offline_page, - base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback, - base::Unretained(this), ADD)); - this->PumpLoop(); - EXPECT_EQ(ADD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); + offline_page, base::Bind(&OfflinePageMetadataStoreTest::UpdateCallback, + base::Unretained(this), ADD)); + PumpLoop(); + EXPECT_EQ(ADD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); - this->ClearResults(); + ClearResults(); // Load the store. - store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback, + store->Load(base::Bind(&OfflinePageMetadataStoreTest::LoadCallback, base::Unretained(this))); - this->PumpLoop(); - EXPECT_EQ(LOAD, this->last_called_callback_); - EXPECT_EQ(1U, this->offline_pages_.size()); + PumpLoop(); + EXPECT_EQ(LOAD, last_called_callback_); + EXPECT_EQ(1U, offline_pages_.size()); // Remove the offline page. std::vector<int64_t> ids_to_remove; ids_to_remove.push_back(offline_page.offline_id); store->RemoveOfflinePages( - ids_to_remove, - base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback, - base::Unretained(this), REMOVE)); - this->PumpLoop(); - EXPECT_EQ(REMOVE, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); + ids_to_remove, base::Bind(&OfflinePageMetadataStoreTest::UpdateCallback, + base::Unretained(this), REMOVE)); + PumpLoop(); + EXPECT_EQ(REMOVE, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); - this->ClearResults(); + ClearResults(); // Load the store. - store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback, + store->Load(base::Bind(&OfflinePageMetadataStoreTest::LoadCallback, base::Unretained(this))); - this->PumpLoop(); - EXPECT_EQ(LOAD, this->last_called_callback_); - EXPECT_EQ(0U, this->offline_pages_.size()); + PumpLoop(); + EXPECT_EQ(LOAD, last_called_callback_); + EXPECT_EQ(0U, offline_pages_.size()); - this->ClearResults(); + ClearResults(); // Close and reload the store. store.reset(); - store = this->BuildStore(); - EXPECT_EQ(LOAD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - EXPECT_EQ(0U, this->offline_pages_.size()); + store = BuildStore(); + EXPECT_EQ(LOAD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); + EXPECT_EQ(0U, offline_pages_.size()); } // Adds metadata of multiple offline pages into a store and removes some. -TYPED_TEST(OfflinePageMetadataStoreTest, AddRemoveMultipleOfflinePages) { - std::unique_ptr<OfflinePageMetadataStore> store(this->BuildStore()); +TEST_F(OfflinePageMetadataStoreTest, AddRemoveMultipleOfflinePages) { + std::unique_ptr<OfflinePageMetadataStore> store(BuildStore()); // Add an offline page. OfflinePageItem offline_page_1(GURL(kTestURL), 12345LL, kTestClientId1, @@ -374,195 +416,182 @@ base::Time::Now()); offline_page_2.expiration_time = base::Time::Now(); store->AddOrUpdateOfflinePage( - offline_page_1, - base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback, - base::Unretained(this), ADD)); - this->PumpLoop(); - EXPECT_EQ(ADD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); + offline_page_1, base::Bind(&OfflinePageMetadataStoreTest::UpdateCallback, + base::Unretained(this), ADD)); + PumpLoop(); + EXPECT_EQ(ADD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); - this->ClearResults(); + ClearResults(); // Add anther offline page. store->AddOrUpdateOfflinePage( - offline_page_2, - base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback, - base::Unretained(this), ADD)); - this->PumpLoop(); - EXPECT_EQ(ADD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); + offline_page_2, base::Bind(&OfflinePageMetadataStoreTest::UpdateCallback, + base::Unretained(this), ADD)); + PumpLoop(); + EXPECT_EQ(ADD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); - this->ClearResults(); + ClearResults(); // Load the store. - store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback, + store->Load(base::Bind(&OfflinePageMetadataStoreTest::LoadCallback, base::Unretained(this))); - this->PumpLoop(); + PumpLoop(); - EXPECT_EQ(LOAD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - EXPECT_EQ(2U, this->offline_pages_.size()); + EXPECT_EQ(LOAD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); + EXPECT_EQ(2U, offline_pages_.size()); // Remove the offline page. std::vector<int64_t> ids_to_remove; ids_to_remove.push_back(offline_page_1.offline_id); store->RemoveOfflinePages( - ids_to_remove, - base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback, - base::Unretained(this), REMOVE)); - this->PumpLoop(); - EXPECT_EQ(REMOVE, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); + ids_to_remove, base::Bind(&OfflinePageMetadataStoreTest::UpdateCallback, + base::Unretained(this), REMOVE)); + PumpLoop(); + EXPECT_EQ(REMOVE, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); - this->ClearResults(); + ClearResults(); // Close and reload the store. store.reset(); - store = this->BuildStore(); - store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback, + store = BuildStore(); + store->Load(base::Bind(&OfflinePageMetadataStoreTest::LoadCallback, base::Unretained(this))); - this->PumpLoop(); + PumpLoop(); - EXPECT_EQ(LOAD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - ASSERT_EQ(1U, this->offline_pages_.size()); - EXPECT_EQ(offline_page_2.url, this->offline_pages_[0].url); - EXPECT_EQ(offline_page_2.offline_id, this->offline_pages_[0].offline_id); - EXPECT_EQ(offline_page_2.version, this->offline_pages_[0].version); - EXPECT_EQ(offline_page_2.file_path, this->offline_pages_[0].file_path); - EXPECT_EQ(offline_page_2.file_size, this->offline_pages_[0].file_size); - EXPECT_EQ(offline_page_2.creation_time, - this->offline_pages_[0].creation_time); + EXPECT_EQ(LOAD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); + ASSERT_EQ(1U, offline_pages_.size()); + EXPECT_EQ(offline_page_2.url, offline_pages_[0].url); + EXPECT_EQ(offline_page_2.offline_id, offline_pages_[0].offline_id); + EXPECT_EQ(offline_page_2.version, offline_pages_[0].version); + EXPECT_EQ(offline_page_2.file_path, offline_pages_[0].file_path); + EXPECT_EQ(offline_page_2.file_size, offline_pages_[0].file_size); + EXPECT_EQ(offline_page_2.creation_time, offline_pages_[0].creation_time); EXPECT_EQ(offline_page_2.last_access_time, - this->offline_pages_[0].last_access_time); - EXPECT_EQ(offline_page_2.expiration_time, - this->offline_pages_[0].expiration_time); - EXPECT_EQ(offline_page_2.access_count, this->offline_pages_[0].access_count); - EXPECT_EQ(offline_page_2.client_id, this->offline_pages_[0].client_id); + offline_pages_[0].last_access_time); + EXPECT_EQ(offline_page_2.expiration_time, offline_pages_[0].expiration_time); + EXPECT_EQ(offline_page_2.access_count, offline_pages_[0].access_count); + EXPECT_EQ(offline_page_2.client_id, offline_pages_[0].client_id); } // Tests updating offline page metadata from the store. -TYPED_TEST(OfflinePageMetadataStoreTest, UpdateOfflinePage) { - std::unique_ptr<OfflinePageMetadataStore> store(this->BuildStore()); +TEST_F(OfflinePageMetadataStoreTest, UpdateOfflinePage) { + std::unique_ptr<OfflinePageMetadataStore> store(BuildStore()); // First, adds a fresh page. OfflinePageItem offline_page(GURL(kTestURL), 1234LL, kTestClientId1, base::FilePath(kFilePath), kFileSize); store->AddOrUpdateOfflinePage( - offline_page, - base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback, - base::Unretained(this), ADD)); - this->PumpLoop(); - EXPECT_EQ(ADD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); + offline_page, base::Bind(&OfflinePageMetadataStoreTest::UpdateCallback, + base::Unretained(this), ADD)); + PumpLoop(); + EXPECT_EQ(ADD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); - this->ClearResults(); - store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback, + ClearResults(); + store->Load(base::Bind(&OfflinePageMetadataStoreTest::LoadCallback, base::Unretained(this))); - this->PumpLoop(); + PumpLoop(); - EXPECT_EQ(LOAD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - ASSERT_EQ(1U, this->offline_pages_.size()); - EXPECT_EQ(offline_page.url, this->offline_pages_[0].url); - EXPECT_EQ(offline_page.offline_id, this->offline_pages_[0].offline_id); - EXPECT_EQ(offline_page.version, this->offline_pages_[0].version); - EXPECT_EQ(offline_page.file_path, this->offline_pages_[0].file_path); - EXPECT_EQ(offline_page.file_size, this->offline_pages_[0].file_size); - EXPECT_EQ(offline_page.creation_time, this->offline_pages_[0].creation_time); - EXPECT_EQ(offline_page.last_access_time, - this->offline_pages_[0].last_access_time); - EXPECT_EQ(offline_page.expiration_time, - this->offline_pages_[0].expiration_time); - EXPECT_EQ(offline_page.access_count, this->offline_pages_[0].access_count); - EXPECT_EQ(offline_page.client_id, this->offline_pages_[0].client_id); + EXPECT_EQ(LOAD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); + ASSERT_EQ(1U, offline_pages_.size()); + EXPECT_EQ(offline_page.url, offline_pages_[0].url); + EXPECT_EQ(offline_page.offline_id, offline_pages_[0].offline_id); + EXPECT_EQ(offline_page.version, offline_pages_[0].version); + EXPECT_EQ(offline_page.file_path, offline_pages_[0].file_path); + EXPECT_EQ(offline_page.file_size, offline_pages_[0].file_size); + EXPECT_EQ(offline_page.creation_time, offline_pages_[0].creation_time); + EXPECT_EQ(offline_page.last_access_time, offline_pages_[0].last_access_time); + EXPECT_EQ(offline_page.expiration_time, offline_pages_[0].expiration_time); + EXPECT_EQ(offline_page.access_count, offline_pages_[0].access_count); + EXPECT_EQ(offline_page.client_id, offline_pages_[0].client_id); // Then update some data. offline_page.file_size = kFileSize + 1; offline_page.access_count++; offline_page.expiration_time = base::Time::Now(); store->AddOrUpdateOfflinePage( - offline_page, - base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback, - base::Unretained(this), ADD)); - this->PumpLoop(); - EXPECT_EQ(ADD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); + offline_page, base::Bind(&OfflinePageMetadataStoreTest::UpdateCallback, + base::Unretained(this), ADD)); + PumpLoop(); + EXPECT_EQ(ADD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); - this->ClearResults(); - store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback, + ClearResults(); + store->Load(base::Bind(&OfflinePageMetadataStoreTest::LoadCallback, base::Unretained(this))); - this->PumpLoop(); + PumpLoop(); - EXPECT_EQ(LOAD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - ASSERT_EQ(1U, this->offline_pages_.size()); - EXPECT_EQ(offline_page.url, this->offline_pages_[0].url); - EXPECT_EQ(offline_page.offline_id, this->offline_pages_[0].offline_id); - EXPECT_EQ(offline_page.version, this->offline_pages_[0].version); - EXPECT_EQ(offline_page.file_path, this->offline_pages_[0].file_path); - EXPECT_EQ(offline_page.file_size, this->offline_pages_[0].file_size); - EXPECT_EQ(offline_page.creation_time, this->offline_pages_[0].creation_time); - EXPECT_EQ(offline_page.last_access_time, - this->offline_pages_[0].last_access_time); - EXPECT_EQ(offline_page.expiration_time, - this->offline_pages_[0].expiration_time); - EXPECT_EQ(offline_page.access_count, this->offline_pages_[0].access_count); - EXPECT_EQ(offline_page.client_id, this->offline_pages_[0].client_id); + EXPECT_EQ(LOAD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); + ASSERT_EQ(1U, offline_pages_.size()); + EXPECT_EQ(offline_page.url, offline_pages_[0].url); + EXPECT_EQ(offline_page.offline_id, offline_pages_[0].offline_id); + EXPECT_EQ(offline_page.version, offline_pages_[0].version); + EXPECT_EQ(offline_page.file_path, offline_pages_[0].file_path); + EXPECT_EQ(offline_page.file_size, offline_pages_[0].file_size); + EXPECT_EQ(offline_page.creation_time, offline_pages_[0].creation_time); + EXPECT_EQ(offline_page.last_access_time, offline_pages_[0].last_access_time); + EXPECT_EQ(offline_page.expiration_time, offline_pages_[0].expiration_time); + EXPECT_EQ(offline_page.access_count, offline_pages_[0].access_count); + EXPECT_EQ(offline_page.client_id, offline_pages_[0].client_id); } -TYPED_TEST(OfflinePageMetadataStoreTest, ClearAllOfflinePages) { - std::unique_ptr<OfflinePageMetadataStore> store(this->BuildStore()); +TEST_F(OfflinePageMetadataStoreTest, ClearAllOfflinePages) { + std::unique_ptr<OfflinePageMetadataStore> store(BuildStore()); // Add 2 offline pages. OfflinePageItem offline_page(GURL(kTestURL), 1234LL, kTestClientId1, base::FilePath(kFilePath), kFileSize); store->AddOrUpdateOfflinePage( - offline_page, - base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback, - base::Unretained(this), ADD)); - this->PumpLoop(); - EXPECT_EQ(ADD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); + offline_page, base::Bind(&OfflinePageMetadataStoreTest::UpdateCallback, + base::Unretained(this), ADD)); + PumpLoop(); + EXPECT_EQ(ADD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); - this->ClearResults(); + ClearResults(); OfflinePageItem offline_page2(GURL("http://test.com"), 5678LL, kTestClientId2, base::FilePath(kFilePath), kFileSize); store->AddOrUpdateOfflinePage( - offline_page2, - base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback, - base::Unretained(this), ADD)); - this->PumpLoop(); - EXPECT_EQ(ADD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); + offline_page2, base::Bind(&OfflinePageMetadataStoreTest::UpdateCallback, + base::Unretained(this), ADD)); + PumpLoop(); + EXPECT_EQ(ADD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); - this->ClearResults(); + ClearResults(); // Load the store. - store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback, + store->Load(base::Bind(&OfflinePageMetadataStoreTest::LoadCallback, base::Unretained(this))); - this->PumpLoop(); + PumpLoop(); - EXPECT_EQ(LOAD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - EXPECT_EQ(2U, this->offline_pages_.size()); + EXPECT_EQ(LOAD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); + EXPECT_EQ(2U, offline_pages_.size()); // Clear all records from the store. - store->Reset(base::Bind(&OfflinePageMetadataStoreTestBase::UpdateCallback, + store->Reset(base::Bind(&OfflinePageMetadataStoreTest::UpdateCallback, base::Unretained(this), RESET)); - this->PumpLoop(); - EXPECT_EQ(RESET, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); + PumpLoop(); + EXPECT_EQ(RESET, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); // Load the store. - store->Load(base::Bind(&OfflinePageMetadataStoreTestBase::LoadCallback, + store->Load(base::Bind(&OfflinePageMetadataStoreTest::LoadCallback, base::Unretained(this))); - this->PumpLoop(); + PumpLoop(); - EXPECT_EQ(LOAD, this->last_called_callback_); - EXPECT_EQ(STATUS_TRUE, this->last_status_); - ASSERT_EQ(0U, this->offline_pages_.size()); + EXPECT_EQ(LOAD, last_called_callback_); + EXPECT_EQ(STATUS_TRUE, last_status_); + ASSERT_EQ(0U, offline_pages_.size()); } } // namespace
diff --git a/components/offline_pages/offline_page_metadata_store_sql.cc b/components/offline_pages/offline_page_metadata_store_sql.cc index 3d7cdcf..e99e267 100644 --- a/components/offline_pages/offline_page_metadata_store_sql.cc +++ b/components/offline_pages/offline_page_metadata_store_sql.cc
@@ -45,7 +45,8 @@ " client_id VARCHAR NOT NULL," " online_url VARCHAR NOT NULL," " offline_url VARCHAR NOT NULL DEFAULT ''," - " file_path VARCHAR NOT NULL" + " file_path VARCHAR NOT NULL," + " title VARCHAR NOT NULL DEFAULT ''" ")"; // This is cloned from //content/browser/appcache/appcache_database.cc @@ -68,12 +69,13 @@ OP_ACCESS_COUNT, OP_STATUS, OP_USER_INITIATED, - OP_EXPIRATION_TIME, + OP_EXPIRATION_TIME, // Added in M53. OP_CLIENT_NAMESPACE, OP_CLIENT_ID, OP_ONLINE_URL, OP_OFFLINE_URL, OP_FILE_PATH, + OP_TITLE, // Added in M54. }; bool CreateTable(sql::Connection* db, const TableInfo& table_info) { @@ -83,7 +85,7 @@ return db->Execute(sql.c_str()); } -bool RefreshColumns(sql::Connection* db) { +bool RefreshColumnsFrom52To54(sql::Connection* db) { if (!db->Execute("ALTER TABLE " OFFLINE_PAGES_TABLE_NAME " RENAME TO temp_" OFFLINE_PAGES_TABLE_NAME)) { return false; @@ -108,6 +110,31 @@ return true; } +bool RefreshColumnsFrom53To54(sql::Connection* db) { + if (!db->Execute("ALTER TABLE " OFFLINE_PAGES_TABLE_NAME + " RENAME TO temp_" OFFLINE_PAGES_TABLE_NAME)) { + return false; + } + if (!CreateTable(db, kOfflinePagesTable)) + return false; + if (!db->Execute( + "INSERT INTO " OFFLINE_PAGES_TABLE_NAME + " (offline_id, creation_time, file_size, version, last_access_time, " + "access_count, status, user_initiated, client_namespace, client_id, " + "online_url, offline_url, file_path, expiration_time) " + "SELECT offline_id, creation_time, file_size, version, " + "last_access_time, " + "access_count, status, user_initiated, client_namespace, client_id, " + "online_url, offline_url, file_path, expiration_time " + "FROM temp_" OFFLINE_PAGES_TABLE_NAME)) { + return false; + } + if (!db->Execute("DROP TABLE IF EXISTS temp_" OFFLINE_PAGES_TABLE_NAME)) + return false; + + return true; +} + bool CreateSchema(sql::Connection* db) { // If you create a transaction but don't Commit() it is automatically // rolled back by its destructor when it falls out of scope. @@ -121,7 +148,10 @@ } if (!db->DoesColumnExist(kOfflinePagesTable.table_name, "expiration_time")) { - if (!RefreshColumns(db)) + if (!RefreshColumnsFrom52To54(db)) + return false; + } else if (!db->DoesColumnExist(kOfflinePagesTable.table_name, "title")) { + if (!RefreshColumnsFrom53To54(db)) return false; } @@ -162,6 +192,7 @@ item.access_count = statement->ColumnInt(OP_ACCESS_COUNT); item.expiration_time = base::Time::FromInternalValue(statement->ColumnInt64(OP_EXPIRATION_TIME)); + item.title = statement->ColumnString16(OP_TITLE); return item; } @@ -170,9 +201,9 @@ "INSERT OR REPLACE INTO " OFFLINE_PAGES_TABLE_NAME " (offline_id, online_url, client_namespace, client_id, file_path, " "file_size, creation_time, last_access_time, version, access_count, " - "expiration_time)" + "expiration_time, title)" " VALUES " - " (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"; + " (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"; sql::Statement statement(db->GetCachedStatement(SQL_FROM_HERE, kSql)); statement.BindInt64(0, item.offline_id); @@ -193,6 +224,7 @@ statement.BindInt(8, item.version); statement.BindInt(9, item.access_count); statement.BindInt64(10, item.expiration_time.ToInternalValue()); + statement.BindString16(11, item.title); return statement.Run(); }
diff --git a/content/renderer/android/content_detector.cc b/content/renderer/android/content_detector.cc index 94d7e3d4..22a4fe9 100644 --- a/content/renderer/android/content_detector.cc +++ b/content/renderer/android/content_detector.cc
@@ -9,54 +9,36 @@ #include "third_party/WebKit/public/web/WebHitTestResult.h" #include "third_party/WebKit/public/web/WebSurroundingText.h" -using blink::WebRange; +using blink::WebURL; +using blink::WebHitTestResult; using blink::WebSurroundingText; namespace content { -ContentDetector::Result::Result() : valid(false) {} - -ContentDetector::Result::Result(const blink::WebRange& content_boundaries, - const std::string& text, - const GURL& intent_url) - : valid(true), - content_boundaries(content_boundaries), - text(text), - intent_url(intent_url) { -} - -ContentDetector::Result::Result(const Result& other) = default; - -ContentDetector::Result::~Result() {} - -ContentDetector::Result ContentDetector::FindTappedContent( - const blink::WebHitTestResult& hit_test) { +WebURL ContentDetector::FindTappedContent(const WebHitTestResult& hit_test) { if (hit_test.isNull()) - return Result(); + return WebURL(); std::string content_text; - blink::WebRange range = FindContentRange(hit_test, &content_text); - if (range.isNull()) - return Result(); + if (!FindContentRange(hit_test, &content_text)) + return WebURL(); - GURL intent_url = GetIntentURL(content_text); - return Result(range, content_text, intent_url); + return GetIntentURL(content_text); } -WebRange ContentDetector::FindContentRange( - const blink::WebHitTestResult& hit_test, - std::string* content_text) { +bool ContentDetector::FindContentRange(const WebHitTestResult& hit_test, + std::string* content_text) { // As the surrounding text extractor looks at maxLength/2 characters on // either side of the hit point, we need to double max content length here. WebSurroundingText surrounding_text; surrounding_text.initialize(hit_test.node(), hit_test.localPoint(), GetMaximumContentLength() * 2); if (surrounding_text.isNull()) - return WebRange(); + return false; base::string16 content = surrounding_text.textContent(); if (content.empty()) - return WebRange(); + return false; size_t selected_offset = surrounding_text.hitOffsetInTextContent(); for (size_t start_offset = 0; start_offset < content.length();) { @@ -69,18 +51,14 @@ size_t content_end = start_offset + relative_end; DCHECK(content_end <= content.length()); - if (selected_offset >= content_start && selected_offset < content_end) { - WebRange range = surrounding_text.rangeFromContentOffsets( - content_start, content_end); - DCHECK(!range.isNull()); - return range; - } else { + if (selected_offset >= content_start && selected_offset < content_end) + return true; + else start_offset += relative_end; - } } } - return WebRange(); + return false; } } // namespace content
diff --git a/content/renderer/android/content_detector.h b/content/renderer/android/content_detector.h index 4488f14..ea85fc7 100644 --- a/content/renderer/android/content_detector.h +++ b/content/renderer/android/content_detector.h
@@ -8,7 +8,7 @@ #include <stddef.h> #include "base/macros.h" -#include "third_party/WebKit/public/web/WebRange.h" +#include "third_party/WebKit/public/platform/WebURL.h" #include "url/gurl.h" namespace blink { @@ -20,26 +20,11 @@ // Base class for text-based content detectors. class ContentDetector { public: - // Holds the content detection results. - struct Result { - Result(); - Result(const blink::WebRange& content_boundaries, - const std::string& text, - const GURL& intent_url); - Result(const Result& other); - ~Result(); - - bool valid; - blink::WebRange content_boundaries; - std::string text; // Processed text of the content. - GURL intent_url; // URL of the intent that should process this content. - }; - virtual ~ContentDetector() {} - // Returns a WebKit range delimiting the contents found around the tapped - // position. If no content is found a null range will be returned. - Result FindTappedContent(const blink::WebHitTestResult& hit_test); + // Returns an intent URL for the content around the hit_test. + // If no content is found, an empty URL will be returned. + blink::WebURL FindTappedContent(const blink::WebHitTestResult& hit_test); protected: ContentDetector() {} @@ -61,8 +46,8 @@ // position in order to search for content. virtual size_t GetMaximumContentLength() = 0; - blink::WebRange FindContentRange(const blink::WebHitTestResult& hit_test, - std::string* content_text); + bool FindContentRange(const blink::WebHitTestResult& hit_test, + std::string* content_text); DISALLOW_COPY_AND_ASSIGN(ContentDetector); };
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc index 5766f5e..cacdbad 100644 --- a/content/renderer/render_view_impl.cc +++ b/content/renderer/render_view_impl.cc
@@ -2813,9 +2813,9 @@ // Process the position with all the registered content detectors until // a match is found. Priority is provided by their relative order. for (const auto& detector : content_detectors_) { - ContentDetector::Result content = detector->FindTappedContent(touch_hit); - if (content.valid) { - return content.intent_url; + WebURL intent = detector->FindTappedContent(touch_hit); + if (intent.isValid()) { + return intent; } } return WebURL();
diff --git a/third_party/WebKit/Source/core/editing/SurroundingText.cpp b/third_party/WebKit/Source/core/editing/SurroundingText.cpp index dabea49..bb77ee8 100644 --- a/third_party/WebKit/Source/core/editing/SurroundingText.cpp +++ b/third_party/WebKit/Source/core/editing/SurroundingText.cpp
@@ -89,27 +89,6 @@ DCHECK(m_contentRange); } -Range* SurroundingText::rangeFromContentOffsets(unsigned startOffsetInContent, unsigned endOffsetInContent) -{ - if (startOffsetInContent >= endOffsetInContent || endOffsetInContent > content().length()) - return nullptr; - - CharacterIterator iterator(m_contentRange->startPosition(), m_contentRange->endPosition()); - - DCHECK(!iterator.atEnd()); - iterator.advance(startOffsetInContent); - - Position start = iterator.startPosition(); - - DCHECK(!iterator.atEnd()); - iterator.advance(endOffsetInContent - startOffsetInContent); - - Position end = iterator.startPosition(); - - DCHECK(start.document()); - return Range::create(*start.document(), start, end); -} - String SurroundingText::content() const { if (m_contentRange)
diff --git a/third_party/WebKit/Source/core/editing/SurroundingText.h b/third_party/WebKit/Source/core/editing/SurroundingText.h index c0a4b69d..f386468e 100644 --- a/third_party/WebKit/Source/core/editing/SurroundingText.h +++ b/third_party/WebKit/Source/core/editing/SurroundingText.h
@@ -51,8 +51,6 @@ unsigned startOffsetInContent() const; unsigned endOffsetInContent() const; - Range* rangeFromContentOffsets(unsigned startOffsetInContent, unsigned endOffsetInContent); - private: void initialize(const Position&, const Position&, unsigned maxLength);
diff --git a/third_party/WebKit/Source/web/WebSurroundingText.cpp b/third_party/WebKit/Source/web/WebSurroundingText.cpp index 6fd33f8e..1584e7c 100644 --- a/third_party/WebKit/Source/web/WebSurroundingText.cpp +++ b/third_party/WebKit/Source/web/WebSurroundingText.cpp
@@ -80,11 +80,6 @@ return m_private->endOffsetInContent(); } -WebRange WebSurroundingText::rangeFromContentOffsets(size_t startOffsetInContent, size_t endOffsetInContent) -{ - return m_private->rangeFromContentOffsets(startOffsetInContent, endOffsetInContent); -} - bool WebSurroundingText::isNull() const { return !m_private.get();
diff --git a/third_party/WebKit/public/web/WebSurroundingText.h b/third_party/WebKit/public/web/WebSurroundingText.h index 4b7af237..e5315c6 100644 --- a/third_party/WebKit/public/web/WebSurroundingText.h +++ b/third_party/WebKit/public/web/WebSurroundingText.h
@@ -72,10 +72,6 @@ // End offset of the initial text in the text content. BLINK_EXPORT size_t endOffsetInTextContent() const; - // Convert start/end positions in the content text string into a WebKit text - // range. - BLINK_EXPORT WebRange rangeFromContentOffsets(size_t startOffsetInContent, size_t endOffsetInContent); - protected: std::unique_ptr<SurroundingText> m_private; };