Changeset 112064 in webkit

Timestamp:

Mar 26, 2012, 1:56:30 AM (15 years ago)

Author:

pfeldman@chromium.org

Message:

Web Inspector: breakpoints should be disabled while live editing JavaScript.
​https://bugs.webkit.org/show_bug.cgi?id=81947

Reviewed by Yury Semikhatsky.

Source/WebCore:

This change disables (removes from the backend) the breakpoints while editing script.
Undoing or committing the change restores the breakpoints. Main logic is implemented within
JavaScriptSourceFrame, but it required minor tweaking of the text editor: more correct
undo/redo wrapper and better detection of the noop opterations.

• inspector/front-end/JavaScriptSourceFrame.js:

(WebInspector.JavaScriptSourceFrame):
(WebInspector.JavaScriptSourceFrame.prototype.afterTextChanged):
(WebInspector.JavaScriptSourceFrame.prototype.beforeTextChanged):
(WebInspector.JavaScriptSourceFrame.prototype.didEditContent):
(WebInspector.JavaScriptSourceFrame.prototype.addBreakpoint):
(WebInspector.JavaScriptSourceFrame.prototype.removeBreakpoint):
(WebInspector.JavaScriptSourceFrame.prototype._onMouseDown):

• inspector/front-end/TextEditorModel.js:

(WebInspector.TextEditorModel.endsWithBracketRegex.):

• inspector/front-end/TextViewer.js:

(WebInspector.TextEditorMainPanel.prototype.handleUndoRedo.before):
(WebInspector.TextEditorMainPanel.prototype.handleUndoRedo.after):
(WebInspector.TextEditorMainPanel.prototype.handleUndoRedo):
(WebInspector.TextEditorMainPanel.prototype._applyDomUpdates):

LayoutTests:

• inspector/editor/highlighter-basics-expected.txt:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/inspector/editor/highlighter-basics-expected.txt (modified) (2 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/inspector/front-end/JavaScriptSourceFrame.js (modified) (8 diffs)
• Source/WebCore/inspector/front-end/TextEditorModel.js (modified) (7 diffs)
• Source/WebCore/inspector/front-end/TextViewer.js (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-03-26  Pavel Feldman  <pfeldman@chromium.org>
+
+        Web Inspector: breakpoints should be disabled while live editing JavaScript.
+        https://bugs.webkit.org/show_bug.cgi?id=81947
+
+        Reviewed by Yury Semikhatsky.
+
+        * inspector/editor/highlighter-basics-expected.txt:
+
 2012-03-26  Csaba Osztrogonác  <ossy@webkit.org>
 

trunk/LayoutTests/inspector/editor/highlighter-basics-expected.txt

@@ -34 +34 @@
 2 :  * line #1 : javascript-comment[0-10]
 3 :  * line #2 : javascript-comment[0-10]
-4 : /** : javascript-comment[0-10]
+4 : /** : null
 5 :  * line #1 : null
 6 :  * line #2 : null
@@ -45 +45 @@
 13 :  * var a = new A(); //*/ : null
 14 : some text : null
-15 :  * line #6 : null
+15 :  * line #6 : javascript-comment[0-10]
 16 :  * line #7 : javascript-comment[0-10]
 17 :  * line #8 : javascript-comment[0-10]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-03-26  Pavel Feldman  <pfeldman@chromium.org>
+
+        Web Inspector: breakpoints should be disabled while live editing JavaScript.
+        https://bugs.webkit.org/show_bug.cgi?id=81947
+
+        Reviewed by Yury Semikhatsky.
+
+        This change disables (removes from the backend) the breakpoints while editing script.
+        Undoing or committing the change restores the breakpoints. Main logic is implemented within
+        JavaScriptSourceFrame, but it required minor tweaking of the text editor: more correct
+        undo/redo wrapper and better detection of the noop opterations.
+
+        * inspector/front-end/JavaScriptSourceFrame.js:
+        (WebInspector.JavaScriptSourceFrame):
+        (WebInspector.JavaScriptSourceFrame.prototype.afterTextChanged):
+        (WebInspector.JavaScriptSourceFrame.prototype.beforeTextChanged):
+        (WebInspector.JavaScriptSourceFrame.prototype.didEditContent):
+        (WebInspector.JavaScriptSourceFrame.prototype.addBreakpoint):
+        (WebInspector.JavaScriptSourceFrame.prototype.removeBreakpoint):
+        (WebInspector.JavaScriptSourceFrame.prototype._onMouseDown):
+        * inspector/front-end/TextEditorModel.js:
+        (WebInspector.TextEditorModel.endsWithBracketRegex.):
+        * inspector/front-end/TextViewer.js:
+        (WebInspector.TextEditorMainPanel.prototype.handleUndoRedo.before):
+        (WebInspector.TextEditorMainPanel.prototype.handleUndoRedo.after):
+        (WebInspector.TextEditorMainPanel.prototype.handleUndoRedo):
+        (WebInspector.TextEditorMainPanel.prototype._applyDomUpdates):
+
 2012-03-25  Nat Duca  <nduca@chromium.org>
 

trunk/Source/WebCore/inspector/front-end/JavaScriptSourceFrame.js

@@ -41 +41 @@
     this._model = model;
     this._uiSourceCode = uiSourceCode;
-    this._breakpoints = {};
 
     WebInspector.SourceFrame.call(this, uiSourceCode.url);
@@ -111 +110 @@
             function addConditionalBreakpoint()
             {
-                this.addBreakpoint(lineNumber, true, true, true);
+                this.addBreakpoint(lineNumber, true, true, true, false);
                 function didEditBreakpointCondition(committed, condition)
                 {
@@ -160 +159 @@
     afterTextChanged: function(oldRange, newRange)
     {
-        if (!oldRange || !newRange)
-            return;
-
-        // Adjust execution line number.
-        if (typeof this._executionLineNumber === "number") {
-            var newExecutionLineNumber = this._lineNumberAfterEditing(this._executionLineNumber, oldRange, newRange);
-            this.clearExecutionLine();
-            this.setExecutionLine(newExecutionLineNumber);
-        }
-
-        // Adjust breakpoints.
-        var oldBreakpoints = this._breakpoints;
-        this._breakpoints = {};
-        for (var lineNumber in oldBreakpoints) {
-            lineNumber = Number(lineNumber);
-            var breakpoint = oldBreakpoints[lineNumber];
-            var newLineNumber = this._lineNumberAfterEditing(lineNumber, oldRange, newRange);
-            if (lineNumber === newLineNumber)
-                this._breakpoints[lineNumber] = breakpoint;
-            else {
-                this.removeBreakpoint(lineNumber);
-                this.addBreakpoint(newLineNumber, breakpoint.resolved, breakpoint.conditional, breakpoint.enabled);
-            }
-        }
-
-        this._scriptsPanel.setScriptSourceIsDirty(this._uiSourceCode, this.textModel.copyRange() !== this._originalContent);
+        var isDirty = this.textModel.copyRange() !== this._originalContent;
+        if (isDirty)
+            this._scriptsPanel.setScriptSourceIsDirty(this._uiSourceCode, true);
+        else
+            this.didEditContent(null, this._originalContent);
     },
 
     beforeTextChanged: function()
     {
-        if (!this._javaScriptSourceFrameState) {
-            this._javaScriptSourceFrameState = {
-                executionLineNumber: this._executionLineNumber,
-                breakpoints: this._breakpoints
-            }
-        }
+        if (!this._isDirty) {
+            // Disable all breakpoints in the model, store them as muted breakpoints.
+            for (var lineNumber = 0; lineNumber < this.textModel.linesCount; ++lineNumber) {
+                var breakpoint = this.textModel.getAttribute(lineNumber, "breakpoint");
+                if (breakpoint) {
+                    this._model.removeBreakpoint(this._uiSourceCode, lineNumber);
+                    // Re-adding decoration only.
+                    this.addBreakpoint(lineNumber, breakpoint.resolved, breakpoint.conditional, breakpoint.enabled, true); 
+                }
+            }
+        }
+
+        this._isDirty = true;
         WebInspector.SourceFrame.prototype.beforeTextChanged.call(this);
     },
@@ -202 +187 @@
     {
         delete this._editingContent;
+
         WebInspector.SourceFrame.prototype.didEditContent.call(this, error, content);
         if (error)
@@ -207 +193 @@
 
         this._originalContent = content;
-        var newBreakpoints = {};
-        for (var lineNumber in this._breakpoints) {
-            newBreakpoints[lineNumber] = this._breakpoints[lineNumber];
-            this.removeBreakpoint(Number(lineNumber));
-        }
-
-        for (var lineNumber in this._javaScriptSourceFrameState.breakpoints)
-            this._model.removeBreakpoint(this._uiSourceCode, Number(lineNumber));
-
-        for (var lineNumber in newBreakpoints) {
-            var breakpoint = newBreakpoints[lineNumber];
-            this._setBreakpoint(Number(lineNumber), breakpoint.condition, breakpoint.enabled);
+        this._isDirty = false;
+
+        // Restore all muted breakpoints.
+        for (var lineNumber = 0; lineNumber < this.textModel.linesCount; ++lineNumber) {
+            var breakpoint = this.textModel.getAttribute(lineNumber, "breakpoint");
+            if (breakpoint) {
+                // Remove fake decoration
+                this.removeBreakpoint(lineNumber);
+                // Set new breakpoint
+                this._setBreakpoint(lineNumber, breakpoint.condition, breakpoint.enabled);
+            }
         }
         this._scriptsPanel.setScriptSourceIsDirty(this._uiSourceCode, false);
-        delete this._javaScriptSourceFrameState;
     },
 
@@ -333 +317 @@
     },
 
-    addBreakpoint: function(lineNumber, resolved, conditional, enabled)
-    {
-        this._breakpoints[lineNumber] = {
+    addBreakpoint: function(lineNumber, resolved, conditional, enabled, mutedWhileEditing)
+    {
+        var breakpoint = {
             resolved: resolved,
             conditional: conditional,
             enabled: enabled
         };
+        this.textModel.setAttribute(lineNumber, "breakpoint", breakpoint);
+
         this.textViewer.beginUpdates();
         this.textViewer.addDecoration(lineNumber, "webkit-breakpoint");
-        if (!enabled)
+        if (!enabled || mutedWhileEditing)
             this.textViewer.addDecoration(lineNumber, "webkit-breakpoint-disabled");
         if (conditional)
@@ -351 +337 @@
     removeBreakpoint: function(lineNumber)
     {
-        delete this._breakpoints[lineNumber];
+        this.textModel.removeAttribute(lineNumber, "breakpoint");
         this.textViewer.beginUpdates();
         this.textViewer.removeDecoration(lineNumber, "webkit-breakpoint");
@@ -367 +353 @@
     _onMouseDown: function(event)
     {
+        if (this._isDirty)
+            return;
+
         if (event.button != 0 || event.altKey || event.ctrlKey || event.metaKey)
             return;

trunk/Source/WebCore/inspector/front-end/TextEditorModel.js

@@ -161 +161 @@
         } else {
             this._setLine(range.startLine, prefix + newLines[0]);
+
             for (var i = 1; i < newLines.length; ++i)
-                this._insertLine(range.startLine + i, newLines[i]);
+                this._lines.splice(range.startLine + i, 0, newLines[i]);
+            // Adjust attributes, attributes move with the first character of line.
+            var spliceParameters = new Array(newLines.length + 1); // 2 + number of items to insert.
+            spliceParameters[0] = range.startColumn ? range.startLine + 1 : range.startLine;
+            spliceParameters[1] = 0;
+            this._attributes.splice.apply(this._attributes, spliceParameters);
+
             this._setLine(range.startLine + newLines.length - 1, newLines[newLines.length - 1] + suffix);
             postCaret = newLines[newLines.length - 1].length;
         }
+
         return new WebInspector.TextRange(range.startLine, range.startColumn,
                                           range.startLine + newLines.length - 1, postCaret);
@@ -178 +186 @@
         var suffix = this._lines[range.endLine].substring(range.endColumn);
 
-        if (range.endLine > range.startLine)
-            this._removeLines(range.startLine + 1, range.endLine - range.startLine);
+        if (range.endLine > range.startLine) {
+            this._lines.splice(range.startLine + 1, range.endLine - range.startLine);
+            // Adjust attributes, attributes move with the first character of line.
+            this._attributes.splice(range.startColumn ? range.startLine + 1 : range.startLine, range.endLine - range.startLine);
+        }
         this._setLine(range.startLine, prefix + suffix);
     },
@@ -186 +197 @@
     {
         this._lines[lineNumber] = text;
-    },
-
-    _removeLines: function(fromLine, count)
-    {
-        this._lines.splice(fromLine, count);
-        this._attributes.splice(fromLine, count);
-    },
-
-    _insertLine: function(lineNumber, text)
-    {
-        this._lines.splice(lineNumber, 0, text);
-        this._attributes.splice(lineNumber, 0, {});
     },
 
@@ -283 +282 @@
     },
 
-    undo: function(callback)
+    undo: function(beforeCallback, afterCallback)
     {
         this._markRedoableState();
 
         this._inUndo = true;
-        var range = this._doUndo(this._undoStack, callback);
+        var range = this._doUndo(this._undoStack, beforeCallback, afterCallback);
         delete this._inUndo;
 
@@ -294 +293 @@
     },
 
-    redo: function(callback)
+    redo: function(beforeCallback, afterCallback)
     {
         this.markUndoableState();
 
         this._inRedo = true;
-        var range = this._doUndo(this._redoStack, callback);
+        var range = this._doUndo(this._redoStack, beforeCallback, afterCallback);
         delete this._inRedo;
 
@@ -305 +304 @@
     },
 
-    _doUndo: function(stack, callback)
+    _doUndo: function(stack, beforeCallback, afterCallback)
     {
         var range = null;
@@ -312 +311 @@
             stack.length = i;
 
+            if (beforeCallback)
+                beforeCallback();
+
             range = this.setText(command.range, command.text);
-            if (callback)
-                callback(command.range, range);
+
+            if (afterCallback)
+                afterCallback(command.range, range);
+
             if (i > 0 && stack[i - 1].explicit)
                 return range;

trunk/Source/WebCore/inspector/front-end/TextViewer.js

@@ -1070 +1070 @@
 
         this.beginUpdates();
-        this._enterTextChangeMode();
-
-        function callback(oldRange, newRange)
+
+        function before()
+        {
+            this._enterTextChangeMode();
+        }
+
+        function after(oldRange, newRange)
         {
             this._exitTextChangeMode(oldRange, newRange);
-            this._enterTextChangeMode();
-        }
-        var range = redo ? this._textModel.redo(callback.bind(this)) : this._textModel.undo(callback.bind(this));
-        this._exitTextChangeMode(null, null);
+        }
+
+        var range = redo ? this._textModel.redo(before.bind(this), after.bind(this)) : this._textModel.undo(before.bind(this), after.bind(this));
+
         this.endUpdates();
 
@@ -1755 +1759 @@
         }
 
-        // This is a "foreign" call outside of this class. Should be before we delete the dirty lines flag.
-        this._enterTextChangeMode();
-
         var dirtyLines = this._dirtyLines;
-        delete this._dirtyLines;
 
         var firstChunkNumber = this._chunkNumberForLine(dirtyLines.start);
@@ -1837 +1837 @@
             var oldRange = new WebInspector.TextRange(startLine, startColumn, endLine - 1, endColumn);
 
-        var newRange = this._setText(oldRange, lines.join("\n"));
+        var newContent = lines.join("\n");
+        if (this._textModel.copyRange(oldRange) === newContent) {
+            delete this._dirtyLines;
+            return; // Noop
+        }
+
+        // This is a "foreign" call outside of this class. Should be before we delete the dirty lines flag.
+        this._enterTextChangeMode();
+
+        delete this._dirtyLines;
+
+        var newRange = this._setText(oldRange, newContent);
 
         this._paintScheduledLines(true);