Closed Bug 572618 Opened 9 years ago Closed 9 years ago

Make debugging the editor easier

Categories

(Core :: Editor, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b1

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(4 files, 5 obsolete files)

It's been a long time that I wanted to do this.  We can use warning messages to make debugging the editor easier, by switching to using macros like NS_ENSURE_*.  This can be done using automatic search and replacement.  It won't change any behavior, but would make reading the code easier as an added bonus.
This is the result of this command:

find . -type f -name \*.cpp -or -name \*.h | xargs perl -pi -e 's/if\s+\(NS_FAILED\(([^)]+)\)\)\s+return\s+([^;]+)/NS_ENSURE_SUCCESS(\1, \2)/g'
Attachment #451846 - Flags: review?(roc)
This is the result of this command:

find . -type f -name \*.cpp -or -name \*.h | xargs perl -pi -e 's/if\s+\(!([^)]+)\)\s+return\s+([^;]+)/NS_ENSURE_TRUE(\1, \2)/g'

with two manual tweaks in nsHTMLDataTransfer.cpp because in two places in that file, the |if| in question had an |else|.
Attachment #451849 - Flags: review?(roc)
This is the result of this command:

find . -type f -name \*.cpp -or -name \*.h | xargs perl -pi -e 'undef $/; s/if\s+\(!([^)]+)\)\s*\n\s*return\s+([^;]+)/NS_ENSURE_TRUE(\1, \2)/gms'

with one manual fix in nsHTMLEditor.cpp of this pattern:

  if (foo)
// if (bar)
    return NS_ERROR_baz;
Attachment #451857 - Flags: review?(roc)
This is the result of this command:

find . -type f -name \*.cpp -or -name \*.h | xargs perl -pi -e 'undef $/; s/if\s+\(NS_FAILED\(([^)]+)\)\)\s*\n\s*return\s+([^;]+)/NS_ENSURE_SUCCESS(\1, \2)/g'
Attachment #451859 - Flags: review?(roc)
Handle compound conditions correctly.

For example, handle constructs such as the following correctly:

-  if (!foo || !bar) return baz;
+  NS_ENSURE_TRUE(foo && bar, baz);

Interdiff with the previous version of part 2: http://pastebin.mozilla.org/736447
Attachment #451849 - Attachment is obsolete: true
Attachment #451869 - Flags: review?(roc)
Attachment #451849 - Flags: review?(roc)
I had to update these patches when I removed the patch to bug 572513 from my queue.  I'll attach the new versions.
Attachment #451846 - Attachment is obsolete: true
Attachment #452065 - Flags: review?(roc)
Attachment #451846 - Flags: review?(roc)
Attachment #451857 - Attachment is obsolete: true
Attachment #452068 - Flags: review?(roc)
Attachment #451857 - Flags: review?(roc)
Attachment #451859 - Attachment is obsolete: true
Attachment #452070 - Flags: review?(roc)
Attachment #451859 - Flags: review?(roc)
I backed out one of the changes:
http://hg.mozilla.org/mozilla-central/rev/5332eb29d243
because it was causing huge amounts of debug spew, causing some tinderbox builds to fail due to huge log size:
WARNING: NS_ENSURE_TRUE(mRedoStack) failed: file e:/builds/moz2_slave/mozilla-central-win32-debug/build/editor/txmgr/src/nsTransactionItem.cpp, line 337
Depends on: 575187
Depends on: 581166
Depends on: 583493
diff --git a/editor/libeditor/base/nsEditorEventListener.cpp b/editor/libeditor/base/nsEditorEventListener.cpp
...
   nsCOMPtr<nsIDOMNode> sourceNode;
   dataTransferNS->GetMozSourceNode(getter_AddRefs(sourceNode));
-  if (!sourceNode)
-    return PR_TRUE;
+  NS_ENSURE_TRUE(sourceNode, PR_TRUE);

This case isn't an error condition. Was this change desired? It causes the warning to dump out continuously from an external location while dragging over a textbox.
(In reply to comment #13)
> diff --git a/editor/libeditor/base/nsEditorEventListener.cpp
> b/editor/libeditor/base/nsEditorEventListener.cpp
> ...
>    nsCOMPtr<nsIDOMNode> sourceNode;
>    dataTransferNS->GetMozSourceNode(getter_AddRefs(sourceNode));
> -  if (!sourceNode)
> -    return PR_TRUE;
> +  NS_ENSURE_TRUE(sourceNode, PR_TRUE);
> 
> This case isn't an error condition. Was this change desired? It causes the
> warning to dump out continuously from an external location while dragging over
> a textbox.

This was a bogus check.  I removed it in http://hg.mozilla.org/mozilla-central/rev/5f38f9a55345.  Thanks for catching this.
See Also: → 1184280
See Also: → 1184289
See Also: → 1184689
You need to log in before you can comment on or make changes to this bug.