Create API to run regression tests, dump frames, content etc

RESOLVED FIXED in mozilla1.4beta

Status

()

Core
Layout
P2
normal
RESOLVED FIXED
16 years ago
15 years ago

People

(Reporter: Kevin McCluskey (gone), Assigned: Simon Fraser)

Tracking

({topembed+})

Trunk
mozilla1.4beta
topembed+
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [has review] [whitebox])

Attachments

(3 attachments, 5 obsolete attachments)

(Reporter)

Description

16 years ago
Replace viewer with a plugin which can be installed into any Gecko layout
engine. The plugin will provide basic functions for dumping the frame model and
comparing frame models. The control logic for cycling through test pages will be
provided by a web page which contains JavaScript which makes calls to the
plugin. The plugin should also provide calls for dumping the frame, content, and
view tree's to provide a complete replacement for viewer

Updated

16 years ago
Priority: -- → P2

Comment 1

16 years ago
What's the progress on this? 
We're talking about an XPCOM component here and not a 4x style plugin, right?

Comment 3

16 years ago
Yes.. the debugging part is a service, access to this sevice is thru a plugin or
XUL.  The plugin and service are both finished if you have a debug build.  I am
going to start working on getting this working for release builds now.  You can
find all the source and build this in Mozilla/extensions/layout-debug. 
Currently the way to build this is to go into that directory and do an nmake -f
makefile.win and then go into the plugin folder and do an nmake -f makefile.win.
 I will combine these to in a day or so.  I also need to make a gmake build for
linux and mac which I have not done yet.  I will get to those within this week.
(Reporter)

Updated

16 years ago
Keywords: nsbeta1+
Target Milestone: --- → mozilla1.2beta

Comment 4

16 years ago
Don - what's the latest ETA on this - to be in the commerical build?

Comment 5

16 years ago
batch: adding topembed per Gecko2 document
http://rocknroll.mcom.com/users/marek/publish/Gecko/Gecko2Tasks.html
Keywords: topembed

Comment 6

16 years ago
great thing - eager to contribute!

Comment 7

16 years ago
Created attachment 99978 [details] [diff] [review]
Cleanup of the current plugin code.  Really simplifies.

Comment 8

16 years ago
Comment on attachment 99978 [details] [diff] [review]
Cleanup of the current plugin code.  Really simplifies.

r=rods
Attachment #99978 - Flags: review+

Updated

16 years ago
Keywords: topembed → topembed+

Updated

16 years ago
QA Contact: petersen → praveenqa

Updated

16 years ago
Blocks: 176349

Comment 9

16 years ago
Can we get this into for 1.3b ?
Whiteboard: [has review]

Updated

16 years ago
Whiteboard: [has review] → [has review] [whitebox]
(Reporter)

Comment 10

16 years ago
-> peterl
Assignee: dcone → peterl
Target Milestone: mozilla1.2beta → mozilla1.4alpha
(Assignee)

Comment 11

16 years ago
Taking. This isn't going to be a plugin.
Assignee: peterl → sfraser
Summary: Create plugin to run regression tests, dump frames, content etc → Create API to run regression tests, dump frames, content etc
(Assignee)

Comment 12

16 years ago
Created attachment 111889 [details] [diff] [review]
WIP patch, extends nsIDebugObject
Attachment #99978 - Attachment is obsolete: true
(Assignee)

Comment 13

16 years ago
Created attachment 111890 [details] [diff] [review]
Some html/js for layout/tools/tests that drive nsIDebugObject

Comment 14

16 years ago
Simon, does that also include the printing regression tests? I will send out a
beer if you fix that in a way  that those linux printing guys who love to break
those tests so frequently will be stopped.
(Assignee)

Comment 15

16 years ago
I haven't done anything specific to printing tests. Would it be possible to get
layout to dump frame models for printing output? I'm not sure.
(Assignee)

Comment 16

16 years ago
Created attachment 112215 [details] [diff] [review]
Patch fills out nsDebugObject impl

This patch fleshes out nsIDebugObject and its impl to add methods for dumping
frames, content, views, and webshells for a given nsIDOMWindow.

I also changed the impl so that passing a null nsIFile destination will dump
data to stdout.
Attachment #111889 - Attachment is obsolete: true
(Assignee)

Comment 17

16 years ago
Patch is ready for r/sr stamping.

I decided that keeping this code in a separate component was a good thing, to
avoid adding debug-only dependencies to the layout module.
Status: NEW → ASSIGNED

Comment 18

16 years ago
any ideas on the footprint win?
(Reporter)

Updated

16 years ago
Blocks: 17027
(Reporter)

Comment 19

16 years ago
Once this lands we will no longer have to link against the native implementation
for nsLabel, nsButton, nsCheckButton, nsTextWidget on Linux (bug 17027). I'm not
sure what the fooprint reduction will be, it depends on how big the GTK
implementation of these are. I assume nsTextWidget will provide the largest
reduction.
(Assignee)

Updated

16 years ago
Attachment #112215 - Flags: superreview?(bryner)
Attachment #112215 - Flags: review?(peterl)

Comment 20

16 years ago
Comment on attachment 112215 [details] [diff] [review]
Patch fills out nsDebugObject impl

Does changing the browser.underline_anchors pref cause a reframe or does it
reload the document's DOM? Can you note that in a comment.

Is nsIDebugObject::createDirectory really needed since nsIFile can do that?
Attachment #112215 - Flags: review?(peterl) → review+
(Assignee)

Comment 21

16 years ago
> Does changing the browser.underline_anchors pref cause a reframe or does it
> reload the document's DOM? Can you note that in a comment.

It changes some style rules and does a reflow.

> Is nsIDebugObject::createDirectory really needed since nsIFile can do that?

Not really, but it was there before so I didn't remove it. I can.
Comment on attachment 112215 [details] [diff] [review]
Patch fills out nsDebugObject impl

>Index: idl/nsIDebugObject.idl
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/layout-debug/idl/nsIDebugObject.idl,v
>retrieving revision 1.6
>diff -b -u -8 -r1.6 nsIDebugObject.idl
>--- idl/nsIDebugObject.idl	4 Dec 2002 22:11:35 -0000	1.6
>+++ idl/nsIDebugObject.idl	22 Jan 2003 00:20:29 -0000
>@@ -34,44 +34,71 @@
>  * and other provisions required by the GPL or the LGPL. If you do not delete
>  * the provisions above, a recipient may use your version of this file under
>  * the terms of any one of the NPL, the GPL or the LGPL.
>  *
>  * ***** END LICENSE BLOCK ***** */
> 
> #include "nsISupports.idl"
> 
>+interface nsIDOMWindow;
>+interface nsIFile;
>+
> [scriptable, uuid(1B4CD090-0531-11d6-A876-00105A183419)]
> interface nsIDebugObject : nsISupports
> {
> 
>   /**
>-   * creates a directory.. only locally
>-   * @param 
>+   * creates a local directory
>+   * @param aDirectoryPath a full path to the directory (not a URL)
>    * @param 

Could you remove these empty @param comments? (there are others too)


>+  void dumpRegressionData(in nsIDOMWindow aWindowToDump,in nsIFile aFile, in unsigned long aFlagsMask, [retval] out long aResult);

can't this just return a long?

> 
> %{ C++
> 
> %}

Remove this useless cruft at the end.

>Index: src/nsDebugObject.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/layout-debug/src/nsDebugObject.cpp,v
>retrieving revision 1.10
>diff -b -u -8 -r1.10 nsDebugObject.cpp
>--- src/nsDebugObject.cpp	8 Jan 2003 20:10:25 -0000	1.10
>+++ src/nsDebugObject.cpp	22 Jan 2003 00:20:30 -0000
>@@ -42,36 +42,44 @@
> 
> #include "nsXPIDLString.h"
> #include "nsReadableUtils.h"
> #include "nsIFileSpec.h"
> #include "nsIWindowWatcher.h"
> #include "nsVoidArray.h"
> #include "prmem.h"
> #include "nsIDocShellTreeItem.h"
>+#include "nsIDocShellTreeNode.h"
> #include "nsIDOMWindowInternal.h"
> #include "nsIPresShell.h"
>+#include "nsIDocument.h"
> #include "nsIDOMDocument.h"
> #include "nsIURI.h"
> #include "nsIDOMHTMLDocument.h"
> #include "nsISimpleEnumerator.h"
> #include "nsIScriptGlobalObject.h"
> #include "nsIDocShell.h"
> #include "nsIFrameDebug.h"
> #include "nsIFrame.h"
> #include "nsStyleStruct.h"
> #include "nsIFrameUtil.h"
> #include "nsLayoutCID.h"
> #include "nsNetUtil.h"
> #include "nsIFile.h"
>+#include "nsIPrefService.h"
>+#include "nsIViewManager.h"
>+#include "nsIView.h"
>+#include "nsIStyleSet.h"
>+
> 
> NS_IMPL_ISUPPORTS1(nsDebugObject, nsIDebugObject)
> 
> static NS_DEFINE_IID(kFrameUtilCID, NS_FRAME_UTIL_CID);

can you fix this to be NS_DEFINE_CID while you're here?

> /** ---------------------------------------------------
>  *  See documentation in nsDebugObject.h
>  *	@update 5/16/02 dwc
>  */
> NS_IMETHODIMP
>-nsDebugObject::OutputTextToFile(PRBool aNewFile, const PRUnichar *aFilePath, const PRUnichar *aFileName, const PRUnichar *aOutputString) 
>+nsDebugObject::OutputTextToFile(nsIFile *aFile, PRBool aTruncateFile, const char *aOutputString) 
> {
>-  nsresult      result = NS_ERROR_FAILURE;
>-  nsCAutoString outputPath;
>-  outputPath.AssignWithConversion(aFilePath);
>-  outputPath.AppendWithConversion(aFileName);
>-  char* filePath = ToNewCString(outputPath);
>+  NS_ENSURE_ARG(aFile);
>+  NS_ENSURE_ARG(aOutputString);
>+  
>+  const char* options = (aTruncateFile) ? "wt" : "at";
>   FILE* fp;
>   
>-  if ( aNewFile ) {
>-    fp = fopen(filePath, "wt");
>-  } else {
>-    fp = fopen(filePath, "at");
>-  }
>+  nsCOMPtr<nsILocalFile> localFile(do_QueryInterface(aFile));
>+  if (!localFile) return NS_ERROR_NO_INTERFACE;

Could this function just take a nsILocalFile* as an argument instead of
nsIFile* ?  Same applies to the rest of the Dump* functions.

>+/* void dumpContent (in nsIDOMWindow aWindow, in nsIFile aDestFile); */
>+NS_IMETHODIMP
>+nsDebugObject::DumpContent(nsIDOMWindow *aWindow, nsIFile *aDestFile)
>+{
>+  NS_ENSURE_ARG(aWindow);
>+
>+  nsCOMPtr<nsIDocShell> docShell;
>+  nsresult rv = GetDocShellFromWindow(aWindow, getter_AddRefs(docShell));
>+  if (NS_FAILED(rv)) return rv;
>+
>+  FILE* fp = stdout;
>+  if (aDestFile)
>+  {
>+    nsCOMPtr<nsILocalFile> localFile(do_QueryInterface(aDestFile));
>+    if (!localFile) return NS_ERROR_NO_INTERFACE;
>+    rv = localFile->OpenANSIFileDesc("w", &fp);
>+    if (NS_FAILED(rv)) return rv;
>         }

indenting looks off.

>+/* void dumpReflowStats (in nsIDOMWindow aWindow, in nsIFile aDestFile); */
>+NS_IMETHODIMP
>+nsDebugObject::DumpReflowStats(nsIDOMWindow *aWindow, nsIFile* /* aDestFile */)
>+{
>+  NS_ENSURE_ARG(aWindow);
>+
>+  nsCOMPtr<nsIPresShell> presShell;
>+  nsresult rv = GetPresShellFromWindow(aWindow, getter_AddRefs(presShell));
>+  if (NS_FAILED(rv)) return rv;
>+
>+#ifdef MOZ_REFLOW_PERF
>+  presShell->DumpReflows();
>+#else
>+  fprintf(stdout, "***********************************\n");
>+  fprintf(stdout, "Sorry, you have built with MOZ_REFLOW_PERF=1\n");

should this be _haven't_ built?

>+  fprintf(stdout, "***********************************\n");
>+#endif
>+
>+  return  NS_OK;
> }
> 
> /** ---------------------------------------------------
>  *  See documentation in nsDebugObject.h
>  *	@update 5/16/02 dwc
>  */
> NS_IMETHODIMP
>-nsDebugObject::CompareFrameModels(const PRUnichar *aBasePath, const PRUnichar *aVerifyPath,
>-            const PRUnichar *aBaseLineFileName, const PRUnichar *aVerifyFileName, PRUint32 aFlags) 
>+nsDebugObject::CompareFrameModels(nsIFile *aBaseFile, nsIFile *aVerFile, PRUint32 aFlags, PRInt32 *aResult) 
> {
...
>+  FILE* verFile;
>+  rv = localVer->OpenANSIFileDesc("r", &verFile);
>+  if (NS_FAILED(rv)) {
>+    fclose(baseFile);
>+    return rv;
>     }

indenting here.

>+nsresult
>+nsDebugObject::RefreshAllWindows()
>+{
>+  nsresult rv;
>+  // hack. Toggle the underline links pref to get stuff to redisplay
>+  nsCOMPtr<nsIPrefBranch> prefBranch = do_GetService(NS_PREFSERVICE_CONTRACTID);
>+  if (prefBranch)
>+  {
>+    PRBool underlineLinksPref;
>+    rv = prefBranch->GetBoolPref("browser.underline_anchors", &underlineLinksPref);
>+    if (NS_SUCCEEDED(rv))
>+    {
>+      prefBranch->SetBoolPref("browser.underline_anchors", !underlineLinksPref);
>+      prefBranch->SetBoolPref("browser.underline_anchors", underlineLinksPref);
>+    }
>+  }
>+  return NS_OK;

Maybe call nsIPresContext::ClearStyleDataAndReflow() instead? (that's what it
does in response to this pref, more or less).

>+}
>+
>+
>+nsresult
>+nsDebugObject::GetDocShellFromWindow(nsIDOMWindow* inWindow, nsIDocShell** outShell)
>+{
>+  nsCOMPtr<nsIDOMWindowInternal> theInternWindow = do_QueryInterface(inWindow);
>+  if (!theInternWindow)
>+    return NS_ERROR_FAILURE;
>+  
>+  nsCOMPtr<nsIScriptGlobalObject> scriptObj(do_QueryInterface(theInternWindow));
>+  if (!scriptObj) return NS_ERROR_FAILURE;

Why not just QI directly to nsIScriptGlobalObject?
Attachment #112215 - Flags: superreview?(bryner) → superreview-
(Assignee)

Comment 23

16 years ago
Created attachment 116265 [details] [diff] [review]
New patch; addresses comments, renames module and API

This patch addresses the above comments, other than bryner's suggestion to use
a method on nsIPresContext (rather than flipping prefs) to get the windows to
refresh. Doing that would require enumerating all open windows.

I removed a bunch of unimplemented methods related to printing on
nsIDebugOject.

This patch also renames nsIDebugObject to nsIFrameDebugObject, and changes the
module name from 'debug' to 'layout_debug'.
Attachment #112215 - Attachment is obsolete: true

Comment 24

16 years ago
it would be nice to get this before 1.4alpha if possible...  what are the chances?
(Assignee)

Comment 25

15 years ago
Created attachment 119218 [details] [diff] [review]
Revised version of last patch, changed to use nsILocalFiles in the API
Attachment #116265 - Attachment is obsolete: true
(Assignee)

Comment 26

15 years ago
Created attachment 119219 [details] [diff] [review]
Final answer: idl methods changed to return long, rather than using [retval
(Assignee)

Comment 27

15 years ago
Created attachment 119220 [details] [diff] [review]
Final answer: idl methods changed to return long, rather than using [retval]
Attachment #119218 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #119220 - Flags: superreview?(bryner)
Attachment #119220 - Flags: superreview?(bryner) → superreview+
(Assignee)

Updated

15 years ago
Target Milestone: mozilla1.4alpha → mozilla1.4beta
(Assignee)

Comment 28

15 years ago
Checked in, and old debug plugin files removed.
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
So how does this affect the process of running regression tests, if at all?
(Assignee)

Comment 30

15 years ago
I'll post some docs on mozilla.org, and reference here.
See the links to such instructions in http://www.mozilla.org/newlayout/doc/ --
you may want to put whatever you put up in the same dir and link it from there....

Updated

15 years ago
Blocks: 213938
You need to log in before you can comment on or make changes to this bug.