Skip to content

Commit

Permalink
Add Mojo IPC for seeding new Renderer with Browser's cached blob state.
Browse files Browse the repository at this point in the history
Cache key state is currently stored in the renderer and is used
by the image processor to determine which images should be transcoded
and sent to the client, and which images do not need to be sent,
reducing latency and compute resources. A drawback of storing this
state in the renderer is that the cache state is dumped when the
renderer is torn down, as in the case when the user navigates
to another site.

This patch provides the renderer with a snapshot of the browser's
cache state, which means that the knowledge of cache state is
retained when the renderer of a given site is torn down
and brought back again.

* Add GetCacheState() to BlobCache
* Add GetCacheState() to BlobChannelSender()
* Add GetCacheState() to .mojom file.

R=nyquist@chromium.org,wez@chromium.org
BUG=600719

Review-Url: https://codereview.chromium.org/2056993003
Cr-Commit-Position: refs/heads/master@{#406658}
  • Loading branch information
kmarshall authored and Commit bot committed Jul 20, 2016
1 parent 96e149c commit 2cfb64f
Show file tree
Hide file tree
Showing 17 changed files with 175 additions and 19 deletions.
1 change: 0 additions & 1 deletion blimp/OWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@ dtrainor@chromium.org
kmarshall@chromium.org
nyquist@chromium.org
wez@chromium.org
haibinlu@chromium.org

3 changes: 3 additions & 0 deletions blimp/common/blob_cache/blob_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ class BLIMP_COMMON_EXPORT BlobCache {
public:
virtual ~BlobCache() {}

// Gets the keys of items stored by the cache.
virtual std::vector<BlobId> GetCachedBlobIds() const = 0;

// Returns true if there is a cache item |id|.
virtual bool Contains(const BlobId& id) const = 0;

Expand Down
10 changes: 10 additions & 0 deletions blimp/common/blob_cache/in_memory_blob_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "blimp/common/blob_cache/in_memory_blob_cache.h"

#include <utility>

#include "base/logging.h"

namespace blimp {
Expand All @@ -12,6 +14,14 @@ InMemoryBlobCache::InMemoryBlobCache() {}

InMemoryBlobCache::~InMemoryBlobCache() {}

std::vector<BlobId> InMemoryBlobCache::GetCachedBlobIds() const {
std::vector<BlobId> cached_ids;
for (const auto& blob_id_and_data_pair : cache_) {
cached_ids.push_back(blob_id_and_data_pair.first);
}
return cached_ids;
}

void InMemoryBlobCache::Put(const BlobId& id, BlobDataPtr data) {
if (Contains(id)) {
// In cases where the engine has miscalculated what is already available
Expand Down
2 changes: 2 additions & 0 deletions blimp/common/blob_cache/in_memory_blob_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <map>
#include <string>
#include <vector>

#include "base/macros.h"
#include "base/memory/ref_counted.h"
Expand All @@ -23,6 +24,7 @@ class BLIMP_COMMON_EXPORT InMemoryBlobCache : public BlobCache {
~InMemoryBlobCache() override;

// BlobCache implementation.
std::vector<BlobId> GetCachedBlobIds() const override;
bool Contains(const BlobId& id) const override;
void Put(const BlobId& id, BlobDataPtr data) override;
BlobDataPtr Get(const BlobId& id) const override;
Expand Down
21 changes: 17 additions & 4 deletions blimp/common/blob_cache/in_memory_blob_cache_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,25 @@ class InMemoryBlobCacheTest : public testing::Test {
TEST_F(InMemoryBlobCacheTest, SimplePutContainsAndGetOperations) {
EXPECT_FALSE(cache_.Contains(kFoo));
EXPECT_FALSE(cache_.Get(kFoo));
EXPECT_FALSE(cache_.Contains(kBar));
EXPECT_FALSE(cache_.Get(kBar));

BlobDataPtr blob_data = CreateBlobDataPtr(kDeadbeef);
cache_.Put(kFoo, blob_data);
BlobDataPtr blob_data_1 = CreateBlobDataPtr(kDeadbeef);
cache_.Put(kFoo, blob_data_1);

EXPECT_TRUE(cache_.Contains(kFoo));
EXPECT_FALSE(cache_.Contains(kBar));

BlobDataPtr out = cache_.Get(kFoo);
BlobDataPtr blob_data_2 = CreateBlobDataPtr(kDeadbeef);
cache_.Put(kBar, blob_data_2);

EXPECT_EQ(blob_data_1, cache_.Get(kFoo));
EXPECT_EQ(blob_data_2, cache_.Get(kBar));

EXPECT_EQ(blob_data, out);
auto cache_state = cache_.GetCachedBlobIds();
EXPECT_EQ(2u, cache_state.size());
EXPECT_EQ(kBar, cache_state[0]);
EXPECT_EQ(kFoo, cache_state[1]);
}

TEST_F(InMemoryBlobCacheTest, TestDuplicatePut) {
Expand All @@ -59,6 +68,10 @@ TEST_F(InMemoryBlobCacheTest, TestDuplicatePut) {
cache_.Put(kFoo, duplicate);
BlobDataPtr out2 = cache_.Get(kFoo);
EXPECT_EQ(first, out2);

auto cache_state = cache_.GetCachedBlobIds();
EXPECT_EQ(1u, cache_state.size());
EXPECT_EQ(kFoo, cache_state[0]);
}

} // namespace
Expand Down
3 changes: 3 additions & 0 deletions blimp/common/blob_cache/mock_blob_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#ifndef BLIMP_COMMON_BLOB_CACHE_MOCK_BLOB_CACHE_H_
#define BLIMP_COMMON_BLOB_CACHE_MOCK_BLOB_CACHE_H_

#include <vector>

#include "blimp/common/blob_cache/blob_cache.h"
#include "testing/gmock/include/gmock/gmock.h"

Expand All @@ -18,6 +20,7 @@ class MockBlobCache : public BlobCache {
MOCK_CONST_METHOD1(Contains, bool(const BlobId&));
MOCK_METHOD2(Put, void(const BlobId&, BlobDataPtr));
MOCK_CONST_METHOD1(Get, BlobDataPtr(const BlobId&));
MOCK_CONST_METHOD0(GetCachedBlobIds, std::vector<BlobId>());

private:
DISALLOW_COPY_AND_ASSIGN(MockBlobCache);
Expand Down
2 changes: 0 additions & 2 deletions blimp/engine/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -364,8 +364,6 @@ mojom("blob_channel_mojo") {
sources = [
"mojo/blob_channel.mojom",
]

use_new_wrapper_types = false
}

source_set("test_support") {
Expand Down
8 changes: 8 additions & 0 deletions blimp/engine/mojo/blob_channel.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@ module blimp.engine.mojom;
// The renderer can use this service to push bulk data such as images to
// the client.
interface BlobChannel {
// Gets the list of cached BlobIDs and their replication status in the
// browser process' BlobCache. The replication status is represented
// as a boolean which, when true, indicates that the blob has been
// sent to the client.
// TODO(kmarshall): Add a delegate receiver to process cache invalidation
// events from the browser as they occur.
GetCachedBlobIds() => (map<string, bool> cache_state);

// Stores the blob |id| in the BlobCache.
// Because the IPC channel is a shared resource and payloads can be quite
// large, we use shared memory to reduce channel contention and associated
Expand Down
18 changes: 16 additions & 2 deletions blimp/engine/mojo/blob_channel_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,12 @@

#include "blimp/engine/mojo/blob_channel_service.h"

#include <string>
#include <unordered_map>
#include <utility>
#include <vector>

#include "base/memory/ptr_util.h"
#include "blimp/net/blob_channel/blob_channel_sender.h"
#include "mojo/public/cpp/system/buffer.h"

Expand All @@ -21,7 +25,17 @@ BlobChannelService::BlobChannelService(BlobChannelSender* blob_channel_sender,

BlobChannelService::~BlobChannelService() {}

void BlobChannelService::PutBlob(const mojo::String& id,
void BlobChannelService::GetCachedBlobIds(
const BlobChannelService::GetCachedBlobIdsCallback& response_callback) {
VLOG(1) << "BlobChannel::GetCachedBlobIds called.";
std::unordered_map<std::string, bool> cache_state;
for (const auto& next_entry : blob_channel_sender_->GetCachedBlobIds()) {
cache_state[next_entry.id] = next_entry.was_delivered;
}
response_callback.Run(std::move(cache_state));
}

void BlobChannelService::PutBlob(const std::string& id,
mojo::ScopedSharedBufferHandle data,
uint32_t size) {
// Map |data| into the address space and copy out its contents.
Expand All @@ -43,7 +57,7 @@ void BlobChannelService::PutBlob(const mojo::String& id,
blob_channel_sender_->PutBlob(id, std::move(new_blob));
}

void BlobChannelService::DeliverBlob(const mojo::String& id) {
void BlobChannelService::DeliverBlob(const std::string& id) {
blob_channel_sender_->DeliverBlob(id);
}

Expand Down
8 changes: 6 additions & 2 deletions blimp/engine/mojo/blob_channel_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#ifndef BLIMP_ENGINE_MOJO_BLOB_CHANNEL_SERVICE_H_
#define BLIMP_ENGINE_MOJO_BLOB_CHANNEL_SERVICE_H_

#include <string>

#include "blimp/engine/mojo/blob_channel.mojom.h"
#include "mojo/public/cpp/bindings/strong_binding.h"

Expand All @@ -31,10 +33,12 @@ class BlobChannelService : public mojom::BlobChannel {
mojom::BlobChannelRequest request);

// BlobChannel implementation.
void PutBlob(const mojo::String& id,
void GetCachedBlobIds(
const GetCachedBlobIdsCallback& response_callback) override;
void PutBlob(const std::string& id,
mojo::ScopedSharedBufferHandle data,
uint32_t size) override;
void DeliverBlob(const mojo::String& id) override;
void DeliverBlob(const std::string& id) override;

// Binds |this| and its object lifetime to a Mojo connection.
mojo::StrongBinding<mojom::BlobChannel> binding_;
Expand Down
25 changes: 23 additions & 2 deletions blimp/engine/renderer/blob_channel_sender_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "blimp/engine/renderer/blob_channel_sender_proxy.h"

#include <unordered_map>
#include <utility>

#include "blimp/common/blob_cache/id_util.h"
Expand Down Expand Up @@ -61,13 +62,17 @@ mojo::ScopedSharedBufferHandle SharedMemoryBlob::CreateRemoteHandle() {
} // namespace

BlobChannelSenderProxy::BlobChannelSenderProxy()
: BlobChannelSenderProxy(GetConnectedBlobChannel()) {}
: blob_channel_(GetConnectedBlobChannel()), weak_factory_(this) {
blob_channel_->GetCachedBlobIds(
base::Bind(&BlobChannelSenderProxy::OnGetCacheStateComplete,
weak_factory_.GetWeakPtr()));
}

BlobChannelSenderProxy::~BlobChannelSenderProxy() {}

BlobChannelSenderProxy::BlobChannelSenderProxy(
mojom::BlobChannelPtr blob_channel)
: blob_channel_(std::move(blob_channel)) {}
: blob_channel_(std::move(blob_channel)), weak_factory_(this) {}

// static
std::unique_ptr<BlobChannelSenderProxy> BlobChannelSenderProxy::CreateForTest(
Expand Down Expand Up @@ -110,5 +115,21 @@ void BlobChannelSenderProxy::DeliverBlob(const std::string& id) {
blob_channel_->DeliverBlob(id);
}

std::vector<BlobChannelSender::CacheStateEntry>
BlobChannelSenderProxy::GetCachedBlobIds() const {
NOTREACHED();
return std::vector<BlobChannelSender::CacheStateEntry>();
}

void BlobChannelSenderProxy::OnGetCacheStateComplete(
const std::unordered_map<std::string, bool>& cache_state) {
VLOG(1) << "Received cache state from browser (" << cache_state.size()
<< " items)";
replication_state_.clear();
for (const auto& next_item : cache_state) {
replication_state_[next_item.first] = next_item.second;
}
}

} // namespace engine
} // namespace blimp
18 changes: 18 additions & 0 deletions blimp/engine/renderer/blob_channel_sender_proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,14 @@
#ifndef BLIMP_ENGINE_RENDERER_BLOB_CHANNEL_SENDER_PROXY_H_
#define BLIMP_ENGINE_RENDERER_BLOB_CHANNEL_SENDER_PROXY_H_

#include <map>
#include <memory>
#include <string>
#include <unordered_map>
#include <vector>

#include "base/containers/hash_tables.h"
#include "base/memory/weak_ptr.h"
#include "blimp/common/blimp_common_export.h"
#include "blimp/engine/mojo/blob_channel.mojom.h"
#include "blimp/net/blob_channel/blob_channel_sender.h"
Expand All @@ -20,6 +24,14 @@ namespace engine {
// process.
class BLIMP_COMMON_EXPORT BlobChannelSenderProxy : public BlobChannelSender {
public:
// Asynchronously request the set of cache keys from the browser process.
// Knowledge of the browser's cache allows the renderer to avoid re-encoding
// and re-transferring image assets that are already tracked by the
// browser-side cache.
//
// If |this| is used in the brief time before the response arrives,
// the object will continue to work, but false negatives will be returned
// for IsInEngineCache() and IsInClientCache().
BlobChannelSenderProxy();

~BlobChannelSenderProxy() override;
Expand All @@ -34,10 +46,14 @@ class BLIMP_COMMON_EXPORT BlobChannelSenderProxy : public BlobChannelSender {
bool IsInClientCache(const std::string& id) const;

// BlobChannelSender implementation.
std::vector<CacheStateEntry> GetCachedBlobIds() const override;
void PutBlob(const BlobId& id, BlobDataPtr data) override;
void DeliverBlob(const BlobId& id) override;

private:
void OnGetCacheStateComplete(
const std::unordered_map<std::string, bool>& cache_state);

// Testing constructor, used to supply a BlobChannel Mojo proxy directly.
explicit BlobChannelSenderProxy(mojom::BlobChannelPtr blob_channel);

Expand All @@ -50,6 +66,8 @@ class BLIMP_COMMON_EXPORT BlobChannelSenderProxy : public BlobChannelSender {
// image encoding and transferral if the content is already in the system.
base::hash_map<std::string, bool> replication_state_;

base::WeakPtrFactory<BlobChannelSenderProxy> weak_factory_;

DISALLOW_COPY_AND_ASSIGN(BlobChannelSenderProxy);
};

Expand Down
10 changes: 10 additions & 0 deletions blimp/net/blob_channel/blob_channel_sender.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#ifndef BLIMP_NET_BLOB_CHANNEL_BLOB_CHANNEL_SENDER_H_
#define BLIMP_NET_BLOB_CHANNEL_BLOB_CHANNEL_SENDER_H_

#include <vector>

#include "blimp/common/blob_cache/blob_cache.h"
#include "blimp/net/blimp_net_export.h"

Expand All @@ -15,8 +17,16 @@ const size_t kMaxBlobSizeBytes = 10 * 1024 * 1024;

class BLIMP_NET_EXPORT BlobChannelSender {
public:
struct CacheStateEntry {
BlobId id;
bool was_delivered;
};

virtual ~BlobChannelSender() {}

// Gets the list of cache keys and their replication status in the BlobCache.
virtual std::vector<CacheStateEntry> GetCachedBlobIds() const = 0;

// Puts a blob in the local BlobChannel. The blob can then be pushed to the
// remote receiver via "DeliverBlob()".
// Does nothing if there is already a blob |id| in the channel.
Expand Down
18 changes: 18 additions & 0 deletions blimp/net/blob_channel/blob_channel_sender_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

#include "blimp/net/blob_channel/blob_channel_sender_impl.h"

#include <utility>

#include "base/stl_util.h"
#include "base/strings/string_number_conversions.h"
#include "blimp/common/blob_cache/blob_cache.h"
#include "blimp/common/blob_cache/id_util.h"
Expand All @@ -19,6 +22,21 @@ BlobChannelSenderImpl::BlobChannelSenderImpl(std::unique_ptr<BlobCache> cache,

BlobChannelSenderImpl::~BlobChannelSenderImpl() {}

std::vector<BlobChannelSender::CacheStateEntry>
BlobChannelSenderImpl::GetCachedBlobIds() const {
const auto cache_state = cache_->GetCachedBlobIds();
std::vector<CacheStateEntry> output;
output.reserve(cache_state.size());
for (const std::string& cached_id : cache_state) {
CacheStateEntry next_output;
next_output.id = cached_id;
next_output.was_delivered =
ContainsKey(receiver_cache_contents_, cached_id);
output.push_back(next_output);
}
return output;
}

void BlobChannelSenderImpl::PutBlob(const BlobId& id, BlobDataPtr data) {
DCHECK(data);
DCHECK(!id.empty());
Expand Down
3 changes: 3 additions & 0 deletions blimp/net/blob_channel/blob_channel_sender_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <memory>
#include <set>
#include <string>
#include <vector>

#include "base/macros.h"
#include "base/threading/thread_checker.h"
Expand Down Expand Up @@ -37,6 +38,8 @@ class BLIMP_NET_EXPORT BlobChannelSenderImpl : public BlobChannelSender {
~BlobChannelSenderImpl() override;

// BlobChannelSender implementation.
std::vector<BlobChannelSender::CacheStateEntry> GetCachedBlobIds()
const override;
void PutBlob(const BlobId& id, BlobDataPtr data) override;
void DeliverBlob(const BlobId& id) override;

Expand Down
Loading

0 comments on commit 2cfb64f

Please sign in to comment.