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

Commit

Permalink
dont’t call to get site settings outside of mergeProps
Browse files Browse the repository at this point in the history
  • Loading branch information
@bridiver
bridiver committed Jun 30, 2017
1 parent 4147821 commit cc3eedd
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 64 deletions.
7 changes: 7 additions & 0 deletions app/common/state/tabState.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,13 @@ const tabState = {
return entry ? entry.get('url') : ''
},

getVisibleOrigin: (state, tabId) => {
const entry = tabState.getVisibleEntry(state, tabId)
const origin = entry ? entry.get('origin') : ''
// TODO(bridiver) - all origins in browser-laptop should be changed to have a trailing slash to match chromium
return (origin || '').replace(/\/$/, '')
},

getVisibleVirtualURL: (state, tabId) => {
const entry = tabState.getVisibleEntry(state, tabId)
return entry ? entry.get('virtualURL') : ''
Expand Down
105 changes: 44 additions & 61 deletions app/renderer/components/frame/frame.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,6 @@ class Frame extends React.Component {
windowActions.closeFrame(this.props.frameKey)
}

getFrameBraverySettings (props) {
props = props || this.props
const frameSiteSettings =
siteSettings.getSiteSettingsForURL(props.allSiteSettings, props.location)
return Immutable.fromJS(siteSettings.activeSettings(frameSiteSettings,
appStoreRenderer.state,
appConfig))
}

isAboutPage () {
return aboutUrls.get(getBaseUrl(this.props.location))
}
Expand All @@ -112,58 +103,44 @@ class Frame extends React.Component {
return !this.webview
}

runInsecureContent () {
const activeSiteSettings = siteSettings.getSiteSettingsForHostPattern(this.props.allSiteSettings, this.origin)
return activeSiteSettings === undefined
? false : activeSiteSettings.get('runInsecureContent')
}

allowRunningWidevinePlugin (url) {
if (!this.props.isWidevineEnabled) {
return false
}
const origin = url ? siteUtil.getOrigin(url) : this.origin
if (!origin) {
if (!this.props.origin) {
return false
}
// Check for at least one CtP allowed on this origin
if (!this.props.allSiteSettings) {
return false
}
const activeSiteSettings = siteSettings.getSiteSettingsForHostPattern(this.props.allSiteSettings, origin)
if (activeSiteSettings && typeof activeSiteSettings.get('widevine') === 'number') {
if (typeof this.props.widevine === 'number') {
return true
}
return false
}

expireContentSettings (origin) {
expireContentSettings (props) {
// Expired Flash settings should be deleted when the webview is
// navigated or closed. Same for NoScript's allow-once option.
const activeSiteSettings = siteSettings.getSiteSettingsForHostPattern(this.props.allSiteSettings, origin)
if (!activeSiteSettings) {
return
}
if (typeof activeSiteSettings.get('flash') === 'number') {
if (activeSiteSettings.get('flash') < Date.now()) {
appActions.removeSiteSetting(origin, 'flash', this.props.isPrivate)
if (typeof props.flash === 'number') {
if (props.flash < Date.now()) {
appActions.removeSiteSetting(props.origin, 'flash', props.isPrivate)
}
}
if (activeSiteSettings.get('widevine') === 0) {
appActions.removeSiteSetting(origin, 'widevine', this.props.isPrivate)
if (props.widevine === 0) {
appActions.removeSiteSetting(props.origin, 'widevine', props.isPrivate)
}
if (activeSiteSettings.get('noScript') === 0) {
appActions.removeSiteSetting(origin, 'noScript', this.props.isPrivate)
if (props.noScript === 0) {
appActions.removeSiteSetting(props.origin, 'noScript', props.isPrivate)
}
const noScriptExceptions = activeSiteSettings.get('noScriptExceptions')
if (noScriptExceptions) {
appActions.noScriptExceptionsAdded(origin,
noScriptExceptions.map(value => value === 0 ? false : value))
if (props.noScriptExceptions) {
appActions.noScriptExceptionsAdded(props.origin, props.noScriptExceptions.map(value => value === 0 ? false : value))
}
}

componentWillUnmount () {
this.expireContentSettings(this.origin)
this.expireContentSettings(this.props)
}

updateWebview (cb, prevProps = {}) {
Expand Down Expand Up @@ -218,7 +195,7 @@ class Frame extends React.Component {

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

Expand Down Expand Up @@ -288,9 +265,8 @@ class Frame extends React.Component {
}

// For cross-origin navigation, clear temp approvals
const prevOrigin = siteUtil.getOrigin(prevProps.location)
if (this.origin !== prevOrigin) {
this.expireContentSettings(prevOrigin)
if (this.props.origin !== prevProps.origin) {
this.expireContentSettings(prevProps)
}

this.updateWebview(cb, prevProps)
Expand Down Expand Up @@ -388,13 +364,15 @@ class Frame extends React.Component {
* @param {function=} widevineCallback - Optional callback to run if Widevine is
* accepted
*/
showWidevineNotification (location, origin, noWidevineCallback, widevineCallback) {
showWidevineNotification (noWidevineCallback, widevineCallback) {
// https://www.nfl.com is said to be a widevine site but it actually uses Flash for me Oct 10, 2016
const widevineSites = ['https://www.netflix.com',
'http://bitmovin.com',
'https://www.primevideo.com',
'https://www.spotify.com',
'https://shaka-player-demo.appspot.com']
const origin = this.props.origin
const location = this.props.location
const isForWidevineTest = process.env.NODE_ENV === 'test' && location.endsWith('/drm.html')
if (!isForWidevineTest && (!origin || !widevineSites.includes(origin))) {
noWidevineCallback()
Expand All @@ -406,15 +384,15 @@ class Frame extends React.Component {
const nonce = Math.random().toString()

if (this.props.isWidevineEnabled) {
const message = locale.translation('allowWidevine').replace(/{{\s*origin\s*}}/, this.origin)
const message = locale.translation('allowWidevine').replace(/{{\s*origin\s*}}/, origin)
// Show Widevine notification bar
appActions.showNotification({
buttons: [
{text: locale.translation('deny')},
{text: locale.translation('allow')}
],
message,
frameOrigin: this.origin,
frameOrigin: origin,
options: {
nonce,
persist: true
Expand All @@ -423,16 +401,16 @@ class Frame extends React.Component {
this.notificationCallbacks[message] = (buttonIndex, persist) => {
if (buttonIndex === 1) {
if (persist) {
appActions.changeSiteSetting(this.origin, 'widevine', 1)
appActions.changeSiteSetting(origin, 'widevine', 1)
} else {
appActions.changeSiteSetting(this.origin, 'widevine', 0)
appActions.changeSiteSetting(origin, 'widevine', 0)
}
if (widevineCallback) {
widevineCallback()
}
} else {
if (persist) {
appActions.changeSiteSetting(this.origin, 'widevine', false)
appActions.changeSiteSetting(origin, 'widevine', false)
}
}
appActions.hideNotification(message)
Expand Down Expand Up @@ -611,7 +589,7 @@ class Frame extends React.Component {
}
if (e.isMainFrame && !e.isErrorPage && !e.isFrameSrcDoc) {
if (e.url && e.url.startsWith(appConfig.noScript.twitterRedirectUrl) &&
this.getFrameBraverySettings(this.props).get('noScript') === true) {
this.props.noScript === true) {
// This result will be canceled immediately by sitehacks, so don't
// update the load state; otherwise it will not show the security
// icon.
Expand All @@ -628,7 +606,7 @@ class Frame extends React.Component {
windowActions.onWebviewLoadEnd(this.frame, url)
const parsedUrl = urlParse(url)
if (!this.allowRunningWidevinePlugin()) {
this.showWidevineNotification(this.props.location, this.origin, () => {
this.showWidevineNotification(() => {
}, () => {
appActions.loadURLRequested(this.props.tabId, this.props.provisionalLocation)
})
Expand Down Expand Up @@ -698,7 +676,7 @@ class Frame extends React.Component {
return
}
let isSecure = null
let runInsecureContent = this.runInsecureContent()
let runInsecureContent = this.props.runInsecureContent
if (e.securityState === 'secure') {
isSecure = true
} else if (e.securityState === 'insecure') {
Expand Down Expand Up @@ -766,8 +744,8 @@ class Frame extends React.Component {
})
this.webview.addEventListener('did-finish-load', (e) => {
loadEnd(true, e.validatedURL)
if (this.runInsecureContent()) {
appActions.removeSiteSetting(this.origin, 'runInsecureContent', this.props.isPrivate)
if (this.props.runInsecureContent) {
appActions.removeSiteSetting(this.props.origin, 'runInsecureContent', this.props.isPrivate)
}
})
this.webview.addEventListener('did-navigate-in-page', (e) => {
Expand Down Expand Up @@ -839,10 +817,6 @@ class Frame extends React.Component {
this.webview.addEventListener('mousewheel', this.onMouseWheel.bind(this))
}

get origin () {
return siteUtil.getOrigin(this.props.location)
}

onFocus () {
if (!this.frame.isEmpty()) {
windowActions.setTabPageIndexByFrame(this.frame)
Expand Down Expand Up @@ -885,13 +859,16 @@ class Frame extends React.Component {
mergeProps (state, ownProps) {
const currentWindow = state.get('currentWindow')
const frame = frameStateUtil.getFrameByKey(currentWindow, ownProps.frameKey) || Immutable.Map()
const tabId = frame.get('tabId')

const location = frame.get('location')
const allSiteSettings = siteSettingsState.getAllSiteSettings(state, frame.get('isPrivate'))
const frameSiteSettings = frame.get('location')
? siteSettings.getSiteSettingsForURL(allSiteSettings, frame.get('location'))
: undefined
const origin = tabState.getVisibleOrigin(state, tabId)
const isPrivate = frame.get('isPrivate', false)

const allSiteSettings = siteSettingsState.getAllSiteSettings(state, isPrivate)
const frameSiteSettings = siteSettings.getSiteSettingsForURL(allSiteSettings, location) || Immutable.Map()

const contextMenu = currentWindow.get('contextMenuDetail')
const tabId = frame.get('tabId')
const tab = tabId && tabId > -1 && tabState.getByTabId(state, tabId)

const props = {}
Expand All @@ -909,6 +886,12 @@ class Frame extends React.Component {

// used in other functions
props.frameKey = ownProps.frameKey
props.origin = origin
props.runInsecureContent = frameSiteSettings.get('runInsecureContent')
props.noScript = frameSiteSettings.get('noScript')
props.noScriptExceptions = frameSiteSettings.get('noScriptExceptions')
props.widevine = frameSiteSettings.get('widevine')
props.flash = frameSiteSettings.get('flash')
props.urlBarFocused = frame && frame.getIn(['navbar', 'urlbar', 'focused'])
props.isAutFillContextMenu = contextMenu && contextMenu.get('type') === 'autofill'
props.isSecure = frame.getIn(['security', 'isSecure'])
Expand All @@ -930,7 +913,7 @@ class Frame extends React.Component {
props.aboutDetailsErrorCode = frame.getIn(['aboutDetails', 'errorCode'])
props.unloaded = frame.get('unloaded')
props.isWidevineEnabled = state.get('widevine') && state.getIn(['widevine', 'enabled'])
props.siteZoomLevel = frameSiteSettings && frameSiteSettings.get('zoomLevel')
props.siteZoomLevel = frameSiteSettings.get('zoomLevel')
props.allSiteSettings = allSiteSettings // TODO (nejc) can be improved even more
props.tabUrl = tab && tab.get('url')
props.partitionNumber = frame.get('partitionNumber')
Expand Down
16 changes: 13 additions & 3 deletions test/unit/app/common/state/tabStateTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -763,18 +763,22 @@ describe('tabState unit tests', function () {
before(function () {
this.url = 'http://url'
this.virtualURL = 'http://virtualURL'
this.origin = 'http://url/'
this.navigationState = Immutable.fromJS({
visibleEntry: {
url: this.url,
virtualURL: this.virtualURL
virtualURL: this.virtualURL,
origin: this.origin
},
activeEntry: {
url: 'active entry',
virtualURL: 'active virtual entry'
virtualURL: 'active virtual entry',
origin: 'active origin/'
},
lastCommittedEntry: {
url: 'last entry',
virtual: 'last virtual entry'
virtual: 'last virtual entry',
origin: 'last origin/'
}
})
this.appState = twoTabsAppState.setIn(['tabs', 0, 'navigationState'], this.navigationState)
Expand Down Expand Up @@ -806,6 +810,12 @@ describe('tabState unit tests', function () {
})
})

describe('getVisibleOrigin', function () {
it('returns the value from appState.tabs.`tabId`.navigationState.visibleEntry.origin with trailing slash removed', function () {
assert.deepEqual(tabState.getVisibleOrigin(this.appState, 1), 'http://url')
})
})

describe('getVisibleEntry', function () {
it('returns the value from appState.tabs.`tabId`.navigationState.visibleEntry', function () {
assert.deepEqual(tabState.getVisibleEntry(this.appState, 1).toJS(), this.navigationState.get('visibleEntry').toJS())
Expand Down

0 comments on commit cc3eedd

Please sign in to comment.