Skip to content

Commit

Permalink
[Sync] Reland r329370 (Don't create a sync node when updating an URL …
Browse files Browse the repository at this point in the history
…that wasn't typed)

Revert "Revert of Don't create a sync node when updating an URL that wasn't typed. (patchset #1 id:1 of https://codereview.chromium.org/1126633005/)"

This reverts commit ebb1803.
Original CL: https://codereview.chromium.org/1126633005

[email protected]
BUG=496855

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

Cr-Commit-Position: refs/heads/master@{#332960}
(cherry picked from commit 4414915)

[email protected]

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

Cr-Commit-Position: refs/branch-heads/2403@{crosswalk-project#268}
Cr-Branched-From: f54b809-refs/heads/master@{#330231}
  • Loading branch information
Nicolas Zea committed Jun 10, 2015
1 parent d333c70 commit 6170f24
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 1 deletion.
8 changes: 8 additions & 0 deletions chrome/browser/sync/glue/typed_url_change_processor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,14 @@ bool TypedUrlChangeProcessor::CreateOrUpdateSyncNode(
return false;
}

if (std::find_if(visit_vector.begin(), visit_vector.end(),
[](const history::VisitRow& visit) {
return ui::PageTransitionCoreTypeIs(
visit.transition, ui::PAGE_TRANSITION_TYPED);
}) == visit_vector.end())
// This URL has no TYPED visits, don't sync it.
return false;

syncer::ReadNode typed_url_root(trans);
if (typed_url_root.InitTypeRoot(syncer::TYPED_URLS) !=
syncer::BaseNode::INIT_OK) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,44 @@ IN_PROC_BROWSER_TEST_F(TwoClientTypedUrlsSyncTest, UpdateToNonTypedURL) {
ASSERT_EQ(2, GetVisitCountForFirstURL(0));
}

IN_PROC_BROWSER_TEST_F(TwoClientTypedUrlsSyncTest,
DontSyncUpdatedNonTypedURLs) {
// Checks if a non-typed URL that has been updated (modified) doesn't get
// synced. This is a regression test after fixing a bug where adding a
// non-typed URL was guarded against but later modifying it was not. Since
// "update" is "update or create if missing", non-typed URLs were being
// created.
const GURL kNonTypedURL("http://link.google.com/");
const GURL kTypedURL("http://typed.google.com/");
ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
AddUrlToHistoryWithTransition(0, kNonTypedURL, ui::PAGE_TRANSITION_LINK,
history::SOURCE_BROWSED);
AddUrlToHistoryWithTransition(0, kTypedURL, ui::PAGE_TRANSITION_TYPED,
history::SOURCE_BROWSED);

// Modify the non-typed URL. It should not get synced.
typed_urls_helper::SetPageTitle(0, kNonTypedURL, "Welcome to Non-Typed URL");
ASSERT_TRUE(AwaitCheckAllProfilesHaveSameURLsAsVerifier());

history::VisitVector visits;
// First client has both visits.
visits = typed_urls_helper::GetVisitsForURLFromClient(0, kNonTypedURL);
ASSERT_EQ(1U, visits.size());
EXPECT_TRUE(ui::PageTransitionCoreTypeIs(visits[0].transition,
ui::PAGE_TRANSITION_LINK));
visits = typed_urls_helper::GetVisitsForURLFromClient(0, kTypedURL);
ASSERT_EQ(1U, visits.size());
EXPECT_TRUE(ui::PageTransitionCoreTypeIs(visits[0].transition,
ui::PAGE_TRANSITION_TYPED));
// Second client has only the typed visit.
visits = typed_urls_helper::GetVisitsForURLFromClient(1, kNonTypedURL);
ASSERT_EQ(0U, visits.size());
visits = typed_urls_helper::GetVisitsForURLFromClient(1, kTypedURL);
ASSERT_EQ(1U, visits.size());
EXPECT_TRUE(ui::PageTransitionCoreTypeIs(visits[0].transition,
ui::PAGE_TRANSITION_TYPED));
}

IN_PROC_BROWSER_TEST_F(TwoClientTypedUrlsSyncTest, SyncTypedRedirects) {
const base::string16 kHistoryUrl(ASCIIToUTF16("http://typed.google.com/"));
const base::string16 kRedirectedHistoryUrl(
Expand Down
11 changes: 10 additions & 1 deletion chrome/browser/sync/test/integration/typed_urls_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ void AddToHistory(history::HistoryService* service,
transition,
source,
false);
service->SetPageTitle(url, base::ASCIIToUTF16(url.spec() + " - title"));
}

history::URLRows GetTypedUrlsFromHistoryService(
Expand Down Expand Up @@ -332,6 +331,16 @@ void DeleteUrlsFromHistory(int index, const std::vector<GURL>& urls) {
WaitForHistoryDBThread(index);
}

void SetPageTitle(int index, const GURL& url, const std::string& title) {
HistoryServiceFactory::GetForProfileWithoutCreating(test()->GetProfile(index))
->SetPageTitle(url, base::UTF8ToUTF16(title));
if (test()->use_verifier())
HistoryServiceFactory::GetForProfile(test()->verifier(),
ServiceAccessType::IMPLICIT_ACCESS)
->SetPageTitle(url, base::UTF8ToUTF16(title));
WaitForHistoryDBThread(index);
}

bool CheckURLRowVectorsAreEqual(const history::URLRows& left,
const history::URLRows& right) {
if (left.size() != right.size())
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/sync/test/integration/typed_urls_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ void DeleteUrlFromHistory(int index, const GURL& url);
// profile.
void DeleteUrlsFromHistory(int index, const std::vector<GURL>& urls);

// Modifies an URL stored in history by setting a new title.
void SetPageTitle(int index, const GURL& url, const std::string& title);

// Returns true if all clients match the verifier profile.
bool CheckAllProfilesHaveSameURLsAsVerifier();

Expand Down

0 comments on commit 6170f24

Please sign in to comment.