897417 - Add metrofx browser chrome talos test support to pageloader

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Jim Mathies [:jimm]]

See bug 773817, pageloader currently assumes it can create desktop windows. We need to be able to load pageloader in a background tab to do chrome talos testing.

Comment 1

[Jim Mathies [:jimm]]

Attachment: talos patch v.1

Comment 2

[Jim Mathies [:jimm]]

Attachment: browser patch v.1

Comment 3

[Jim Mathies [:jimm]]

Comment on attachment 780375 [details] [diff] [review]
browser patch v.1

I think test specific stuff like this is ok provided it doesn't ad overhead. mbrubeck, curious what you think?

This comes at us from one of two places - 

normal browser startup - 

http://mxr.mozilla.org/mozilla-central/source/browser/metro/components/BrowserCLH.js#239

and talos pageloader command line processor - 

http://mxr.mozilla.org/build/source/talos/talos/pageloader/components/tp-cmdline.js#103

which I've modified slightly in the patch posted here.

Comment 4

[Matt Brubeck (:mbrubeck)]

Comment on attachment 780375 [details] [diff] [review]
browser patch v.1

Review of attachment 780375 [details] [diff] [review]:
-----------------------------------------------------------------

r=mbrubeck, but please remove getBrowserObject unless it's actually required.

::: browser/metro/base/content/browser.js
@@ +32,5 @@
> + * Returns the Browser object.
> + */
> +function getBrowserObject() {
> +  return Browser;
> +}

Why is this function useful?  Can't we just use chromewin.Browser instead of chromewin.getBrowserObject()?

Comment 5

[Jim Mathies [:jimm]]

(In reply to Matt Brubeck (:mbrubeck) from comment #4)
> Why is this function useful?  Can't we just use chromewin.Browser instead of
> chromewin.getBrowserObject()?

I think I tried that but maybe I was on the wrong window object. Will test a bit, if you say it'll work, I'm sure it will. :)

Comment 6

[Jim Mathies [:jimm]]

Comment on attachment 780373 [details] [diff] [review]
talos patch v.1

I can't really test these changes aside from local runs. Can I hand this off to releng from here jmaher? Be happy to help test or whatever help is needed.

Comment 7

[Jim Mathies [:jimm]]

Attachment: talos patch v.2

Updated per mbrubeck's requested changes.

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 780911 [details] [diff] [review]
talos patch v.2

Review of attachment 780911 [details] [diff] [review]:
-----------------------------------------------------------------

this looks fine, let me test a bit more in depth on mobile and other configs before landing it.

Comment 9

[Jim Mathies [:jimm]]

metro browser.js part:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e8ef43506a4d

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e8ef43506a4d

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

this patch for talos doesn't seem to cause any problems with other tests on all platforms.  

I did the f+, are there other changes needed, would you like to get it checked in?  I plan to update talos on Monday or Tuesday on the trees.

Comment 12

[Jim Mathies [:jimm]]

(In reply to Joel Maher (:jmaher) from comment #11)
> this patch for talos doesn't seem to cause any problems with other tests on
> all platforms.  
> 
> I did the f+, are there other changes needed, would you like to get it
> checked in?  I plan to update talos on Monday or Tuesday on the trees.

Nope, this should do it.

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

talos changes landed:
https://hg.mozilla.org/build/talos/rev/0c96f76c9a26