Skip to content

Commit

Permalink
Bug 1776608: Record bookmark source. r=mak
Browse files Browse the repository at this point in the history
  • Loading branch information
Daisuke Akatsuka committed Aug 1, 2022
1 parent 8bcb083 commit d795fbe
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ const OPEN_TYPE = {
NEWTAB_BY_CONTEXTMENU: 2,
};

const { VISIT_SOURCE_ORGANIC, VISIT_SOURCE_SPONSORED } = PlacesUtils.history;
const {
VISIT_SOURCE_ORGANIC,
VISIT_SOURCE_SPONSORED,
VISIT_SOURCE_BOOKMARKED,
} = PlacesUtils.history;

async function assertDatabase({ targetURL, expected }) {
const placesId = await PlacesTestUtils.fieldInDB(targetURL, "id");
Expand Down Expand Up @@ -175,6 +179,44 @@ add_task(async function basic() {
source: VISIT_SOURCE_SPONSORED,
},
},
{
description: "Bookmarked result",
link: {
label: "test_label",
url: "http://example.com/",
},
bookmarks: [
{
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: Services.io.newURI("http://example.com/"),
title: "test bookmark",
},
],
expected: {
source: VISIT_SOURCE_BOOKMARKED,
},
},
{
description: "Sponsored and bookmarked result",
link: {
label: "test_label",
url: "http://example.com/",
sponsored_position: 1,
sponsored_tile_id: 12345,
sponsored_impression_url: "http://impression.example.com/",
sponsored_click_url: "http://click.example.com/",
},
bookmarks: [
{
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: Services.io.newURI("http://example.com/"),
title: "test bookmark",
},
],
expected: {
source: VISIT_SOURCE_SPONSORED,
},
},
{
description: "Organic tile",
link: {
Expand All @@ -187,14 +229,17 @@ add_task(async function basic() {
},
];

for (const { description, link, expected } of testData) {
for (const { description, link, bookmarks, expected } of testData) {
info(description);

await BrowserTestUtils.withNewTab("about:home", async () => {
// Setup test tile.
await pin(link);

// Test with new tab.
for (const bookmark of bookmarks || []) {
await PlacesUtils.bookmarks.insert(bookmark);
}
await openAndTest({
linkSelector: ".top-site-button",
linkURL: link.url,
Expand All @@ -205,6 +250,9 @@ add_task(async function basic() {
await clearHistoryAndBookmarks();

// Test with same tab.
for (const bookmark of bookmarks || []) {
await PlacesUtils.bookmarks.insert(bookmark);
}
await openAndTest({
linkSelector: ".top-site-button",
linkURL: link.url,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@

// Test whether a visit information is annotated correctly when picking a result.

const { VISIT_SOURCE_ORGANIC, VISIT_SOURCE_SPONSORED } = PlacesUtils.history;
const {
VISIT_SOURCE_ORGANIC,
VISIT_SOURCE_SPONSORED,
VISIT_SOURCE_BOOKMARKED,
} = PlacesUtils.history;

async function assertDatabase({ targetURL, expected }) {
const placesId = await PlacesTestUtils.fieldInDB(targetURL, "id");
Expand Down Expand Up @@ -89,6 +93,41 @@ add_task(async function basic() {
source: VISIT_SOURCE_SPONSORED,
},
},
{
description: "Bookmarked result",
input: "exa",
payload: {
url: "http://example.com/",
},
bookmarks: [
{
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: Services.io.newURI("http://example.com/"),
title: "test bookmark",
},
],
expected: {
source: VISIT_SOURCE_BOOKMARKED,
},
},
{
description: "Sponsored and bookmarked result",
input: "exa",
payload: {
url: "http://example.com/",
isSponsored: true,
},
bookmarks: [
{
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
url: Services.io.newURI("http://example.com/"),
title: "test bookmark",
},
],
expected: {
source: VISIT_SOURCE_SPONSORED,
},
},
{
description: "Organic result",
input: "exa",
Expand All @@ -101,10 +140,14 @@ add_task(async function basic() {
},
];

for (const { description, input, payload, expected } of testData) {
for (const { description, input, payload, bookmarks, expected } of testData) {
info(description);
const provider = registerProvider(payload);

for (const bookmark of bookmarks || []) {
await PlacesUtils.bookmarks.insert(bookmark);
}

await BrowserTestUtils.withNewTab("about:blank", async () => {
info("Pick result");
await pickResult({ input, payloadURL: payload.url });
Expand Down
21 changes: 16 additions & 5 deletions toolkit/components/places/History.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ struct VisitData {
useFrecencyRedirectBonus(false),
source(nsINavHistoryService::VISIT_SOURCE_ORGANIC),
triggeringPlaceId(0),
triggeringSponsoredURLVisitTimeMS(0) {
triggeringSponsoredURLVisitTimeMS(0),
bookmarked(false) {
guid.SetIsVoid(true);
title.SetIsVoid(true);
baseDomain.SetIsVoid(true);
Expand All @@ -115,7 +116,8 @@ struct VisitData {
useFrecencyRedirectBonus(false),
source(nsINavHistoryService::VISIT_SOURCE_ORGANIC),
triggeringPlaceId(0),
triggeringSponsoredURLVisitTimeMS(0) {
triggeringSponsoredURLVisitTimeMS(0),
bookmarked(false) {
MOZ_ASSERT(aURI);
if (aURI) {
(void)aURI->GetSpec(spec);
Expand Down Expand Up @@ -185,6 +187,7 @@ struct VisitData {
nsCString triggeringSponsoredURL;
nsCString triggeringSponsoredURLBaseDomain;
int64_t triggeringSponsoredURLVisitTimeMS;
bool bookmarked;
};

////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1210,7 +1213,9 @@ class InsertVisitedURIs final : public Runnable {
}

nsresult UpdateVisitSource(VisitData& aPlace, History* aHistory) {
aPlace.source = nsINavHistoryService::VISIT_SOURCE_ORGANIC;
aPlace.source = aPlace.bookmarked
? nsINavHistoryService::VISIT_SOURCE_BOOKMARKED
: nsINavHistoryService::VISIT_SOURCE_ORGANIC;

if (aPlace.triggeringSponsoredURL.IsEmpty()) {
// No triggeringSponsoredURL.
Expand Down Expand Up @@ -1668,7 +1673,8 @@ nsresult History::FetchPageInfo(VisitData& _place, bool* _exists) {
"last_visit_date, "
"(SELECT id FROM moz_historyvisits "
"WHERE place_id = h.id AND visit_date = h.last_visit_date) AS "
"last_visit_id "
"last_visit_id, "
"EXISTS(SELECT 1 FROM moz_bookmarks WHERE fk = h.id) AS bookmarked "
"FROM moz_places h "
"WHERE url_hash = hash(:page_url) AND url = :page_url ");
NS_ENSURE_STATE(stmt);
Expand All @@ -1681,7 +1687,8 @@ nsresult History::FetchPageInfo(VisitData& _place, bool* _exists) {
"last_visit_date, "
"(SELECT id FROM moz_historyvisits "
"WHERE place_id = h.id AND visit_date = h.last_visit_date) AS "
"last_visit_id "
"last_visit_id, "
"EXISTS(SELECT 1 FROM moz_bookmarks WHERE fk = h.id) AS bookmarked "
"FROM moz_places h "
"WHERE guid = :guid ");
NS_ENSURE_STATE(stmt);
Expand Down Expand Up @@ -1751,6 +1758,10 @@ nsresult History::FetchPageInfo(VisitData& _place, bool* _exists) {
NS_ENSURE_SUCCESS(rv, rv);
rv = stmt->GetInt64(8, &_place.lastVisitId);
NS_ENSURE_SUCCESS(rv, rv);
int32_t bookmarked;
rv = stmt->GetInt32(9, &bookmarked);
NS_ENSURE_SUCCESS(rv, rv);
_place.bookmarked = bookmarked == 1;

return NS_OK;
}
Expand Down
5 changes: 5 additions & 0 deletions toolkit/components/places/nsINavHistoryService.idl
Original file line number Diff line number Diff line change
Expand Up @@ -1032,6 +1032,11 @@ interface nsINavHistoryService : nsISupports
*/
const unsigned short VISIT_SOURCE_SPONSORED = 1;

/**
* Insert this value into moz_historyvisits if the visit source is bookmarked.
*/
const unsigned short VISIT_SOURCE_BOOKMARKED = 2;

/**
* Returns the current database status
*/
Expand Down

0 comments on commit d795fbe

Please sign in to comment.