diff --git a/front_end/bindings/BreakpointManager.js b/front_end/bindings/BreakpointManager.js index 39d1f7867e..9c57793682 100644 --- a/front_end/bindings/BreakpointManager.js +++ b/front_end/bindings/BreakpointManager.js @@ -63,6 +63,7 @@ export class BreakpointManager extends Common.ObjectWrapper.ObjectWrapper { this._breakpointByStorageId = new Map(); this._workspace.addEventListener(Workspace.Workspace.Events.UISourceCodeAdded, this._uiSourceCodeAdded, this); + this._workspace.addEventListener(Workspace.Workspace.Events.UISourceCodeRemoved, this._uiSourceCodeRemoved, this); } /** @@ -132,6 +133,15 @@ export class BreakpointManager extends Common.ObjectWrapper.ObjectWrapper { this._restoreBreakpoints(uiSourceCode); } + /** + * @param {!Common.EventTarget.EventTargetEvent} event + */ + _uiSourceCodeRemoved(event) { + const uiSourceCode = /** @type {!Workspace.UISourceCode.UISourceCode} */ (event.data); + const breakpoints = this.breakpointLocationsForUISourceCode(uiSourceCode); + breakpoints.forEach(bp => bp.breakpoint.removeUISourceCode(uiSourceCode)); + } + /** * @param {!Workspace.UISourceCode.UISourceCode} uiSourceCode * @param {number} lineNumber @@ -164,7 +174,7 @@ export class BreakpointManager extends Common.ObjectWrapper.ObjectWrapper { let breakpoint = this._breakpointByStorageId.get(itemId); if (breakpoint) { breakpoint._updateState(condition, enabled); - breakpoint.setPrimaryUISourceCode(uiSourceCode); + breakpoint.addUISourceCode(uiSourceCode); breakpoint._updateBreakpoint(); return breakpoint; } @@ -334,20 +344,21 @@ export class Breakpoint { this._lineNumber = lineNumber; this._columnNumber = columnNumber; - /** @type {?Workspace.UISourceCode.UILocation} */ - this._defaultUILocation = null; /** @type {!Set} */ - this._uiLocations = new Set(); + this._uiLocations = new Set(); // Bound locations + /** @type {!Set} */ + this._uiSourceCodes = new Set(); // All known UISourceCodes with this url /** @type {string} */ this._condition; /** @type {boolean} */ this._enabled; /** @type {boolean} */ this._isRemoved; this._currentState = null; + /** @type {!Map.}*/ this._modelBreakpoints = new Map(); this._updateState(condition, enabled); - this.setPrimaryUISourceCode(primaryUISourceCode); + this.addUISourceCode(primaryUISourceCode); this._breakpointManager._targetManager.observeModels(SDK.DebuggerModel.DebuggerModel, this); } @@ -379,19 +390,48 @@ export class Breakpoint { } /** - * @param {?Workspace.UISourceCode.UISourceCode} primaryUISourceCode + * @param {!Workspace.UISourceCode.UISourceCode} uiSourceCode */ - setPrimaryUISourceCode(primaryUISourceCode) { - if (this._uiLocations.size === 0 && this._defaultUILocation) { - this._breakpointManager._uiLocationRemoved(this, this._defaultUILocation); + addUISourceCode(uiSourceCode) { + if (!this._uiSourceCodes.has(uiSourceCode)) { + this._uiSourceCodes.add(uiSourceCode); + if (!this._isBound()) { + this._breakpointManager._uiLocationAdded(this, this._defaultUILocation(uiSourceCode)); + } } - if (primaryUISourceCode) { - this._defaultUILocation = primaryUISourceCode.uiLocation(this._lineNumber, this._columnNumber); - } else { - this._defaultUILocation = null; + } + + clearUISourceCodes() { + if (!this._isBound()) { + this._removeAllUnboundLocations(); } - if (this._uiLocations.size === 0 && this._defaultUILocation && !this._isRemoved) { - this._breakpointManager._uiLocationAdded(this, this._defaultUILocation); + this._uiSourceCodes.clear(); + } + + /** + * @param {!Workspace.UISourceCode.UISourceCode} uiSourceCode + */ + removeUISourceCode(uiSourceCode) { + if (this._uiSourceCodes.has(uiSourceCode)) { + this._uiSourceCodes.delete(uiSourceCode); + if (!this._isBound()) { + this._breakpointManager._uiLocationRemoved(this, this._defaultUILocation(uiSourceCode)); + } + } + + // Do we need to do this? Not sure if bound locations will leak... + if (this._isBound()) { + for (const uiLocation of this._uiLocations) { + if (uiLocation.uiSourceCode === uiSourceCode) { + this._uiLocations.delete(uiLocation); + this._breakpointManager._uiLocationRemoved(this, uiLocation); + } + } + + if (!this._isBound() && !this._isRemoved) { + // Switch to unbound locations + this._addAllUnboundLocations(); + } } } @@ -423,8 +463,9 @@ export class Breakpoint { if (this._isRemoved) { return; } - if (this._uiLocations.size === 0 && this._defaultUILocation) { - this._breakpointManager._uiLocationRemoved(this, this._defaultUILocation); + if (!this._isBound()) { + // This is our first bound location; remove all unbound locations + this._removeAllUnboundLocations(); } this._uiLocations.add(uiLocation); this._breakpointManager._uiLocationAdded(this, uiLocation); @@ -434,10 +475,12 @@ export class Breakpoint { * @param {!Workspace.UISourceCode.UILocation} uiLocation */ _uiLocationRemoved(uiLocation) { - this._uiLocations.delete(uiLocation); - this._breakpointManager._uiLocationRemoved(this, uiLocation); - if (this._uiLocations.size === 0 && this._defaultUILocation && !this._isRemoved) { - this._breakpointManager._uiLocationAdded(this, this._defaultUILocation); + if (this._uiLocations.has(uiLocation)) { + this._uiLocations.delete(uiLocation); + this._breakpointManager._uiLocationRemoved(this, uiLocation); + if (!this._isBound() && !this._isRemoved) { + this._addAllUnboundLocations(); + } } } @@ -484,11 +527,11 @@ export class Breakpoint { } _updateBreakpoint() { - if (this._uiLocations.size === 0 && this._defaultUILocation) { - this._breakpointManager._uiLocationRemoved(this, this._defaultUILocation); - } - if (this._uiLocations.size === 0 && this._defaultUILocation && !this._isRemoved) { - this._breakpointManager._uiLocationAdded(this, this._defaultUILocation); + if (!this._isBound()) { + this._removeAllUnboundLocations(); + if (!this._isRemoved) { + this._addAllUnboundLocations(); + } } for (const modelBreakpoint of this._modelBreakpoints.values()) { modelBreakpoint._scheduleUpdateInDebugger(); @@ -508,7 +551,7 @@ export class Breakpoint { this._breakpointManager._removeBreakpoint(this, removeFromStorage); this._breakpointManager._targetManager.unobserveModels(SDK.DebuggerModel.DebuggerModel, this); - this.setPrimaryUISourceCode(null); + this.clearUISourceCodes(); } /** @@ -519,11 +562,38 @@ export class Breakpoint { } _resetLocations() { - this.setPrimaryUISourceCode(null); + this.clearUISourceCodes(); for (const modelBreakpoint of this._modelBreakpoints.values()) { modelBreakpoint._resetLocations(); } } + + /** + * @return {boolean} + */ + _isBound() { + return this._uiLocations.size !== 0; + } + + /** + * @param {!Workspace.UISourceCode.UISourceCode} uiSourceCode + * @return {!Workspace.UISourceCode.UILocation} + */ + _defaultUILocation(uiSourceCode) { + return uiSourceCode.uiLocation(this._lineNumber, this._columnNumber); + } + + _removeAllUnboundLocations() { + for (const uiSourceCode of this._uiSourceCodes) { + this._breakpointManager._uiLocationRemoved(this, this._defaultUILocation(uiSourceCode)); + } + } + + _addAllUnboundLocations() { + for (const uiSourceCode of this._uiSourceCodes) { + this._breakpointManager._uiLocationAdded(this, this._defaultUILocation(uiSourceCode)); + } + } } /** @@ -588,13 +658,13 @@ export class ModelBreakpoint { * @return {boolean} */ _scriptDiverged() { - const uiLocation = this._breakpoint._defaultUILocation; - const uiSourceCode = uiLocation ? uiLocation.uiSourceCode : null; - if (!uiSourceCode) { - return false; + for (const uiSourceCode of this._breakpoint._uiSourceCodes) { + const scriptFile = this._debuggerWorkspaceBinding.scriptFile(uiSourceCode, this._debuggerModel); + if (scriptFile && scriptFile.hasDivergedFromVM()) { + return true; + } } - const scriptFile = this._debuggerWorkspaceBinding.scriptFile(uiSourceCode, this._debuggerModel); - return !!scriptFile && scriptFile.hasDivergedFromVM(); + return false; } /** @@ -608,17 +678,18 @@ export class ModelBreakpoint { return; } - const uiLocation = this._breakpoint._defaultUILocation; - const uiSourceCode = uiLocation ? uiLocation.uiSourceCode : null; const lineNumber = this._breakpoint._lineNumber; const columnNumber = this._breakpoint._columnNumber; const condition = this._breakpoint.condition(); let debuggerLocation = null; - if (uiSourceCode) { + for (const uiSourceCode of this._breakpoint._uiSourceCodes) { const locations = await DebuggerWorkspaceBinding.instance().uiLocationToRawLocations(uiSourceCode, lineNumber, columnNumber); debuggerLocation = locations.find(location => location.debuggerModel === this._debuggerModel); + if (debuggerLocation) { + break; + } } let newState; if (this._breakpoint._isRemoved || !this._breakpoint.enabled() || this._scriptDiverged()) { @@ -635,8 +706,8 @@ export class ModelBreakpoint { } else if (this._breakpoint._currentState && this._breakpoint._currentState.url) { const position = this._breakpoint._currentState; newState = new Breakpoint.State(position.url, null, null, position.lineNumber, position.columnNumber, condition); - } else if (uiSourceCode) { - newState = new Breakpoint.State(uiSourceCode.url(), null, null, lineNumber, columnNumber, condition); + } else if (this._breakpoint._uiSourceCodes.size > 0) { // Uncertain if this condition is necessary + newState = new Breakpoint.State(this._breakpoint.url(), null, null, lineNumber, columnNumber, condition); } if (this._debuggerId && Breakpoint.State.equals(newState, this._currentState)) { callback(); @@ -734,17 +805,11 @@ export class ModelBreakpoint { */ async _locationUpdated(liveLocation) { const oldUILocation = this._uiLocations.get(liveLocation); + const uiLocation = await liveLocation.uiLocation(); + if (oldUILocation) { this._breakpoint._uiLocationRemoved(oldUILocation); } - let uiLocation = await liveLocation.uiLocation(); - - if (uiLocation) { - const breakpointLocation = this._breakpoint._breakpointManager.findBreakpoint(uiLocation); - if (breakpointLocation && breakpointLocation.uiLocation !== breakpointLocation.breakpoint._defaultUILocation) { - uiLocation = null; - } - } if (uiLocation) { this._uiLocations.set(liveLocation, uiLocation); diff --git a/front_end/bindings/DebuggerWorkspaceBinding.js b/front_end/bindings/DebuggerWorkspaceBinding.js index ee501ef36e..d4bdf3b915 100644 --- a/front_end/bindings/DebuggerWorkspaceBinding.js +++ b/front_end/bindings/DebuggerWorkspaceBinding.js @@ -192,7 +192,7 @@ export class DebuggerWorkspaceBinding { } } const modelData = this._debuggerModelToData.get(rawLocation.debuggerModel); - return modelData._rawLocationToUILocation(rawLocation); + return modelData ? modelData._rawLocationToUILocation(rawLocation) : null; } /** diff --git a/front_end/sources/TabbedEditorContainer.js b/front_end/sources/TabbedEditorContainer.js index fbc7df5fbd..5dc8bfdb6c 100644 --- a/front_end/sources/TabbedEditorContainer.js +++ b/front_end/sources/TabbedEditorContainer.js @@ -91,7 +91,7 @@ export class TabbedEditorContainer extends Common.ObjectWrapper.ObjectWrapper { this._previouslyViewedFilesSetting = setting; this._history = History.fromObject(this._previouslyViewedFilesSetting.get()); - this._historyUriToUISourceCode = new Map(); + this._uriToUISourceCode = new Map(); } /** @@ -188,7 +188,7 @@ export class TabbedEditorContainer extends Common.ObjectWrapper.ObjectWrapper { * @param {!Workspace.UISourceCode.UISourceCode} uiSourceCode */ showFile(uiSourceCode) { - this._innerShowFile(uiSourceCode, true); + this._innerShowFile(this._canonicalUISourceCode(uiSourceCode), true); } /** @@ -213,7 +213,7 @@ export class TabbedEditorContainer extends Common.ObjectWrapper.ObjectWrapper { const result = []; const uris = this._history._urls(); for (const uri of uris) { - const uiSourceCode = this._historyUriToUISourceCode.get(uri); + const uiSourceCode = this._uriToUISourceCode.get(uri); if (uiSourceCode) { result.push(uiSourceCode); } @@ -377,12 +377,33 @@ export class TabbedEditorContainer extends Common.ObjectWrapper.ObjectWrapper { } } + /** + * @param {!Workspace.UISourceCode.UISourceCode} uiSourceCode + * @return {!Workspace.UISourceCode.UISourceCode} + */ + _canonicalUISourceCode(uiSourceCode) { + // Check if we have already a UISourceCode for this url + if (this._uriToUISourceCode.has(uiSourceCode.url())) { + // Ignore incoming uiSourceCode, we already have this file. + return this._uriToUISourceCode.get(uiSourceCode.url()); + } + this._uriToUISourceCode.set(uiSourceCode.url(), uiSourceCode); + return uiSourceCode; + } + /** * @param {!Workspace.UISourceCode.UISourceCode} uiSourceCode */ addUISourceCode(uiSourceCode) { - const binding = self.Persistence.persistence.binding(uiSourceCode); - uiSourceCode = binding ? binding.fileSystem : uiSourceCode; + const canonicalSourceCode = this._canonicalUISourceCode(uiSourceCode); + const duplicated = canonicalSourceCode !== uiSourceCode; + const binding = self.Persistence.persistence.binding(canonicalSourceCode); + uiSourceCode = binding ? binding.fileSystem : canonicalSourceCode; + + if (duplicated) { + uiSourceCode.disableEdit(); + } + if (this._currentFile === uiSourceCode) { return; } @@ -393,12 +414,6 @@ export class TabbedEditorContainer extends Common.ObjectWrapper.ObjectWrapper { return; } - // Check if we have already opened a tab for this uri.... - if (this._historyUriToUISourceCode.has(uiSourceCode.url())) { - return; - } - this._historyUriToUISourceCode.set(uiSourceCode.url(), uiSourceCode); - if (!this._tabIds.has(uiSourceCode)) { this._appendFileTab(uiSourceCode, false); } @@ -437,8 +452,8 @@ export class TabbedEditorContainer extends Common.ObjectWrapper.ObjectWrapper { if (tabId) { tabIds.push(tabId); } - if (this._historyUriToUISourceCode.get(uiSourceCode.url()) === uiSourceCode) { - this._historyUriToUISourceCode.delete(uiSourceCode.url()); + if (this._uriToUISourceCode.get(uiSourceCode.url()) === uiSourceCode) { + this._uriToUISourceCode.delete(uiSourceCode.url()); } } this._tabbedPane.closeTabs(tabIds); diff --git a/front_end/sources/UISourceCodeFrame.js b/front_end/sources/UISourceCodeFrame.js index 92afa8b0e0..de810a6d0f 100644 --- a/front_end/sources/UISourceCodeFrame.js +++ b/front_end/sources/UISourceCodeFrame.js @@ -242,6 +242,9 @@ export class UISourceCodeFrame extends SourceFrame.SourceFrame.SourceFrameImpl { if (this.hasLoadError()) { return false; } + if (this._uiSourceCode.editDisabled()) { + return false; + } if (self.Persistence.persistence.binding(this._uiSourceCode)) { return true; } diff --git a/front_end/workspace/UISourceCode.js b/front_end/workspace/UISourceCode.js index 92b35ecdf0..e4c77252f8 100644 --- a/front_end/workspace/UISourceCode.js +++ b/front_end/workspace/UISourceCode.js @@ -83,6 +83,7 @@ export class UISourceCode extends Common.ObjectWrapper.ObjectWrapper { this._workingCopy = null; /** @type {?function() : string} */ this._workingCopyGetter = null; + this._disableEdit = false; } /** @@ -628,6 +629,17 @@ export class UISourceCode extends Common.ObjectWrapper.ObjectWrapper { decorationsForType(type) { return this._decorations ? this._decorations.get(type) : null; } + + disableEdit() { + this._disableEdit = true; + } + + /** + * @return {boolean} + */ + editDisabled() { + return this._disableEdit; + } } /** @enum {symbol} */ diff --git a/test/e2e/sources/script-in-multiple-workers.ts b/test/e2e/sources/script-in-multiple-workers.ts index d4b4312774..0bbc609581 100644 --- a/test/e2e/sources/script-in-multiple-workers.ts +++ b/test/e2e/sources/script-in-multiple-workers.ts @@ -15,18 +15,6 @@ async function validateSourceTabs() { assert.deepEqual(await getOpenSources(), ['multi-workers.js']); } -async function validateBreakpoints(frontend: puppeteer.Page) { - assert.deepEqual(await getBreakpointDecorators(frontend), [6, 10]); - assert.deepEqual(await getBreakpointDecorators(frontend, true), [6]); -} - -async function validateBreakpointsWithoutDisabled(frontend: puppeteer.Page) { - // Currently breakpoints do not get copied to workers if they are disabled. - // This behavior is enforced by a web test, which this test will replace. - // TODO(leese): Once breakpoint copying issue is fixed, remove this function. - assert.deepEqual(await getBreakpointDecorators(frontend), [10]); - assert.deepEqual(await getBreakpointDecorators(frontend, true), []); -} describe('Multi-Workers', async () => { beforeEach(async () => { @@ -48,6 +36,13 @@ describe('Multi-Workers', async () => { await waitFor(workerFileSelectors(10).rootSelector); } + async function validateBreakpoints(frontend: puppeteer.Page) { + // TODO(crbug.com/1062308): Fix the source map so the breakpoint at 10 doesn't move + const bpLine = sourceMaps ? 9 : 10; + assert.deepEqual(await getBreakpointDecorators(frontend), [6, bpLine]); + assert.deepEqual(await getBreakpointDecorators(frontend, true), [6]); + } + it(`loads scripts exactly once on reload ${withOrWithout}`, async () => { const {target} = getBrowserAndPages(); @@ -71,8 +66,7 @@ describe('Multi-Workers', async () => { await validateSourceTabs(); }); - // TODO(leese): Enable once chromium:670180 is fixed. - it.skip(`[crbug.com/670180]loads scripts exactly once on break ${withOrWithout}`, async () => { + it(`loads scripts exactly once on break ${withOrWithout}`, async () => { const {target, frontend} = getBrowserAndPages(); // Have the target load the page. @@ -114,57 +108,54 @@ describe('Multi-Workers', async () => { await validateSourceTabs(); }); - // TODO(leese): Enable with source maps once chromium:670180 is fixed. - if (!sourceMaps) { - it(`copies breakpoints between workers ${withOrWithout}`, async () => { - const {target, frontend} = getBrowserAndPages(); + it(`copies breakpoints between workers ${withOrWithout}`, async () => { + const {target, frontend} = getBrowserAndPages(); - // Have the target load the page. - await target.goto(targetPage); + // Have the target load the page. + await target.goto(targetPage); - await click('#tab-sources'); - // Wait for all workers to load - await validateNavigationTree(); - // Open file from second worker - await openNestedWorkerFile(workerFileSelectors(2)); - // Set two breakpoints - await addBreakpointForLine(frontend, 6); - // Disable first breakpoint - const bpEntry = await waitFor('.breakpoint-entry'); - const bpCheckbox = await waitFor('input', bpEntry); - if (!bpCheckbox) { - assert.fail('Could not find checkbox to disable breakpoint'); - return; - } - await bpCheckbox.evaluate(n => (n as HTMLElement).click()); - await frontend.waitFor('.cm-breakpoint-disabled'); - // Add another breakpoint - await addBreakpointForLine(frontend, 10); + await click('#tab-sources'); + // Wait for all workers to load + await validateNavigationTree(); + // Open file from second worker + await openNestedWorkerFile(workerFileSelectors(2)); + // Set two breakpoints + await addBreakpointForLine(frontend, 6); + // Disable first breakpoint + const bpEntry = await waitFor('.breakpoint-entry'); + const bpCheckbox = await waitFor('input', bpEntry); + if (!bpCheckbox) { + assert.fail('Could not find checkbox to disable breakpoint'); + return; + } + await bpCheckbox.evaluate(n => (n as HTMLElement).click()); + await frontend.waitFor('.cm-breakpoint-disabled'); + // Add another breakpoint + await addBreakpointForLine(frontend, 10); - // Check breakpoints - await validateBreakpoints(frontend); + // Check breakpoints + await validateBreakpoints(frontend); - // Close tab - await click('[aria-label="Close multi-workers.js"]'); + // Close tab + await click('[aria-label="Close multi-workers.js"]'); - // Open different worker - await openNestedWorkerFile(workerFileSelectors(3)); + // Open different worker + await openNestedWorkerFile(workerFileSelectors(3)); - // Check breakpoints - await validateBreakpointsWithoutDisabled(frontend); + // Check breakpoints + await validateBreakpoints(frontend); - // Close tab - await click('[aria-label="Close multi-workers.js"]'); + // Close tab + await click('[aria-label="Close multi-workers.js"]'); - // Reload - await target.goto(targetPage); + // Reload + await target.goto(targetPage); - // Open different worker - await openNestedWorkerFile(workerFileSelectors(4)); + // Open different worker + await openNestedWorkerFile(workerFileSelectors(4)); - // Check breakpoints - await validateBreakpointsWithoutDisabled(frontend); - }); - } + // Check breakpoints + await validateBreakpoints(frontend); + }); }); });