Commit fd08c4c4 authored by David Jean's avatar David Jean Committed by Chromium LUCI CQ

[ios] Add limiting to 63k URLS to session experiment

Bug: 1140990
Change-Id: I1ed3c06836d5dcedb40f8f9dde6c60f6ecffc671
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2594908
Commit-Queue: David Jean <djean@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838627}
parent f6e88454
......@@ -16,6 +16,9 @@ class NavigationItemImpl;
// Class that can serialize and deserialize NavigationItems.
class NavigationItemStorageBuilder {
public:
// Returns approximate sizes of the given |navigation_item| without building
// storage. Only string sizes are added.
int ItemStoredSize(NavigationItemImpl* navigation_item) const;
// Creates a serialized NavigationItem from |navigation_Item|.
CRWNavigationItemStorage* BuildStorage(
NavigationItemImpl* navigation_item) const;
......
......@@ -15,6 +15,21 @@
namespace web {
int NavigationItemStorageBuilder::ItemStoredSize(
NavigationItemImpl* navigation_item) const {
DCHECK(navigation_item);
int size = 0;
size += navigation_item->virtual_url_.spec().size();
size += navigation_item->url_.spec().size();
size += navigation_item->referrer_.url.spec().size();
size += navigation_item->title_.size();
for (NSString* key in navigation_item->http_request_headers_) {
NSString* value = navigation_item->http_request_headers_[key];
size += key.length + value.length;
}
return size;
}
CRWNavigationItemStorage* NavigationItemStorageBuilder::BuildStorage(
NavigationItemImpl* navigation_item) const {
DCHECK(navigation_item);
......
......@@ -11,6 +11,9 @@ namespace web {
class WebStateImpl;
// Allow navigation items up to ~63k (like components/sessions/core)
const int kMaxNavigationItemSize = 63 * 1024;
// Class that can serialize and deserialize session information.
class SessionStorageBuilder {
public:
......
......@@ -45,6 +45,7 @@ CRWSessionStorage* SessionStorageBuilder::BuildStorage(
NSMutableArray* item_storages = [[NSMutableArray alloc] init];
NavigationItemStorageBuilder item_storage_builder;
size_t originalIndex = session_storage.lastCommittedItemIndex;
// Drop URLs larger than a certain threshold.
for (size_t index = 0;
index < static_cast<size_t>(navigation_manager->GetItemCount());
++index) {
......@@ -57,6 +58,18 @@ CRWSessionStorage* SessionStorageBuilder::BuildStorage(
}
continue;
}
if (base::FeatureList::IsEnabled(features::kReduceSessionSize)) {
// Go through the builder who's a friend of web::NavigationItemImpl
// and has access to raw fields, so for example url is not
// counted twice if virtyual url is empty.
if (item_storage_builder.ItemStoredSize(item) > kMaxNavigationItemSize) {
if (index <= originalIndex) {
session_storage.lastCommittedItemIndex--;
}
continue;
}
}
[item_storages addObject:item_storage_builder.BuildStorage(item)];
}
......
......@@ -5,6 +5,8 @@
#import "ios/web/navigation/session_storage_builder.h"
#include "base/strings/sys_string_conversions.h"
#include "base/test/scoped_feature_list.h"
#include "ios/web/common/features.h"
#import "ios/web/navigation/wk_navigation_util.h"
#import "ios/web/public/session/crw_navigation_item_storage.h"
#import "ios/web/public/session/crw_session_storage.h"
......@@ -47,6 +49,51 @@ class WebStateWithMockProxy : public WebStateImpl {
using SessionStorageBuilderTest = WebTest;
void SetNavigationItemSizedStrings(WebStateWithMockProxy& web_state,
int index,
int url_length,
int virtual_url_length,
int title_length,
int referrer_url_length) {
NavigationItemImpl* item =
web_state.GetNavigationManagerImpl().GetNavigationItemImplAtIndex(index);
if (url_length) {
NSString* url = [@"https://" stringByPaddingToLength:url_length
withString:@"a"
startingAtIndex:0];
item->SetURL(GURL(base::SysNSStringToUTF8(url)));
} else {
item->SetURL(GURL());
}
if (virtual_url_length) {
NSString* virtual_url =
[@"https://" stringByPaddingToLength:virtual_url_length
withString:@"b"
startingAtIndex:0];
item->SetVirtualURL(GURL(base::SysNSStringToUTF8(virtual_url)));
} else {
item->SetVirtualURL(GURL());
}
if (title_length) {
NSString* title = [@"" stringByPaddingToLength:title_length
withString:@"c"
startingAtIndex:0];
item->SetTitle(base::SysNSStringToUTF16(title));
} else {
item->SetTitle(base::SysNSStringToUTF16(@""));
}
Referrer referrer;
if (referrer_url_length) {
NSString* referrer_url =
[@"https://" stringByPaddingToLength:referrer_url_length
withString:@"d"
startingAtIndex:0];
referrer.url = GURL(base::SysNSStringToUTF8(referrer_url));
}
item->SetReferrer(referrer);
}
// Tests building storage for session that is longer than kMaxSessionSize with
// last committed item at the end of the session.
TEST_F(SessionStorageBuilderTest, BuildStorageForExtraLongSession) {
......@@ -170,4 +217,87 @@ TEST_F(SessionStorageBuilderTest, SkipLongUrls) {
EXPECT_EQ(GURL::EmptyGURL(), [storage.itemStorages.firstObject referrer].url);
}
// Tests building storage for session that has items longer than
// web::kMaxNavigationItemSize.
TEST_F(SessionStorageBuilderTest, SkipItemOverMaxSize) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(features::kReduceSessionSize);
// Create WebState with navigation item count that are below maximum size.
WebState::CreateParams params(GetBrowserState());
WebStateWithMockProxy web_state(params);
NSString* normal_url = @"https://foo.test";
NSString* const current_url = normal_url;
[web_state.fake_wk_list() setCurrentURL:normal_url
backListURLs:@[ normal_url ]
forwardListURLs:nil];
OCMStub([web_state.mock_web_view() URL])
.andReturn([NSURL URLWithString:current_url]);
NavigationManager* navigation_manager = web_state.GetNavigationManager();
ASSERT_EQ(2, navigation_manager->GetItemCount());
SessionStorageBuilder builder;
CRWSessionStorage* storage = builder.BuildStorage(&web_state);
ASSERT_TRUE(storage);
ASSERT_EQ(2U, storage.itemStorages.count);
SetNavigationItemSizedStrings(web_state, 0, web::kMaxNavigationItemSize - 1,
0, 0, 0);
storage = builder.BuildStorage(&web_state);
ASSERT_TRUE(storage);
ASSERT_EQ(2U, storage.itemStorages.count);
SetNavigationItemSizedStrings(web_state, 0, web::kMaxNavigationItemSize, 0, 0,
0);
storage = builder.BuildStorage(&web_state);
ASSERT_TRUE(storage);
ASSERT_EQ(1U, storage.itemStorages.count);
SetNavigationItemSizedStrings(
web_state, 0, web::kMaxNavigationItemSize - 1000 - 1, 1000 - 1, 0, 0);
storage = builder.BuildStorage(&web_state);
ASSERT_TRUE(storage);
ASSERT_EQ(2U, storage.itemStorages.count);
SetNavigationItemSizedStrings(web_state, 0,
web::kMaxNavigationItemSize - 1000, 1000, 0, 0);
storage = builder.BuildStorage(&web_state);
ASSERT_TRUE(storage);
ASSERT_EQ(1U, storage.itemStorages.count);
SetNavigationItemSizedStrings(web_state, 0,
web::kMaxNavigationItemSize - 1040 - 1,
1000 - 1, 40 - 1, 0);
storage = builder.BuildStorage(&web_state);
ASSERT_TRUE(storage);
ASSERT_EQ(2U, storage.itemStorages.count);
SetNavigationItemSizedStrings(
web_state, 0, web::kMaxNavigationItemSize - 1040, 1000, 40, 0);
storage = builder.BuildStorage(&web_state);
ASSERT_TRUE(storage);
ASSERT_EQ(1U, storage.itemStorages.count);
SetNavigationItemSizedStrings(web_state, 0,
web::kMaxNavigationItemSize - 2040 - 1,
1000 - 1, 40 - 1, 1000 - 1);
storage = builder.BuildStorage(&web_state);
ASSERT_TRUE(storage);
ASSERT_EQ(2U, storage.itemStorages.count);
SetNavigationItemSizedStrings(
web_state, 0, web::kMaxNavigationItemSize - 2040, 1000, 40, 1000);
storage = builder.BuildStorage(&web_state);
ASSERT_TRUE(storage);
ASSERT_EQ(1U, storage.itemStorages.count);
}
} // namespace web
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