Exposed GetSecurityState and GetPageType to pyAuto.

chrome/test/pyautolib/pyautolib.i

Index: chrome/test/pyautolib/pyautolib.i
===================================================================
--- chrome/test/pyautolib/pyautolib.i	(revision 124219)
+++ chrome/test/pyautolib/pyautolib.i	(working copy)
@@ -17,6 +17,8 @@
 %module(docstring="Python interface to Automation Proxy.") pyautolib
 %feature("autodoc", "1");
 
+%include "typemaps.i"
+%include "cpointer.i"

[dennis_jeffrey, 2012/02/29 23:37:13]

I'm not very familiar with swig syntax.  Is there a reason why these includes
are specified with quotes, but the ones in lines 22-23 below use angle brackets?

[dyu1, 2012/03/05 19:16:46]

It works with angle brackets as well but I just used the quoted form which I had
found in the swig documentation.

On 2012/02/29 23:37:13, dennis_jeffrey wrote:

 %include <std_wstring.i>
 %include <std_string.i>
 
@@ -30,6 +32,10 @@
 %include "chrome/app/chrome_dll_resource.h"
 %include "chrome/common/automation_constants.h"
 %include "chrome/common/pref_names.h"
+%include "content/public/common/page_type.h"
+%include "content/public/common/security_style.h"
+%include "net/base/net_export.h"
+%include "net/base/cert_status_flags.h"

[dennis_jeffrey, 2012/02/29 23:37:13]

swap the above 2 lines to put them in alphabetical order

[dyu1, 2012/03/05 19:16:46]

If I swap the two include lines from above I get a compile error. 

target/pyautolib/geni/pyautolib_wrap.cc', 'test/pyautolib/pyautolib.i']
../net/base/cert_status_flags.h:50: Error: Syntax error in input(1).
make: *** [out/Debug/obj.target/pyautolib/geni/pyautolib_wrap.cc] Error 1

 
 %{
 #include "chrome/common/automation_constants.h"
@@ -147,6 +153,22 @@
           "button, if false to 'Take me out of there' button.")

[dennis_jeffrey, 2012/02/29 23:37:13]

indent the above 2 lines by 1 more space each to be consistent with the other
lines above.

Yeah, I know I'm nit-picky :-P

[dyu1, 2012/03/05 19:16:46]

Done.

       TakeActionOnSSLBlockingPage;
   bool TakeActionOnSSLBlockingPage(bool proceed);
+  %extend {
+    %feature("docstring", "Retrieves the different security states for the current tab.")

[anantha, 2012/02/29 22:30:02]

Indentation doesn't look correct. Can you please check?

[dennis_jeffrey, 2012/02/29 23:37:13]

Take a look at how other long docstrings are specified above (e.g., lines
151-153).  It looks like separate strings in multiples lines will be
automatically concatenated, so you should be able to separate the docstring here
into 2 separate lines.

[dyu1, 2012/03/05 19:16:46]

Done.

[dyu1, 2012/03/05 19:16:46]

Done.

+      GetSecurityState;

[dennis_jeffrey, 2012/02/29 23:37:13]

this can probably be moved to the line above, like how it's specified in other
cases above (e.g., line 145).

[dyu1, 2012/03/05 19:16:46]

In the examples I see, these are added after the docstring %feature. 

On 2012/02/29 23:37:13, dennis_jeffrey wrote:

+    bool GetSecurityState(int *security_style, uint32 *ssl_cert_status,

[dennis_jeffrey, 2012/02/29 23:37:13]

move the * over so that it touches the type:

int*
uint32*

[Nirnimesh, 2012/02/29 23:41:08]

use int*, not int *
(ie, * should go with the type)

[dyu1, 2012/03/05 19:16:46]

The Swig doc actually shows the examples this way. But I'll change it. 

On 2012/02/29 23:37:13, dennis_jeffrey wrote:

[dyu1, 2012/03/05 19:16:46]

Done.

+                          int * insecure_content_status) {

[dennis_jeffrey, 2012/02/29 23:37:13]

same comment as line 159 above.  Also do the same where * is used down below
(lines 161, 162, 168, 169)

[Nirnimesh, 2012/02/29 23:41:08]

int*

[dyu1, 2012/03/05 19:16:46]

Done.

[dyu1, 2012/03/05 19:16:46]

Done.

+     return $self->GetSecurityState(reinterpret_cast<content::SecurityStyle *>(security_style),

[Nirnimesh, 2012/02/29 23:41:08]

reduce to 80 chars per line

[Nirnimesh, 2012/02/29 23:41:08]

Wouldn't it have been easier to implement the method yourself (like GetPort()
below) so that you wouldn't have to deal with all this swig magic. Besides, I'm
not sure it's a good idea to expose pointer types to python.

BUT, if it works, I won't be totally opposed to your implementation. Just
pointing out that it could have been simpler and also easier on the python side.

[dyu1, 2012/03/05 19:16:46]

Done.

[dyu1, 2012/03/05 19:16:46]

Done.

[dyu1, 2012/03/05 19:16:46]

This is now done. It wasn't easy to implement as there was a lot of reading to
be done about swig and c++ interface with python. It's much more pythonic now
and easier to write the tests. 

GetSecurityState() in pyautolib returns a tuple consisting of a boolean and a
dict rather than having to call it with args like before.

When writing the tests the function can be called without any args and it
returns a tuple consisting of a boolean and a dict like this one:
(True, {'security_style': 2, 'ssl_cert_status': 5, 'insecure_content_status':
0})

On 2012/02/29 23:41:08, Nirnimesh wrote:

+       reinterpret_cast<net::CertStatus *>(ssl_cert_status), insecure_content_status);
+    }
+  }
+  %extend {
+    %feature("docstring", "Returns the type of page currently showing (normal, interstitial, error.")

[dennis_jeffrey, 2012/02/29 23:37:13]

Make sure lines 161, 162, and 166 don't exceed 80 chars each.

[dyu1, 2012/03/05 19:16:46]

Done.

+      GetPageType;
+    bool GetPageType(int *OUTPUT) {

[dennis_jeffrey, 2012/02/29 23:37:13]

OUTPUT --> output

[dyu1, 2012/03/05 19:16:46]

OUTPUT variable is actually a typemap and it cannot be changed. The swig
translates it in a special way and makes it a returning value. Look here:
http://www.swig.org/Doc1.3/Arguments.html
It says you can use another name by adding this directive:
 %apply int *OUTPUT { int *output };
but this didn't work in my case.
If you change it to "output" it will still compile but the GetPageType won't
work correctly. It will expect a pointer arg and all it will return will be a
boolean (success or not).

+      return $self->GetPageType(reinterpret_cast<content::PageType *>(OUTPUT));
+    }
+  }
 
   // HTTP Auth
   %feature("docstring",
@@ -475,3 +497,5 @@
 %{
 typedef net::TestServer::HTTPSOptions HTTPSOptions;
 %}
+%pointer_class(int, intp);

[Nirnimesh, 2012/02/29 23:41:08]

intp -> int_ptr

[dyu1, 2012/03/05 19:16:46]

Done.

+%pointer_class(uint32, uint32p);

[dennis_jeffrey, 2012/02/29 23:37:13]

Just curious: what do these %pointer lines do?  Are they required to be down
here at the bottom of the file?  Should we add a blank line above line 500 to
separate the typedef above from these pointer_class lines?

[Nirnimesh, 2012/02/29 23:41:08]

uint32p -> uint32_ptr

[dyu1, 2012/03/05 19:16:46]

Done.

[dyu1, 2012/03/05 19:16:46]

They are required to dereference constants like this:
pyauto.uint32_ptr.frompointer(pyauto.CERT_STATUS_AUTHORITY_INVALID).value()
They can be put anywhere. I blanked the previous line.

On 2012/02/29 23:37:13, dennis_jeffrey wrote: