Skip to content
This repository was archived by the owner on Apr 3, 2020. It is now read-only.

Commit c6d8e74

Browse files
rohitraoCommit bot
rohitrao
authored and
Commit bot
committed
Rewrites SerializedNavigationEntry to not have any //content member variables.
This will pave the way for a future CL that completely removes all //content dependencies from the class. [email protected] BUG=371476 TEST=No visible impact. Review URL: https://codereview.chromium.org/658073004 Cr-Commit-Position: refs/heads/master@{#300344}
1 parent f55539c commit c6d8e74

10 files changed

+124
-86
lines changed

chrome/browser/sessions/session_restore.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
#include "content/public/browser/session_storage_namespace.h"
4949
#include "content/public/browser/storage_partition.h"
5050
#include "content/public/browser/web_contents.h"
51+
#include "content/public/common/page_state.h"
5152
#include "extensions/browser/extension_registry.h"
5253
#include "extensions/common/extension_set.h"
5354
#include "net/base/network_change_notifier.h"
@@ -1053,7 +1054,8 @@ class SessionRestoreImpl : public content::NotificationObserver {
10531054
// Set up the file access rights for the selected navigation entry.
10541055
const int id = web_contents->GetRenderProcessHost()->GetID();
10551056
const content::PageState& page_state =
1056-
tab.navigations.at(selected_index).page_state();
1057+
content::PageState::CreateFromEncodedData(
1058+
tab.navigations.at(selected_index).encoded_page_state());
10571059
const std::vector<base::FilePath>& file_paths =
10581060
page_state.GetReferencedFiles();
10591061
for (std::vector<base::FilePath>::const_iterator file = file_paths.begin();

chrome/browser/sessions/session_service_unittest.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -822,7 +822,8 @@ TEST_F(SessionServiceTest, RemovePostDataWithPasswords) {
822822

823823
// Expected: the HTTP body was removed from the page state of the POST
824824
// navigation with passwords.
825-
EXPECT_NE(page_state, windows[0]->tabs[0]->navigations[0].page_state());
825+
EXPECT_NE(page_state.ToEncodedData(),
826+
windows[0]->tabs[0]->navigations[0].encoded_page_state());
826827
}
827828

828829
// This test is only applicable to chromeos.

chrome/browser/sessions/session_types_unittest.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ TEST(SessionTab, FromSyncData) {
6464
ASSERT_EQ(5u, tab.navigations.size());
6565
for (int i = 0; i < 5; ++i) {
6666
EXPECT_EQ(i, tab.navigations[i].index());
67-
EXPECT_EQ(GURL("referrer"), tab.navigations[i].referrer().url);
67+
EXPECT_EQ(GURL("referrer"), tab.navigations[i].referrer_url());
6868
EXPECT_EQ(base::ASCIIToUTF16("title"),tab.navigations[i].title());
6969
EXPECT_EQ(ui::PAGE_TRANSITION_TYPED,
7070
tab.navigations[i].transition_type());

chrome/browser/sync/glue/session_sync_test_helper.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ void SessionSyncTestHelper::VerifySyncedSession(
8282
ASSERT_EQ(kAppId, tab->extension_app_id);
8383
ASSERT_EQ(1U, tab->navigations.size());
8484
ASSERT_EQ(tab->navigations[0].virtual_url(), GURL(kVirtualUrl));
85-
ASSERT_EQ(tab->navigations[0].referrer().url, GURL(kReferrer));
85+
ASSERT_EQ(tab->navigations[0].referrer_url(), GURL(kReferrer));
8686
ASSERT_EQ(tab->navigations[0].title(),
8787
base::ASCIIToUTF16(kTitle));
8888
ASSERT_EQ(tab->navigations[0].transition_type(),

chrome/browser/sync/test/integration/sessions_helper.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -289,11 +289,11 @@ bool NavigationEquals(const sessions::SerializedNavigationEntry& expected,
289289
<< ", actual " << actual.virtual_url();
290290
return false;
291291
}
292-
if (expected.referrer().url != actual.referrer().url) {
292+
if (expected.referrer_url() != actual.referrer_url()) {
293293
LOG(ERROR) << "Expected referrer "
294-
<< expected.referrer().url
294+
<< expected.referrer_url()
295295
<< ", actual "
296-
<< actual.referrer().url;
296+
<< actual.referrer_url();
297297
return false;
298298
}
299299
if (expected.title() != actual.title()) {

chrome/browser/ui/browser_commands.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
#include "content/public/browser/render_widget_host_view.h"
7171
#include "content/public/browser/user_metrics.h"
7272
#include "content/public/browser/web_contents.h"
73+
#include "content/public/common/page_state.h"
7374
#include "content/public/common/renderer_preferences.h"
7475
#include "content/public/common/url_constants.h"
7576
#include "content/public/common/url_utils.h"

components/sessions/serialized_navigation_entry.cc

Lines changed: 61 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
#include "content/public/browser/favicon_status.h"
1010
#include "content/public/browser/navigation_controller.h"
1111
#include "content/public/browser/navigation_entry.h"
12+
#include "content/public/common/page_state.h"
13+
#include "content/public/common/referrer.h"
1214
#include "sync/protocol/session_specifics.pb.h"
1315
#include "sync/util/time.h"
1416
#include "third_party/WebKit/public/platform/WebReferrerPolicy.h"
@@ -28,7 +30,9 @@ SerializedNavigationEntry::SerializedNavigationEntry()
2830
is_overriding_user_agent_(false),
2931
http_status_code_(0),
3032
is_restored_(false),
31-
blocked_state_(STATE_INVALID) {}
33+
blocked_state_(STATE_INVALID) {
34+
referrer_policy_ = GetDefaultReferrerPolicy();
35+
}
3236

3337
SerializedNavigationEntry::~SerializedNavigationEntry() {}
3438

@@ -39,10 +43,11 @@ SerializedNavigationEntry SerializedNavigationEntry::FromNavigationEntry(
3943
SerializedNavigationEntry navigation;
4044
navigation.index_ = index;
4145
navigation.unique_id_ = entry.GetUniqueID();
42-
navigation.referrer_ = entry.GetReferrer();
46+
navigation.referrer_url_ = entry.GetReferrer().url;
47+
navigation.referrer_policy_ = entry.GetReferrer().policy;
4348
navigation.virtual_url_ = entry.GetVirtualURL();
4449
navigation.title_ = entry.GetTitle();
45-
navigation.page_state_ = entry.GetPageState();
50+
navigation.encoded_page_state_ = entry.GetPageState().ToEncodedData();
4651
navigation.transition_type_ = entry.GetTransitionType();
4752
navigation.has_post_data_ = entry.GetHasPostData();
4853
navigation.post_id_ = entry.GetPostID();
@@ -68,13 +73,11 @@ SerializedNavigationEntry SerializedNavigationEntry::FromSyncData(
6873
SerializedNavigationEntry navigation;
6974
navigation.index_ = index;
7075
navigation.unique_id_ = sync_data.unique_id();
71-
navigation.referrer_ = content::Referrer(
72-
GURL(sync_data.referrer()),
73-
static_cast<blink::WebReferrerPolicy>(sync_data.referrer_policy()));
76+
navigation.referrer_url_ = GURL(sync_data.referrer());
77+
navigation.referrer_policy_ = sync_data.referrer_policy();
7478
navigation.virtual_url_ = GURL(sync_data.virtual_url());
7579
navigation.title_ = base::UTF8ToUTF16(sync_data.title());
76-
navigation.page_state_ =
77-
content::PageState::CreateFromEncodedData(sync_data.state());
80+
navigation.encoded_page_state_ = sync_data.state();
7881

7982
uint32 transition = 0;
8083
if (sync_data.has_page_transition()) {
@@ -210,13 +213,14 @@ enum TypeMask {
210213
// index_
211214
// virtual_url_
212215
// title_
213-
// page_state_
216+
// encoded_page_state_
214217
// transition_type_
215218
//
216219
// Added on later:
217220
//
218221
// type_mask (has_post_data_)
219-
// referrer_
222+
// referrer_url_
223+
// referrer_policy_
220224
// original_request_url_
221225
// is_overriding_user_agent_
222226
// timestamp_
@@ -234,12 +238,8 @@ void SerializedNavigationEntry::WriteToPickle(int max_size,
234238

235239
WriteString16ToPickle(pickle, &bytes_written, max_size, title_);
236240

237-
content::PageState page_state = page_state_;
238-
if (has_post_data_)
239-
page_state = page_state.RemovePasswordData();
240-
241-
WriteStringToPickle(pickle, &bytes_written, max_size,
242-
page_state.ToEncodedData());
241+
const std::string encoded_page_state = GetSanitizedPageStateForPickle();
242+
WriteStringToPickle(pickle, &bytes_written, max_size, encoded_page_state);
243243

244244
pickle->WriteInt(transition_type_);
245245

@@ -248,9 +248,9 @@ void SerializedNavigationEntry::WriteToPickle(int max_size,
248248

249249
WriteStringToPickle(
250250
pickle, &bytes_written, max_size,
251-
referrer_.url.is_valid() ? referrer_.url.spec() : std::string());
251+
referrer_url_.is_valid() ? referrer_url_.spec() : std::string());
252252

253-
pickle->WriteInt(referrer_.policy);
253+
pickle->WriteInt(referrer_policy_);
254254

255255
// Save info required to override the user agent.
256256
WriteStringToPickle(
@@ -267,16 +267,15 @@ void SerializedNavigationEntry::WriteToPickle(int max_size,
267267

268268
bool SerializedNavigationEntry::ReadFromPickle(PickleIterator* iterator) {
269269
*this = SerializedNavigationEntry();
270-
std::string virtual_url_spec, page_state_data;
270+
std::string virtual_url_spec;
271271
int transition_type_int = 0;
272272
if (!iterator->ReadInt(&index_) ||
273273
!iterator->ReadString(&virtual_url_spec) ||
274274
!iterator->ReadString16(&title_) ||
275-
!iterator->ReadString(&page_state_data) ||
275+
!iterator->ReadString(&encoded_page_state_) ||
276276
!iterator->ReadInt(&transition_type_int))
277277
return false;
278278
virtual_url_ = GURL(virtual_url_spec);
279-
page_state_ = content::PageState::CreateFromEncodedData(page_state_data);
280279
transition_type_ = ui::PageTransitionFromInt(transition_type_int);
281280

282281
// type_mask did not always exist in the written stream. As such, we
@@ -291,15 +290,12 @@ bool SerializedNavigationEntry::ReadFromPickle(PickleIterator* iterator) {
291290
std::string referrer_spec;
292291
if (!iterator->ReadString(&referrer_spec))
293292
referrer_spec = std::string();
293+
referrer_url_ = GURL(referrer_spec);
294+
294295
// The "referrer policy" property was added even later, so we fall back to
295296
// the default policy if the property is not present.
296-
int policy_int;
297-
blink::WebReferrerPolicy policy;
298-
if (iterator->ReadInt(&policy_int))
299-
policy = static_cast<blink::WebReferrerPolicy>(policy_int);
300-
else
301-
policy = blink::WebReferrerPolicyDefault;
302-
referrer_ = content::Referrer(GURL(referrer_spec), policy);
297+
if (!iterator->ReadInt(&referrer_policy_))
298+
referrer_policy_ = GetDefaultReferrerPolicy();
303299

304300
// If the original URL can't be found, leave it empty.
305301
std::string original_request_url_spec;
@@ -339,7 +335,9 @@ scoped_ptr<NavigationEntry> SerializedNavigationEntry::ToNavigationEntry(
339335
scoped_ptr<NavigationEntry> entry(
340336
content::NavigationController::CreateNavigationEntry(
341337
virtual_url_,
342-
referrer_,
338+
content::Referrer(
339+
referrer_url_,
340+
static_cast<blink::WebReferrerPolicy>(referrer_policy_)),
343341
// Use a transition type of reload so that we don't incorrectly
344342
// increase the typed count.
345343
ui::PAGE_TRANSITION_RELOAD,
@@ -349,7 +347,8 @@ scoped_ptr<NavigationEntry> SerializedNavigationEntry::ToNavigationEntry(
349347
browser_context));
350348

351349
entry->SetTitle(title_);
352-
entry->SetPageState(page_state_);
350+
entry->SetPageState(
351+
content::PageState::CreateFromEncodedData(encoded_page_state_));
353352
entry->SetPageID(page_id);
354353
entry->SetHasPostData(has_post_data_);
355354
entry->SetPostID(post_id_);
@@ -372,8 +371,8 @@ scoped_ptr<NavigationEntry> SerializedNavigationEntry::ToNavigationEntry(
372371
sync_pb::TabNavigation SerializedNavigationEntry::ToSyncData() const {
373372
sync_pb::TabNavigation sync_data;
374373
sync_data.set_virtual_url(virtual_url_.spec());
375-
sync_data.set_referrer(referrer_.url.spec());
376-
sync_data.set_referrer_policy(referrer_.policy);
374+
sync_data.set_referrer(referrer_url_.spec());
375+
sync_data.set_referrer_policy(referrer_policy_);
377376
sync_data.set_title(base::UTF16ToUTF8(title_));
378377

379378
// Page transition core.
@@ -509,16 +508,42 @@ std::vector<NavigationEntry*> SerializedNavigationEntry::ToNavigationEntries(
509508
return entries;
510509
}
511510

511+
// TODO(rohitrao): Move this content-specific code into a
512+
// SerializedNavigationEntryHelper class.
513+
int SerializedNavigationEntry::GetDefaultReferrerPolicy() const {
514+
return blink::WebReferrerPolicyDefault;
515+
}
516+
517+
// TODO(rohitrao): Move this content-specific code into a
518+
// SerializedNavigationEntryHelper class.
519+
std::string SerializedNavigationEntry::GetSanitizedPageStateForPickle() const {
520+
content::PageState page_state =
521+
content::PageState::CreateFromEncodedData(encoded_page_state_);
522+
if (has_post_data_)
523+
page_state = page_state.RemovePasswordData();
524+
525+
return page_state.ToEncodedData();
526+
}
527+
528+
// TODO(rohitrao): Move this content-specific code into a
529+
// SerializedNavigationEntryHelper class.
512530
void SerializedNavigationEntry::Sanitize() {
531+
content::Referrer old_referrer(
532+
referrer_url_,
533+
static_cast<blink::WebReferrerPolicy>(referrer_policy_));
513534
content::Referrer new_referrer =
514-
content::Referrer::SanitizeForRequest(virtual_url_, referrer_);
535+
content::Referrer::SanitizeForRequest(virtual_url_, old_referrer);
515536

516537
// No need to compare the policy, as it doesn't change during
517538
// sanitization. If there has been a change, the referrer needs to be
518539
// stripped from the page state as well.
519-
if (referrer_.url != new_referrer.url) {
520-
referrer_ = content::Referrer();
521-
page_state_ = page_state_.RemoveReferrer();
540+
if (referrer_url_ != new_referrer.url) {
541+
referrer_url_ = GURL();
542+
referrer_policy_ = GetDefaultReferrerPolicy();
543+
encoded_page_state_ =
544+
content::PageState::CreateFromEncodedData(encoded_page_state_)
545+
.RemoveReferrer()
546+
.ToEncodedData();
522547
}
523548
}
524549

components/sessions/serialized_navigation_entry.h

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414
#include "base/strings/string16.h"
1515
#include "base/time/time.h"
1616
#include "components/sessions/sessions_export.h"
17-
#include "content/public/common/page_state.h"
18-
#include "content/public/common/referrer.h"
1917
#include "ui/base/page_transition_types.h"
2018
#include "url/gurl.h"
2119

@@ -97,11 +95,12 @@ class SESSIONS_EXPORT SerializedNavigationEntry {
9795
int unique_id() const { return unique_id_; }
9896
const GURL& virtual_url() const { return virtual_url_; }
9997
const base::string16& title() const { return title_; }
100-
const content::PageState& page_state() const { return page_state_; }
98+
const std::string& encoded_page_state() const { return encoded_page_state_; }
10199
const base::string16& search_terms() const { return search_terms_; }
102100
const GURL& favicon_url() const { return favicon_url_; }
103101
int http_status_code() const { return http_status_code_; }
104-
const content::Referrer& referrer() const { return referrer_; }
102+
const GURL& referrer_url() const { return referrer_url_; }
103+
int referrer_policy() const { return referrer_policy_; }
105104
ui::PageTransition transition_type() const {
106105
return transition_type_;
107106
}
@@ -134,6 +133,13 @@ class SESSIONS_EXPORT SerializedNavigationEntry {
134133
private:
135134
friend class SerializedNavigationEntryTestHelper;
136135

136+
// Returns the default referrer policy.
137+
int GetDefaultReferrerPolicy() const;
138+
139+
// Returns a sanitized version of |encoded_page_state_| suitable for writing
140+
// to disk.
141+
std::string GetSanitizedPageStateForPickle() const;
142+
137143
// Sanitizes the data in this class to be more robust against faulty data
138144
// written by older versions.
139145
void Sanitize();
@@ -143,10 +149,11 @@ class SESSIONS_EXPORT SerializedNavigationEntry {
143149

144150
// Member variables corresponding to NavigationEntry fields.
145151
int unique_id_;
146-
content::Referrer referrer_;
152+
GURL referrer_url_;
153+
int referrer_policy_;
147154
GURL virtual_url_;
148155
base::string16 title_;
149-
content::PageState page_state_;
156+
std::string encoded_page_state_;
150157
ui::PageTransition transition_type_;
151158
bool has_post_data_;
152159
int64 post_id_;

components/sessions/serialized_navigation_entry_test_helper.cc

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include "base/strings/utf_string_conversions.h"
88
#include "base/time/time.h"
99
#include "components/sessions/serialized_navigation_entry.h"
10+
#include "content/public/common/page_state.h"
1011
#include "testing/gtest/include/gtest/gtest.h"
1112
#include "third_party/WebKit/public/platform/WebReferrerPolicy.h"
1213
#include "url/gurl.h"
@@ -17,11 +18,11 @@ namespace sessions {
1718
void SerializedNavigationEntryTestHelper::ExpectNavigationEquals(
1819
const SerializedNavigationEntry& expected,
1920
const SerializedNavigationEntry& actual) {
20-
EXPECT_EQ(expected.referrer_.url, actual.referrer_.url);
21-
EXPECT_EQ(expected.referrer_.policy, actual.referrer_.policy);
21+
EXPECT_EQ(expected.referrer_url_, actual.referrer_url_);
22+
EXPECT_EQ(expected.referrer_policy_, actual.referrer_policy_);
2223
EXPECT_EQ(expected.virtual_url_, actual.virtual_url_);
2324
EXPECT_EQ(expected.title_, actual.title_);
24-
EXPECT_EQ(expected.page_state_, actual.page_state_);
25+
EXPECT_EQ(expected.encoded_page_state_, actual.encoded_page_state_);
2526
EXPECT_EQ(expected.transition_type_, actual.transition_type_);
2627
EXPECT_EQ(expected.has_post_data_, actual.has_post_data_);
2728
EXPECT_EQ(expected.original_request_url_, actual.original_request_url_);
@@ -35,13 +36,11 @@ SerializedNavigationEntry SerializedNavigationEntryTestHelper::CreateNavigation(
3536
const std::string& title) {
3637
SerializedNavigationEntry navigation;
3738
navigation.index_ = 0;
38-
navigation.referrer_ =
39-
content::Referrer(GURL("http://www.referrer.com"),
40-
blink::WebReferrerPolicyDefault);
39+
navigation.referrer_url_ = GURL("http://www.referrer.com");
40+
navigation.referrer_policy_ = blink::WebReferrerPolicyDefault;
4141
navigation.virtual_url_ = GURL(virtual_url);
4242
navigation.title_ = base::UTF8ToUTF16(title);
43-
navigation.page_state_ =
44-
content::PageState::CreateFromEncodedData("fake_state");
43+
navigation.encoded_page_state_ = "fake state";
4544
navigation.timestamp_ = base::Time::Now();
4645
navigation.http_status_code_ = 200;
4746
return navigation;
@@ -51,7 +50,7 @@ SerializedNavigationEntry SerializedNavigationEntryTestHelper::CreateNavigation(
5150
void SerializedNavigationEntryTestHelper::SetPageState(
5251
const content::PageState& page_state,
5352
SerializedNavigationEntry* navigation) {
54-
navigation->page_state_ = page_state;
53+
navigation->encoded_page_state_ = page_state.ToEncodedData();
5554
}
5655

5756
// static

0 commit comments

Comments
 (0)