Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Harmonize publisher visits #3852

Merged
merged 2 commits into from
Sep 9, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
keep track of location/tabId pairs
repeats of a location/tabId pair should result in a `revisitP=true`
call to `synopsis.addPublisher` so the `visits` counter is not
incremented.

auditor: @diracdeltas
  • Loading branch information
mrose17 committed Sep 9, 2016
commit a711c39a9b62f3d5384c5983999013131608749e
30 changes: 19 additions & 11 deletions app/ledger.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ var init = () => {
}

var quit = () => {
visit('NOOP', underscore.now())
visit('NOOP', underscore.now(), null)
}

var boot = () => {
Expand Down Expand Up @@ -322,7 +322,7 @@ eventStore.addChangeListener(() => {
})

view = underscore.last(view) || {}
visit(view.url || 'NOOP', view.timestamp || underscore.now())
visit(view.url || 'NOOP', view.timestamp || underscore.now(), view.tabId)
})

/*
Expand Down Expand Up @@ -427,7 +427,7 @@ var enable = (onoff) => {
publishers[publisher] = {}
entries.forEach((entry) => {
locations[entry.location] = entry
publishers[publisher][entry.location] = entry.when
publishers[publisher][entry.location] = { timestamp: entry.when, tabIds: [] }
})
})
} catch (ex) {
Expand Down Expand Up @@ -459,7 +459,8 @@ var updatePublisherInfo = () => {
var entries = []

underscore.keys(publishers[publisher]).forEach((location) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can just do underscore.each(publishers, ...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this case, no... because in the function i need access to both the key and value, not just the value...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both key and value are accessible http://underscorejs.org/#each

var when = publishers[publisher][location]
var when = publishers[publisher][location].timestamp

if (when > then) entries.push({ location: location, when: when })
})

Expand Down Expand Up @@ -579,27 +580,34 @@ var synopsisNormalizer = () => {
var currentLocation = 'NOOP'
var currentTimestamp = underscore.now()

var visit = (location, timestamp) => {
var visit = (location, timestamp, tabId) => {
var setLocation = () => {
var duration, publisher
var duration, publisher, revisitP

if (!synopsis) return

if (publisherInfo._internal.verboseP) {
console.log('locations[' + currentLocation + ']=' + JSON.stringify(locations[currentLocation], null, 2) +
' duration=' + (timestamp - currentTimestamp) + ' msec')
' duration=' + (timestamp - currentTimestamp) + ' msec' + ' tabId=' + tabId)
}
if ((location === currentLocation) || (!locations[currentLocation])) return
if ((location === currentLocation) || (!locations[currentLocation]) || (!tabId)) return

publisher = locations[currentLocation].publisher
if (!publisher) return

if (!publishers[publisher]) publishers[publisher] = {}
publishers[publisher][currentLocation] = timestamp
if (!publishers[publisher][currentLocation]) publishers[publisher][currentLocation] = { tabIds: [] }
publishers[publisher][currentLocation].timestamp = timestamp
revisitP = publishers[publisher][currentLocation].tabIds.indexOf(tabId) !== -1
if (!revisitP) publishers[publisher][currentLocation].tabIds.push(tabId)

duration = timestamp - currentTimestamp
if (publisherInfo._internal.verboseP) console.log('\nadd publisher ' + publisher + ': ' + duration + ' msec')
synopsis.addPublisher(publisher, duration)
if (publisherInfo._internal.verboseP) {
console.log('\nadd publisher ' + publisher + ': ' + duration + ' msec' + ' revisitP=' + revisitP + ' state=' +
JSON.stringify(underscore.extend({ location: currentLocation }, publishers[publisher][currentLocation]),
null, 2))
}
synopsis.addPublisher(publisher, { duration: duration, revisitP: revisitP })
updatePublisherInfo()
}

Expand Down
17 changes: 9 additions & 8 deletions js/stores/eventStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const emitChanges = debounce(eventStore.emitChanges.bind(eventStore), 5)
let lastActivePageUrl = null
let lastActiveTabId = null

const addPageView = (url) => {
const addPageView = (url, tabId) => {
if (url && isSourceAboutUrl(url)) {
url = null
}
Expand All @@ -57,7 +57,8 @@ const addPageView = (url) => {

let pageViewEvent = Immutable.fromJS({
timestamp: new Date().getTime(),
url
url,
tabId
})
eventState = eventState.set('page_view', eventState.get('page_view').push(pageViewEvent))
lastActivePageUrl = url
Expand All @@ -66,7 +67,7 @@ const addPageView = (url) => {
const windowBlurred = (windowId) => {
let windowCount = BrowserWindow.getAllWindows().filter((win) => win.isFocused()).length
if (windowCount === 0) {
addPageView(null)
addPageView(null, null)
}
}

Expand All @@ -81,7 +82,7 @@ const windowClosed = (windowId) => {
}

if (!win || windowCount === 0) {
addPageView(null)
addPageView(null, null)
}
}

Expand All @@ -91,7 +92,7 @@ const doAction = (action) => {
case WindowConstants.WINDOW_WEBVIEW_LOAD_END:
// create a page view event if this is a page load on the active tabId
if (!lastActiveTabId || action.frameProps.get('tabId') === lastActiveTabId) {
addPageView(action.frameProps.get('location'))
addPageView(action.frameProps.get('location'), action.frameProps.get('tabId'))
}

if (action.isError || isSourceAboutUrl(action.frameProps.get('location'))) {
Expand All @@ -105,17 +106,17 @@ const doAction = (action) => {
eventState = eventState.set('page_load', eventState.get('page_load').push(pageLoadEvent))
break
case WindowConstants.WINDOW_SET_FOCUSED_FRAME:
addPageView(action.frameProps.get('location'))
lastActiveTabId = action.frameProps.get('tabId')
addPageView(action.frameProps.get('location'), lastActiveTabId)
break
case AppConstants.APP_WINDOW_BLURRED:
windowBlurred(action.appWindowId)
break
case AppConstants.APP_IDLE_STATE_CHANGED:
if (action.idleState !== 'active') {
addPageView(null)
addPageView(null, null)
} else {
addPageView(lastActivePageUrl)
addPageView(lastActivePageUrl, lastActiveTabId)
}
break
case AppConstants.APP_CLOSE_WINDOW:
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
"ledger-balance": "^0.8.46",
"ledger-client": "^0.8.59",
"ledger-geoip": "https://github.com/brave/ledger-geoip.git",
"ledger-publisher": "^0.8.58",
"ledger-publisher": "^0.8.61",
"lru_cache": "^1.0.0",
"qr-image": "^3.1.0",
"random-lib": "2.1.0",
Expand Down