Major revision of page cycler UI.

chrome/common/extensions/api/experimental_record.json

Index: chrome/common/extensions/api/experimental_record.json
diff --git a/chrome/common/extensions/api/experimental_record.json b/chrome/common/extensions/api/experimental_record.json
index 1a11e721e1be930c1677064d4d2dc58c7fb98424..0da7346ea54e05e82e245500d15b7836e8b8e554 100644
--- a/chrome/common/extensions/api/experimental_record.json
+++ b/chrome/common/extensions/api/experimental_record.json
@@ -7,17 +7,12 @@
         "type": "object",
         "description": "",
         "properties": {
-          "extensionPath": {"type": "string", "optional": true, "description": "A path to an extension to run in the session. Should be an unpacked extension."}
-        }
-      },
-      {
-        "id": "ReplayURLsResult",
-        "type": "object",
-        "description": "",
-        "properties": {
-          "runTime": {"type": "integer", "description": "Time in milliseconds to complete all runs."},
-          "stats": {"type": "string", "description": "Full multiline dump of output stats."},
-          "errors": {"type": "array", "items": {"type": "string"}, "description": "List of errors during replay."}
+          "extensionPath": {
+            "type": "string",
+            "optional": true,
+            "description": 
+                "Path to an unpacked extension to run in the session."

[Jeffrey Yasskin, 2012/08/09 19:27:04]

By path, you mean something like "/home/username/foo/my_extension" or
"C:\src\my_extension"? Are there any requirements on whether that extension can
or must already be loaded?

[clintstaley, 2012/08/10 00:33:13]

Yes, and no, respectively.  Or at least, no because it's moot; the run will be
in a sub-browser and the extension loaded by commandline argument.

Clarified the comment a bit.

+          }
         }
       }
     ],
@@ -28,15 +23,17 @@
         "type": "function",
         "parameters": [
           {
+            "type": "string",
+            "description": "Unique name of the capture.",

[Jeffrey Yasskin, 2012/08/09 19:27:04]

How should extensions allocate these names? What happens if the name collides
with some other capture? Can two different extensions use the same name?

[clintstaley, 2012/08/10 00:33:13]

These are capture names, and their uniqueness will be enforced once I add the
FileSystem storage on the JS side in the next revision.  Not sure what you mean
by allowing two extensions to use the same name ?

[Jeffrey Yasskin, 2012/08/15 01:03:59]

What do you mean by "enforced"? That is, what's the consequence of violating the
rule? If an extension calls captureURLs twice with the same captureName, what's
the effect?
I'd like you to describe the scope of the uniqueness. If two different
extensions both pass "cat" as the captureName argument to captureURLs(), what
happens? What about the same extension on two different computers that are
sync'ed to the same google account? What about the same extension in two
different sessions on the same computer+profile?

[Jeffrey Yasskin, 2012/08/16 18:31:51]

Ping. Remember to check for comment replies on more than the most recent patch.

[clintstaley, 2012/08/17 04:12:24]

Apologies for that.  I've been looking at the CL's themselves, and I should
probably be walking down the summary Emails point by point instead.

In any event, I think these parameters are destined for the ash heap based on
Aaron's switch back to transferring zip files with cache content, and storing
them on the extension side.  The only reason for the unique name originally was
to identify what FileSystem dir from which to draw cache data, but if that's
coming across now as anonymous zip content, I'll be dropping these from the next
rev.

[Jeffrey Yasskin, 2012/08/17 19:32:14]

SGTM.

+            "name": "captureName"
+          },
+          {
             "type": "array",
             "items": {"type": "string"},
+            "description": "URL list to visit during capture.",
             "name": "urls"
           },
           {
-            "type": "string",
-            "name": "cacheDirectoryPath"
-          },
-          {
             "name": "callback",
             "type": "function",
             "description": "Called when capture has completed.",
@@ -46,6 +43,11 @@
                 "type": "array",
                 "items": {"type": "string"},
                 "name": "errors"
+              },
+              { 
+                "type": "string",
+                "name": "cacheDirectory",
+                "description": "Full path to cache directory used for capture"

[Jeffrey Yasskin, 2012/08/09 19:27:04]

Is there a security risk here? What if the directory already exists? Can it be
used to overwrite the user's files?

[clintstaley, 2012/08/10 00:33:13]

Hmm.  Your comment prompts me to think a little more carefully about this (as
yet unimplemented) part of the interface.  No security risk; just a
security-impossibiity.  We can't mess with a C++ side dir from the sandbox.  So,
this is now dropped.

               }
             ]
           }
@@ -57,13 +59,9 @@
         "type": "function",
         "parameters": [
           {
-            "type": "array",
-            "items": {"type": "string"},
-            "name": "urls"
-          },
-          {
             "type": "string",
-            "name": "captureDirectoryPath"
+            "name": "captureName",
+            "description": "Unique name of capture.  Use to determine cache."

[Jeffrey Yasskin, 2012/08/09 19:27:04]

How is it used to determine the cache? Presumably you mean that this has to
match the name passed to a previous captureURLs call, but in that case, why not
have captureURLs "return" an object representing the capture, and pass that into
replayURLs instead of making the user track strings?

[clintstaley, 2012/08/10 00:33:13]

Such an approach was considered (returning a zip of the cache dir, to be stored
on the JS side in FileSystem storage and sent back on playback).  But, Aaron
preferred an approach where we would write to and from FileSystem storage on the
C++ side, so all the JS side needs is to track the names of captures, which are
also the names of the FileSystem directories into which the capture state will
be saved.

[Jeffrey Yasskin, 2012/08/15 01:03:59]

Can the extension get at the storage directory using any other FileSystem APIs?
If so, maybe the extension should just pass a DirectoryEntry to this function?
At the very least, the comment needs to say where the name comes from, and how
it's used to determine the cache.

On the other hand, I think Aaron said this function should just take a zip file
now.

[clintstaley, 2012/08/17 04:12:24]

Right, which makes the other stuff moot.  I guess I'd have been surprised anyway
if an extension that is essentially "third party" in design (no webui, etc)
could have such access, but I don't know this stuff as well as you.

           },
           {
             "type": "integer", 
@@ -80,7 +78,24 @@
             "name": "callback",
             "type": "function",
             "description": "Called when playback has completed.",
-            "parameters": [{"$ref": "ReplayURLsResult", "name": "result"}]
+            "parameters": [
+              {
+                "type": "integer",
+                "name": "runTime",
+                "description": "Time in milliseconds to complete all runs."

[Jeffrey Yasskin, 2012/08/09 19:27:04]

I saw there's a new performance-measuring API that uses a double number of
milliseconds to capture microseconds:
http://gent.ilcore.com/2012/06/better-timer-for-javascript.html. Have you
considered doing the same thing here?

[clintstaley, 2012/08/10 00:33:13]

Given the size of the times we're getting, not sure that resolution is needed.

[Jeffrey Yasskin, 2012/08/15 01:03:59]

It almost certainly won't be needed for the next couple years, but it costs you
very little to return a double from this interface instead of an int, and it
makes sure you won't need to extend the interface in the future, if people do
want more precise results.

[clintstaley, 2012/08/17 04:12:24]

Done.

+              },
+              {
+                "type": "string",
+                "name": "stats",
+                "description": "Full multiline dump of output stats."

[Jeffrey Yasskin, 2012/08/09 19:27:04]

In what format?

[clintstaley, 2012/08/17 04:12:24]

Here again, is something that will be refined in later version (sorry to keep
saying that).  Aaron suggests nice charts of performance data, and at present we
have a big ugly text dump.  I'll describe it here some more, to be complete, but
it's going to change.

[Jeffrey Yasskin, 2012/08/17 19:32:14]

That's fine. Having a precise format all along will remind us to keep the format
precise for the final documentation. FWIW, you're likely to wind up with an
object-based stats format instead of a string in the long run.

[clintstaley, 2012/08/20 23:17:06]

I wouldn't bet against you.  That's almost certainly what we'll end up with.

+              },
+              {
+                "type": "array",
+                "name": "errors",
+                "items": {"type": "string"},
+                "description": "List of errors during replay."

[Jeffrey Yasskin, 2012/08/09 19:27:04]

What do errors look like? What kinds of errors should be expected?

[clintstaley, 2012/08/10 00:33:13]

I can show you some examples.  But I think they'd be a bit lengthy to put into a
JSON description field.  Should I be making these description fields longer??

[Jeffrey Yasskin, 2012/08/15 01:03:59]

Yes, I think so. Remember that this is all programmers have when they're trying
to figure out how to use the API, and you don't want them to have to
guess-and-check, since they're likely to draw the wrong conclusions from that.

[clintstaley, 2012/08/17 04:12:24]

Especially once the capture name is removed, there won't be much to report aside
from unexpected faults, but I did add that to the description, and will pay more
attention to these descriptions going forward.  Hadn't realized they were used
that much.

+              }
+            ]
           }
         ]
       }