Commit f6629637 authored by mtomasz's avatar mtomasz Committed by Commit bot

Allow offsets in blobs larger than 2GB on 32 bit Chromium builds.

The recent patch crrev.com/895933007 introduced a regression by using size_t
for offsets, which on 32bit builds is sizeof(size_t) = 4 which was causing
broken offset values due to assigning a uint64_t variable to such variables.

This CL fixes that by simply converting the types to uint64_t.

TEST=*FileSystemProvider*BigFile* on a 32 bit build Chromium build with
    chromeos=1.
BUG=375297, 458122

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

Cr-Commit-Position: refs/heads/master@{#318808}
parent 47904be7
......@@ -56,7 +56,7 @@ IN_PROC_BROWSER_TEST_F(FileSystemProviderApiTest, ReadFile) {
<< message_;
}
IN_PROC_BROWSER_TEST_F(FileSystemProviderApiTest, DISABLED_BigFile) {
IN_PROC_BROWSER_TEST_F(FileSystemProviderApiTest, BigFile) {
ASSERT_TRUE(RunPlatformAppTestWithFlags("file_system_provider/big_file",
kFlagLoadAsComponent))
<< message_;
......
......@@ -2,6 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "storage/browser/blob/blob_storage_context.h"
#include <limits>
#include <string>
#include "base/files/file_path.h"
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
......@@ -11,24 +16,19 @@
#include "content/browser/fileapi/blob_storage_host.h"
#include "storage/browser/blob/blob_data_builder.h"
#include "storage/browser/blob/blob_data_handle.h"
#include "storage/browser/blob/blob_data_item.h"
#include "storage/browser/blob/blob_data_snapshot.h"
#include "storage/browser/blob/blob_storage_context.h"
#include "testing/gtest/include/gtest/gtest.h"
using storage::BlobDataBuilder;
using storage::BlobDataHandle;
using storage::BlobDataItem;
using storage::BlobDataSnapshot;
using storage::BlobStorageContext;
using storage::DataElement;
namespace content {
namespace {
// This is necessary to clean up the blobs after the handles are released.
void RunEventLoopTillIdle() {
base::RunLoop run_loop;
run_loop.RunUntilIdle();
}
void SetupBasicBlob(BlobStorageHost* host, const std::string& id) {
EXPECT_TRUE(host->StartBuildingBlob(id));
......@@ -38,6 +38,7 @@ void SetupBasicBlob(BlobStorageHost* host, const std::string& id) {
EXPECT_TRUE(host->FinishBuildingBlob(id, "text/plain"));
EXPECT_FALSE(host->StartBuildingBlob(id));
}
} // namespace
TEST(BlobStorageContextTest, IncrementDecrementRef) {
......@@ -54,7 +55,7 @@ TEST(BlobStorageContextTest, IncrementDecrementRef) {
blob_data_handle = context.GetBlobDataFromUUID(kId);
EXPECT_TRUE(blob_data_handle);
blob_data_handle.reset();
RunEventLoopTillIdle();
base::RunLoop().RunUntilIdle();
// Make sure its still there after inc/dec.
EXPECT_TRUE(host.IncrementBlobRefCount(kId));
......@@ -62,7 +63,7 @@ TEST(BlobStorageContextTest, IncrementDecrementRef) {
blob_data_handle = context.GetBlobDataFromUUID(kId);
EXPECT_TRUE(blob_data_handle);
blob_data_handle.reset();
RunEventLoopTillIdle();
base::RunLoop().RunUntilIdle();
// Make sure it goes away in the end.
EXPECT_TRUE(host.DecrementBlobRefCount(kId));
......@@ -113,7 +114,7 @@ TEST(BlobStorageContextTest, BlobDataHandle) {
// Should disappear after dropping both handles.
blob_data_handle.reset();
another_handle.reset();
RunEventLoopTillIdle();
base::RunLoop().RunUntilIdle();
blob_data_handle = context.GetBlobDataFromUUID(kId);
EXPECT_FALSE(blob_data_handle);
......@@ -147,11 +148,11 @@ TEST(BlobStorageContextTest, MemoryUsage) {
EXPECT_EQ(10lu, context.memory_usage());
blob_data_handle.reset();
RunEventLoopTillIdle();
base::RunLoop().RunUntilIdle();
EXPECT_EQ(10lu, context.memory_usage());
blob_data_handle2.reset();
RunEventLoopTillIdle();
base::RunLoop().RunUntilIdle();
EXPECT_EQ(0lu, context.memory_usage());
}
......@@ -190,7 +191,7 @@ TEST(BlobStorageContextTest, AddFinishedBlob) {
blob_data_handle.reset();
data2.reset();
RunEventLoopTillIdle();
base::RunLoop().RunUntilIdle();
blob_data_handle = context.GetBlobDataFromUUID(kId1);
EXPECT_FALSE(blob_data_handle);
......@@ -205,7 +206,7 @@ TEST(BlobStorageContextTest, AddFinishedBlob) {
scoped_ptr<BlobDataHandle> blob_data_handle3 =
context.AddFinishedBlob(&builder3);
blob_data_handle2.reset();
RunEventLoopTillIdle();
base::RunLoop().RunUntilIdle();
blob_data_handle2 = context.GetBlobDataFromUUID(kId2);
EXPECT_FALSE(blob_data_handle2);
......@@ -222,7 +223,43 @@ TEST(BlobStorageContextTest, AddFinishedBlob) {
blob_data_handle.reset();
blob_data_handle2.reset();
blob_data_handle3.reset();
RunEventLoopTillIdle();
base::RunLoop().RunUntilIdle();
}
TEST(BlobStorageContextTest, AddFinishedBlob_LargeOffset) {
// A value which does not fit in a 4-byte data type. Used to confirm that
// large values are supported on 32-bit Chromium builds. Regression test for:
// crbug.com/458122.
const uint64_t kLargeSize = std::numeric_limits<uint64_t>::max();
const uint64_t kBlobLength = 5;
const std::string kId1("id1");
const std::string kId2("id2");
base::MessageLoop fake_io_message_loop;
BlobDataBuilder builder1(kId1);
builder1.AppendFileSystemFile(GURL(), 0, kLargeSize, base::Time::Now());
BlobDataBuilder builder2(kId2);
builder2.AppendBlob(kId1, kLargeSize - kBlobLength, kBlobLength);
BlobStorageContext context;
scoped_ptr<BlobDataHandle> blob_data_handle1 =
context.AddFinishedBlob(&builder1);
scoped_ptr<BlobDataHandle> blob_data_handle2 =
context.AddFinishedBlob(&builder2);
ASSERT_TRUE(blob_data_handle1);
ASSERT_TRUE(blob_data_handle2);
scoped_ptr<BlobDataSnapshot> data = blob_data_handle2->CreateSnapshot();
ASSERT_EQ(1u, data->items().size());
const scoped_refptr<BlobDataItem> item = data->items()[0];
EXPECT_EQ(kLargeSize - kBlobLength, item->offset());
EXPECT_EQ(kBlobLength, item->length());
blob_data_handle1.reset();
blob_data_handle2.reset();
base::RunLoop().RunUntilIdle();
}
TEST(BlobStorageContextTest, CompoundBlobs) {
......@@ -276,7 +313,7 @@ TEST(BlobStorageContextTest, CompoundBlobs) {
EXPECT_EQ(*data, canonicalized_blob_data2);
blob_data_handle.reset();
RunEventLoopTillIdle();
base::RunLoop().RunUntilIdle();
}
TEST(BlobStorageContextTest, PublicBlobUrls) {
......@@ -297,7 +334,7 @@ TEST(BlobStorageContextTest, PublicBlobUrls) {
EXPECT_EQ(kId, blob_data_handle->uuid());
scoped_ptr<BlobDataSnapshot> data = blob_data_handle->CreateSnapshot();
blob_data_handle.reset();
RunEventLoopTillIdle();
base::RunLoop().RunUntilIdle();
// The url registration should keep the blob alive even after
// explicit references are dropped.
......@@ -305,7 +342,7 @@ TEST(BlobStorageContextTest, PublicBlobUrls) {
blob_data_handle = context.GetBlobDataFromPublicURL(kUrl);
EXPECT_TRUE(blob_data_handle);
blob_data_handle.reset();
RunEventLoopTillIdle();
base::RunLoop().RunUntilIdle();
// Finally get rid of the url registration and the blob.
EXPECT_TRUE(host.RevokePublicBlobURL(kUrl));
......
......@@ -375,8 +375,8 @@ bool BlobStorageContext::AppendAllocatedBlobItem(
bool BlobStorageContext::AppendBlob(
const std::string& target_blob_uuid,
const InternalBlobData& blob,
size_t offset,
size_t length,
uint64_t offset,
uint64_t length,
InternalBlobData::Builder* target_blob_builder) {
DCHECK(length > 0);
......@@ -395,10 +395,10 @@ bool BlobStorageContext::AppendBlob(
for (; iter != items.end() && length > 0; ++iter) {
scoped_refptr<ShareableBlobDataItem> shareable_item = iter->get();
const BlobDataItem& item = *(shareable_item->item());
size_t item_length = item.length();
uint64_t item_length = item.length();
DCHECK_GT(item_length, offset);
size_t current_length = item_length - offset;
size_t new_length = current_length > length ? length : current_length;
uint64_t current_length = item_length - offset;
uint64_t new_length = current_length > length ? length : current_length;
bool reusing_blob_item = offset == 0 && new_length == item.length();
UMA_HISTOGRAM_BOOLEAN("Storage.Blob.ReusedItem", reusing_blob_item);
......
......@@ -122,8 +122,8 @@ class STORAGE_EXPORT BlobStorageContext
// have to split an item.
bool AppendBlob(const std::string& target_blob_uuid,
const InternalBlobData& blob,
size_t offset,
size_t length,
uint64_t offset,
uint64_t length,
InternalBlobData::Builder* target_blob_data);
bool IsInUse(const std::string& uuid);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment