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

Using temporarySiteSettings to store private tab preferences #4469

Merged
merged 3 commits into from
Oct 4, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 4 additions & 1 deletion js/components/braveryPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ class BraveryPanel extends ImmutableComponent {
get isFpShown () {
return this.props.braveryPanelDetail.get('expandFp')
}
get isPrivate () {
return this.props.frameProps.getIn(['isPrivate'])
}
get redirectedResources () {
return this.props.frameProps.get('httpsEverywhere')
}
Expand Down Expand Up @@ -144,7 +147,7 @@ class BraveryPanel extends ImmutableComponent {
if (setting !== 'noScript' && (parsedUrl.protocol === 'https:' || parsedUrl.protocol === 'http:')) {
ruleKey = `https?://${parsedUrl.host}`
}
appActions.changeSiteSetting(ruleKey, setting, e.target.value)
appActions.changeSiteSetting(ruleKey, setting, e.target.value, this.isPrivate)
this.onReload()
}
get displayHost () {
Expand Down
6 changes: 3 additions & 3 deletions js/components/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,11 @@ class Frame extends ImmutableComponent {
}
if (typeof activeSiteSettings.get('flash') === 'number') {
if (activeSiteSettings.get('flash') < Date.now()) {
appActions.removeSiteSetting(origin, 'flash')
appActions.removeSiteSetting(origin, 'flash', this.props.isPrivate)
}
}
if (activeSiteSettings.get('noScript') === 0) {
appActions.removeSiteSetting(origin, 'noScript')
appActions.removeSiteSetting(origin, 'noScript', this.props.isPrivate)
}
}

Expand Down Expand Up @@ -314,7 +314,7 @@ class Frame extends ImmutableComponent {

get zoomLevel () {
const zoom = this.props.frameSiteSettings && this.props.frameSiteSettings.get('zoomLevel')
appActions.removeSiteSetting(this.origin, 'zoomLevel')
appActions.removeSiteSetting(this.origin, 'zoomLevel', this.props.isPrivate)
return zoom
}

Expand Down
9 changes: 4 additions & 5 deletions js/components/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,10 @@ class Main extends ImmutableComponent {
}

get allSiteSettings () {
const activeFrame = FrameStateUtil.getActiveFrame(this.props.windowState)
if (activeFrame && activeFrame.get('isPrivate')) {
return this.props.appState.get('siteSettings').mergeDeep(this.props.appState.get('temporarySiteSettings'))
}
Copy link
Member

Choose a reason for hiding this comment

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

need to be careful here that the allSiteSettings frame property is consistent with what is being sent in js/state/contentSettings.js and also registerPermissionHandler in app/filtering.js. the expected behavior is that private context inheirit settings from regular contexts but not vice-versa, so I think what you want here is a merge of temporarySiteSettings and siteSettings for private frames.

Copy link
Member Author

Choose a reason for hiding this comment

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

Address in 7de0c05.

return this.props.appState.get('siteSettings')
}

Expand All @@ -741,11 +745,6 @@ class Main extends ImmutableComponent {
}

get braveShieldsDisabled () {
const activeFrame = FrameStateUtil.getActiveFrame(this.props.windowState)
if (activeFrame && activeFrame.get('isPrivate')) {
return true
}

const activeRequestedLocation = this.activeRequestedLocation
if (!activeRequestedLocation) {
return true
Expand Down
4 changes: 0 additions & 4 deletions js/components/siteInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,6 @@ class SiteInfo extends ImmutableComponent {
</ul>
</li>
}
// Disable in private mode for now
if (this.isPrivate) {
runInsecureContentInfo = null
}

return <Dialog onHide={this.props.onHide} className='siteInfo' isClickDismiss>
<ul onClick={(e) => e.stopPropagation()}>
Expand Down
127 changes: 71 additions & 56 deletions js/state/contentSettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const {cookieExceptions, localStorageExceptions} = require('../data/siteHacks')
const {passwordManagers, defaultPasswordManager} = require('../constants/passwordManagers')
const urlParse = require('url').parse
const siteSettings = require('./siteSettings')
const { registerUserPrefs } = require('./userPrefs')
const { setUserPref } = require('./userPrefs')
const { getSetting } = require('../settings')

// backward compatibility with appState siteSettings
Expand Down Expand Up @@ -79,10 +79,59 @@ const getBlock3rdPartyStorage = (braveryDefaults) => {
}
}

const getContentSettingsFromSiteSettings = (appState) => {
const hostSettingsToContentSettings = (hostSettings, contentSettingsSource) => {
let contentSettings = contentSettingsSource
// We do 2 passes for setting content settings. On the first pass we consider all shield types.
for (let hostPattern in hostSettings) {
let hostSetting = hostSettings[hostPattern]
if (['number', 'boolean'].includes(typeof hostSetting.noScript)) {
addContentSettings(contentSettings.javascript, hostPattern, '*',
hostSetting.noScript === true ? 'block' : 'allow')
}
if (typeof hostSetting.runInsecureContent === 'boolean') {
addContentSettings(contentSettings.runInsecureContent, hostPattern, '*',
hostSetting.runInsecureContent ? 'allow' : 'block')
}
if (hostSetting.cookieControl) {
if (hostSetting.cookieControl === 'block3rdPartyCookie') {
addContentSettings(contentSettings.cookies, hostPattern, '*', 'block')
addContentSettings(contentSettings.cookies, hostPattern, parseSiteSettingsPattern(hostPattern), 'allow')
addContentSettings(contentSettings.referer, hostPattern, '*', 'block')
cookieExceptions.forEach((exceptionPair) =>
addContentSettings(contentSettings.cookies, exceptionPair[0], exceptionPair[1], 'allow'))
} else {
addContentSettings(contentSettings.cookies, hostPattern, '*', 'allow')
addContentSettings(contentSettings.referer, hostPattern, '*', 'allow')
}
}
if (typeof hostSetting.fingerprintingProtection === 'boolean') {
addContentSettings(contentSettings.canvasFingerprinting, hostPattern, '*', hostSetting.fingerprintingProtection ? 'block' : 'allow')
}
if (hostSetting.adControl) {
addContentSettings(contentSettings.adInsertion, hostPattern, '*', hostSetting.adControl === 'showBraveAds' ? 'allow' : 'block')
}
if (typeof hostSetting.flash === 'number') {
addContentSettings(contentSettings.flashActive, hostPattern, '*', 'allow')
}
}
// On the second pass we consider only shieldsUp === false settings since we want those to take precedence.
for (let hostPattern in hostSettings) {
let hostSetting = hostSettings[hostPattern]
if (hostSetting.shieldsUp === false) {
addContentSettings(contentSettings.cookies, hostPattern, '*', 'allow')
addContentSettings(contentSettings.canvasFingerprinting, hostPattern, '*', 'allow')
addContentSettings(contentSettings.adInsertion, hostPattern, '*', 'block')
addContentSettings(contentSettings.javascript, hostPattern, '*', 'allow')
addContentSettings(contentSettings.referer, hostPattern, '*', 'allow')
}
}
return contentSettings
}

const getContentSettingsFromSiteSettings = (appState, isPrivate = false) => {
let braveryDefaults = siteSettings.braveryDefaults(appState, appConfig)

let contentSettings = {
const contentSettings = {
cookies: getBlock3rdPartyStorage(braveryDefaults),
referer: [{
setting: braveryDefaults.cookieControl === 'block3rdPartyCookie' ? 'block' : 'allow',
Expand Down Expand Up @@ -126,85 +175,51 @@ const getContentSettingsFromSiteSettings = (appState) => {
}]
}

let hostSettings = appState.get('siteSettings').toJS()
// We do 2 passes for setting content settings. On the first pass we consider all shield types.
for (let hostPattern in hostSettings) {
let hostSetting = hostSettings[hostPattern]
if (['number', 'boolean'].includes(typeof hostSetting.noScript)) {
addContentSettings(contentSettings.javascript, hostPattern, '*',
hostSetting.noScript === true ? 'block' : 'allow')
}
if (typeof hostSetting.runInsecureContent === 'boolean') {
addContentSettings(contentSettings.runInsecureContent, hostPattern, '*',
hostSetting.runInsecureContent ? 'allow' : 'block')
}
if (hostSetting.cookieControl) {
if (hostSetting.cookieControl === 'block3rdPartyCookie') {
addContentSettings(contentSettings.cookies, hostPattern, '*', 'block')
addContentSettings(contentSettings.cookies, hostPattern, parseSiteSettingsPattern(hostPattern), 'allow')
addContentSettings(contentSettings.referer, hostPattern, '*', 'block')
cookieExceptions.forEach((exceptionPair) =>
addContentSettings(contentSettings.cookies, exceptionPair[0], exceptionPair[1], 'allow'))
} else {
addContentSettings(contentSettings.cookies, hostPattern, '*', 'allow')
addContentSettings(contentSettings.referer, hostPattern, '*', 'allow')
}
}
if (typeof hostSetting.fingerprintingProtection === 'boolean') {
addContentSettings(contentSettings.canvasFingerprinting, hostPattern, '*', hostSetting.fingerprintingProtection ? 'block' : 'allow')
}
if (hostSetting.adControl) {
addContentSettings(contentSettings.adInsertion, hostPattern, '*', hostSetting.adControl === 'showBraveAds' ? 'allow' : 'block')
}
if (typeof hostSetting.flash === 'number') {
addContentSettings(contentSettings.flashActive, hostPattern, '*', 'allow')
}
}
// On the second pass we consider only shieldsUp === false settings since we want those to take precedence.
for (let hostPattern in hostSettings) {
let hostSetting = hostSettings[hostPattern]
if (hostSetting.shieldsUp === false) {
addContentSettings(contentSettings.cookies, hostPattern, '*', 'allow')
addContentSettings(contentSettings.canvasFingerprinting, hostPattern, '*', 'allow')
addContentSettings(contentSettings.adInsertion, hostPattern, '*', 'block')
addContentSettings(contentSettings.javascript, hostPattern, '*', 'allow')
addContentSettings(contentSettings.referer, hostPattern, '*', 'allow')
}
const regularSettings = hostSettingsToContentSettings(appState.get('siteSettings').toJS(), contentSettings)
if (isPrivate) {
const privateSettings =
hostSettingsToContentSettings(appState.get('siteSettings').merge(appState.get('temporarySiteSettings')).toJS(),
Copy link
Member

Choose a reason for hiding this comment

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

s/merge/mergeDeep

Copy link
Collaborator

Choose a reason for hiding this comment

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

sorry I missed this earlier, but the deep merge is not required or desirable. The overlay pref store already handles this correctly if you set the regular settings for the regular profile and only the private settings for the private profile

Copy link
Collaborator

Choose a reason for hiding this comment

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

bool OverlayUserPrefStore::GetValue(const std::string& key,
                                    const base::Value** result) const {
  // If the |key| shall NOT be stored in the overlay store, there must not
  // be an entry.
  DCHECK(ShallBeStoredInOverlay(key) || !overlay_.GetValue(key, NULL));

  if (overlay_.GetValue(key, result))
    return true;
  return underlay_->GetValue(GetUnderlayKey(key), result);
}

contentSettings)
return { content_settings: privateSettings }
}

return { content_settings: contentSettings }
return { content_settings: regularSettings }
}

let updateTrigger

// Register callback to handle all updates
const doAction = (action) => {
switch (action.actionType) {
case AppConstants.APP_CHANGE_SITE_SETTING:
AppDispatcher.waitFor([AppStore.dispatchToken], () => {
updateTrigger('content_settings', action.temporary)
if (action.temporary) {
setUserPref('content_settings', getContentSettingsFromSiteSettings(AppStore.getState(), true).content_settings, true)
} else {
setUserPref('content_settings', getContentSettingsFromSiteSettings(AppStore.getState()).content_settings)
}
})
break
case AppConstants.APP_REMOVE_SITE_SETTING:
AppDispatcher.waitFor([AppStore.dispatchToken], () => {
updateTrigger('content_settings', action.temporary)
if (action.temporary) {
setUserPref('content_settings', getContentSettingsFromSiteSettings(AppStore.getState(), true).content_settings, true)
} else {
setUserPref('content_settings', getContentSettingsFromSiteSettings(AppStore.getState()).content_settings)
}
})
break
case AppConstants.APP_SET_RESOURCE_ENABLED:
AppDispatcher.waitFor([AppStore.dispatchToken], () => {
updateTrigger()
setUserPref('content_settings', getContentSettingsFromSiteSettings(AppStore.getState()).content_settings)
})
break
case AppConstants.APP_CHANGE_SETTING:
AppDispatcher.waitFor([AppStore.dispatchToken], () => {
updateTrigger()
setUserPref('content_settings', getContentSettingsFromSiteSettings(AppStore.getState()).content_settings)
})
break
default:
}
}

module.exports.init = () => {
updateTrigger = registerUserPrefs(() => getContentSettingsFromSiteSettings(AppStore.getState()))
AppDispatcher.register(doAction)
}
6 changes: 3 additions & 3 deletions js/state/userPrefs.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,19 @@ const runCallback = (cb, name, incognito) => {

if (name) {
if (prefs[name]) {
setUserPref(name, prefs[name], incognito)
module.exports.setUserPref(name, prefs[name], incognito)
return true
}
return false
}

for (name in prefs) {
setUserPref(name, prefs[name], incognito)
module.exports.setUserPref(name, prefs[name], incognito)
}
return true
}

const setUserPref = (path, value, incognito = false) => {
module.exports.setUserPref = (path, value, incognito = false) => {
let partitions = incognito ? Object.keys(registeredPrivateSessions) : Object.keys(registeredSessions)
partitions.forEach((partition) => {
setUserPrefType(registeredSessions[partition], path, value)
Expand Down