Made Print Preview tests autogenerated from print_preview.js.

Made Print Preview tests autogenerated from print_preview.js.

Now that the generator exists, make the print preview tests use it as well.  Since the test was flaky when I found it, porting FLAKY_ to the javascript test name.

Since Print Preview needs the preloading, provided a way to tell the generator to not AddLibrary.

BUG=82437
R=thestig@chromium.org,dtseng@chromium.org
TEST=browser_tests --gtest_filter=PrintPreview*.*


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=91787

[Lei Zhang, 2011-07-07 01:27:24]

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
File chrome/browser/ui/webui/print_preview_browsertest.cc (right):

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
chrome/browser/ui/webui/print_preview_browsertest.cc:37: #if
!defined(ARCH_CPU_ARM_FAMILY)
This should be #if defined(ARCH_CPU_X86_FAMILY). I bet if we added a new
ARCH_CPU_FOO_FAMILY, it would work either. Ditto for web_ui_browsertest.cc.

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
chrome/browser/ui/webui/print_preview_browsertest.cc:78: #include
"chrome/test/data/webui/print_preview-inl.h"
This is the dynamically generated test list?

[Sheridan Rawlins, 2011-07-07 04:45:32]

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
File chrome/browser/ui/webui/print_preview_browsertest.cc (right):

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
chrome/browser/ui/webui/print_preview_browsertest.cc:37: #if
!defined(ARCH_CPU_ARM_FAMILY)
See crbug.com/88104 - this only doesn't compile v8_shell#host on Arm because the
flags check against target_arch instead of host_arch.  This flag was checked
with Bradley Nelson.

On 2011/07/07 01:27:24, Lei Zhang wrote:
> This should be #if defined(ARCH_CPU_X86_FAMILY). I bet if we added a new
> ARCH_CPU_FOO_FAMILY, it would work either. Ditto for web_ui_browsertest.cc.

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
chrome/browser/ui/webui/print_preview_browsertest.cc:78: #include
"chrome/test/data/webui/print_preview-inl.h"
On 2011/07/07 01:27:24, Lei Zhang wrote:
> This is the dynamically generated test list?

Yes.  The test is generated from any functions in |this| after importing the
chrome/test/data/webui/print_preview.js

[Lei Zhang, 2011-07-07 05:17:53]

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
File chrome/browser/ui/webui/print_preview_browsertest.cc (right):

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
chrome/browser/ui/webui/print_preview_browsertest.cc:37: #if
!defined(ARCH_CPU_ARM_FAMILY)
On 2011/07/07 04:45:33, Sheridan Rawlins wrote:
> See crbug.com/88104 - this only doesn't compile v8_shell#host on Arm because
the
> flags check against target_arch instead of host_arch.  This flag was checked
> with Bradley Nelson.
> 
> On 2011/07/07 01:27:24, Lei Zhang wrote:
> > This should be #if defined(ARCH_CPU_X86_FAMILY). I bet if we added a new
> > ARCH_CPU_FOO_FAMILY, it would work either. Ditto for web_ui_browsertest.cc.
> 

I don't understand your reasoning. ARCH_CPU_X86_FAMILY and ARCH_CPU_ARM_FAMILY
are mutually exclusive. Defining one is the same as !defining the other.

I'm just trying to say explicitly defining the architecture this works on is
better than !defining the one (of two) architecture it doesn't work on.

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
chrome/browser/ui/webui/print_preview_browsertest.cc:78: #include
"chrome/test/data/webui/print_preview-inl.h"
On 2011/07/07 04:45:33, Sheridan Rawlins wrote:
> On 2011/07/07 01:27:24, Lei Zhang wrote:
> > This is the dynamically generated test list?
> 
> Yes.  The test is generated from any functions in |this| after importing the
> chrome/test/data/webui/print_preview.js

Then it should go into out/{Release,Debug} or it's Win/Mac equivalent. We don't
write into source directories.

[Sheridan Rawlins, 2011-07-07 07:01:55]

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
File chrome/browser/ui/webui/print_preview_browsertest.cc (right):

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
chrome/browser/ui/webui/print_preview_browsertest.cc:37: #if
!defined(ARCH_CPU_ARM_FAMILY)
Will ARCH_CPU_X86_FAMILY be defined for mac?  for other non-arm platforms?  If
any new platforms are added, I'd like to try to build this the new platforms and
get this working on ARM before removing the ifndef.  This must agree with the
gyp conditions which currently check for target_arch != arm, so this ifndef
seemed reasonable and correct.

On 2011/07/07 05:17:53, Lei Zhang wrote:
> On 2011/07/07 04:45:33, Sheridan Rawlins wrote:
> > See crbug.com/88104 - this only doesn't compile v8_shell#host on Arm because
> the
> > flags check against target_arch instead of host_arch.  This flag was checked
> > with Bradley Nelson.
> > 
> > On 2011/07/07 01:27:24, Lei Zhang wrote:
> > > This should be #if defined(ARCH_CPU_X86_FAMILY). I bet if we added a new
> > > ARCH_CPU_FOO_FAMILY, it would work either. Ditto for
web_ui_browsertest.cc.
> > 
> 
> I don't understand your reasoning. ARCH_CPU_X86_FAMILY and ARCH_CPU_ARM_FAMILY
> are mutually exclusive. Defining one is the same as !defining the other.
> 
> I'm just trying to say explicitly defining the architecture this works on is
> better than !defining the one (of two) architecture it doesn't work on.

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
chrome/browser/ui/webui/print_preview_browsertest.cc:78: #include
"chrome/test/data/webui/print_preview-inl.h"
It does go there.  Please see
http://codereview.chromium.org/7087014/diff/75002/chrome/chrome_tests.gypi for
more information on the magic which puts it there and adds it to the include
path.  Also, this is not "out" but "ninja" when building with ninja :)
Basically, it's the '<(SHARED_INTERMEDIATE_DIR)/js2webui'.

On 2011/07/07 05:17:53, Lei Zhang wrote:
> On 2011/07/07 04:45:33, Sheridan Rawlins wrote:
> > On 2011/07/07 01:27:24, Lei Zhang wrote:
> > > This is the dynamically generated test list?
> > 
> > Yes.  The test is generated from any functions in |this| after importing the
> > chrome/test/data/webui/print_preview.js
> 
> Then it should go into out/{Release,Debug} or it's Win/Mac equivalent. We
don't
> write into source directories.

[Lei Zhang, 2011-07-07 19:16:57]

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
File chrome/browser/ui/webui/print_preview_browsertest.cc (right):

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
chrome/browser/ui/webui/print_preview_browsertest.cc:37: #if
!defined(ARCH_CPU_ARM_FAMILY)
On 2011/07/07 07:01:55, Sheridan Rawlins wrote:
> Will ARCH_CPU_X86_FAMILY be defined for mac?  for other non-arm platforms?  If
> any new platforms are added, I'd like to try to build this the new platforms
and
> get this working on ARM before removing the ifndef.  This must agree with the
> gyp conditions which currently check for target_arch != arm, so this ifndef
> seemed reasonable and correct.

Sure, let's leave it as is then. We probably won't be adding any new
architectures anytime soon anyway.
FWIW, ARCH_CPU_X86_FAMILY is defined for Mac, as they use x86 CPUs.

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
chrome/browser/ui/webui/print_preview_browsertest.cc:78: #include
"chrome/test/data/webui/print_preview-inl.h"
On 2011/07/07 07:01:55, Sheridan Rawlins wrote:
> It does go there.  Please see
> http://codereview.chromium.org/7087014/diff/75002/chrome/chrome_tests.gypi for
> more information on the magic which puts it there and adds it to the include
> path.  Also, this is not "out" but "ninja" when building with ninja :)
> Basically, it's the '<(SHARED_INTERMEDIATE_DIR)/js2webui'.
> 
> On 2011/07/07 05:17:53, Lei Zhang wrote:
> > On 2011/07/07 04:45:33, Sheridan Rawlins wrote:
> > > On 2011/07/07 01:27:24, Lei Zhang wrote:
> > > > This is the dynamically generated test list?
> > > 
> > > Yes.  The test is generated from any functions in |this| after importing
the
> > > chrome/test/data/webui/print_preview.js
> > 
> > Then it should go into out/{Release,Debug} or it's Win/Mac equivalent. We
> don't
> > write into source directories.
> 

Let's just make the #include obvious that it's a generated directory and not the
source directory.

[Sheridan Rawlins, 2011-07-07 20:30:54]

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
File chrome/browser/ui/webui/print_preview_browsertest.cc (right):

http://codereview.chromium.org/7304014/diff/3001/chrome/browser/ui/webui/prin...
chrome/browser/ui/webui/print_preview_browsertest.cc:78: #include
"chrome/test/data/webui/print_preview-inl.h"
On 2011/07/07 19:16:57, Lei Zhang wrote:
> On 2011/07/07 07:01:55, Sheridan Rawlins wrote:
> > It does go there.  Please see
> > http://codereview.chromium.org/7087014/diff/75002/chrome/chrome_tests.gypi
for
> > more information on the magic which puts it there and adds it to the include
> > path.  Also, this is not "out" but "ninja" when building with ninja :)
> > Basically, it's the '<(SHARED_INTERMEDIATE_DIR)/js2webui'.
> > 
> > On 2011/07/07 05:17:53, Lei Zhang wrote:
> > > On 2011/07/07 04:45:33, Sheridan Rawlins wrote:
> > > > On 2011/07/07 01:27:24, Lei Zhang wrote:
> > > > > This is the dynamically generated test list?
> > > > 
> > > > Yes.  The test is generated from any functions in |this| after importing
> the
> > > > chrome/test/data/webui/print_preview.js
> > > 
> > > Then it should go into out/{Release,Debug} or it's Win/Mac equivalent. We
> > don't
> > > write into source directories.
> > 
> 
> Let's just make the #include obvious that it's a generated directory and not
the
> source directory.

Done.

[Lei Zhang, 2011-07-07 20:45:29]

LGTM on the non-JS bits.

http://codereview.chromium.org/7304014/diff/13001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

http://codereview.chromium.org/7304014/diff/13001/chrome/chrome_tests.gypi#ne...
chrome/chrome_tests.gypi:2739:
'<(js2webui_out_dir)/js2webui/chrome/<(rule_input_relpath)/<(RULE_INPUT_ROOT)-inl.h',
This is fine, but I just want to note this expand out to
$(SHARED_INTERMEDIATE_DIR)/js2webui/js2webui/...

[Sheridan Rawlins, 2011-07-07 20:53:15]

Jói or Mike, would you be able to give a LG for the javascript part?

[Mike Mammarella, 2011-07-07 23:42:51]

On 2011/07/07 20:53:15, Sheridan Rawlins wrote:
> Jói or Mike, would you be able to give a LG for the javascript part?

I'm not familiar with the function of the JS here, but syntactically it LGTM.

[Sheridan Rawlins, 2011-07-07 23:51:12]

http://codereview.chromium.org/7304014/diff/13001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

http://codereview.chromium.org/7304014/diff/13001/chrome/chrome_tests.gypi#ne...
chrome/chrome_tests.gypi:2739:
'<(js2webui_out_dir)/js2webui/chrome/<(rule_input_relpath)/<(RULE_INPUT_ROOT)-inl.h',
Yes, this is to make sure that there's nothing else in it but _out_ generated
files when we add $(SHARED_INTERMEDIATE_DIR)/js2webui to the includes.  Since
you wanted the #include line to more clearly indicate a path which was
generated, the path unfortunately has this stutter :)  Thanks for dbl checking
and okaying it!

On 2011/07/07 20:45:29, Lei Zhang wrote:
> This is fine, but I just want to note this expand out to
> $(SHARED_INTERMEDIATE_DIR)/js2webui/js2webui/...

[Sheridan Rawlins, 2011-07-07 23:52:06]

http://codereview.chromium.org/7304014/diff/13001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

http://codereview.chromium.org/7304014/diff/13001/chrome/chrome_tests.gypi#ne...
chrome/chrome_tests.gypi:2739:
'<(js2webui_out_dir)/js2webui/chrome/<(rule_input_relpath)/<(RULE_INPUT_ROOT)-inl.h',
/_out_/_our_/.

On 2011/07/07 23:51:12, Sheridan Rawlins wrote:
> Yes, this is to make sure that there's nothing else in it but _out_ generated
> files when we add $(SHARED_INTERMEDIATE_DIR)/js2webui to the includes.  Since
> you wanted the #include line to more clearly indicate a path which was
> generated, the path unfortunately has this stutter :)  Thanks for dbl checking
> and okaying it!
> 
> On 2011/07/07 20:45:29, Lei Zhang wrote:
> > This is fine, but I just want to note this expand out to
> > $(SHARED_INTERMEDIATE_DIR)/js2webui/js2webui/...
>

[commit-bot: I haz the power, 2011-07-08 00:37:35]

Change committed as 91787