Closed Bug 599494 Opened 10 years ago Closed 10 years ago

pageloader needs to log to stdout and have the ability to install in a profile directory as an extension


(Testing :: Talos, defect)

Not set


(Not tracked)



(Reporter: jmaher, Assigned: jmaher)



(Whiteboard: [mobile_unittests])


(1 file, 2 obsolete files)

in order to work well in android automation, we need to install extensions in the profile and write all output to a direct file, not stdout.
Attachment #478398 - Flags: review?(vladimir)
Whiteboard: [mobile_unittests]
Comment on attachment 478398 [details] [diff] [review]
log to stdout and bundle as an extension (1.0)

the install.rdf file is in the wrong place, no? other changes look fine, but who calls dumpLog?
slight update to cleanup the mozillafilelogger.js code.

To answer the questions in the previous comment:

install.rdf is in the root directory (where chrome.manifest is located at.)  This works for installing in a profile.  Please advise if you think we should put install.rdf in a different location.

for the dumpLog() call, it isn't really used.  pageloader already uses dumpLine() which is pretty much what dumpLog() did.  I removed dumpLog() since it wasn't used and kept my MozillaFileLogger.log() call inside of dumpLine().
Assignee: nobody → jmaher
Attachment #478398 - Attachment is obsolete: true
Attachment #478599 - Flags: review?(vladimir)
Attachment #478398 - Flags: review?(vladimir)
Attachment #478599 - Flags: review?(vladimir) → review+
Comment on attachment 478599 [details] [diff] [review]
log to stdout and bundle as an extension (1.1)

looks ok to me, vlad's review still welcome, obviously
Attachment #478599 - Flags: review?(vladimir)
Comment on attachment 478599 [details] [diff] [review]
log to stdout and bundle as an extension (1.1)

won't the call in dumpLine error out if MozillaFileLogger == null?  Or if that's fine, then why is there a check for it being non-null before calling close?

Also, I guess I don't understand where the root is of what this is patching; I assumed layout/tools/pageloader, but that doesn't seem to be the case?
I updated the patch to include a check for MozillaFileLogger on the log() call.  Thanks for the feedback on that.  

As for the root of this, here is where it will live:

Essentially this will be cloned into a profile directory similar to this:
cd profile/extensions
hg clone

If you would like me to change things to work with other locations of pageloader, let me know and I can figure it out.
Attachment #478599 - Attachment is obsolete: true
Attachment #479627 - Flags: review?(vladimir)
Attachment #478599 - Flags: review?(vladimir)
Hmmm.. so the problem is that I believe the pageloader that's checked into the tree is included with --enable-tests in the dist/bin dir.  So then we'll have an extension providing the same thing.  Could lead to problems, but even more of an issue, I don't like that we have a fork.  Cc'ing dbaron, who owns the in-tree layout tools stuff (I believe?).  Is moving the code to build/pageloader for simplicity for releng?  Any chance you guys can just pull from the source tree?

I'm hesitant to suggest removing the in-tree version, since it's useful for people doing local testing without needing to pull a second repo...
I really don't know anything about the in-tree pageloader.
alice, can you comment on the vs mozilla-central in tree version?  I suspect it has to do with supporting all branches?
The version in build/pageloader is a bundle for bug 580698.  We currently install the bundle version for all talos tests - the version in the mozilla source tree is essentially ignored.
Also - the bundle is provided as part of standalone talos - are there any other pageloader tests that are run without talos?
:vlad, are there any other concerns you have with landing this patch on the hg.m.o/build/pageloader repository?
There has been no activity on this bug since last Thursday, can we resolve this today?
Is there a chance we can get this reviewed or acted on - I would like to wrap up our initial list of tests and this one is blocking
landed in hg repo:

we still need to add this to the list of releng downtime tasks.
Closed: 10 years ago
Resolution: --- → FIXED
Blocks: 602055
You need to log in before you can comment on or make changes to this bug.