Cleanups to the refactored extension activity log code

chrome/browser/extensions/activity_log/activity_log.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/activity_log/activity_log.h"
 
 #include <set>
 #include <vector>
 
 #include "base/command_line.h"
@@ skipping 134 matching lines @@
     base::ListValue* args = action->mutable_args();
     if (args->GetInteger(0, &tab_id)) {
       GURL url;
       bool is_incognito;
       if (GetUrlForTabId(tab_id, profile, &url, &is_incognito)) {
         action->set_arg_url(url);
         action->set_arg_incognito(is_incognito);
       }
     } else if ((api_call == "tabs.move" || api_call == "tabs.remove") &&
                args->GetList(0, &id_list)) {
+      // For APIs that take a list of tab IDs: translate all tab IDs into URLs
+      // in-place within the argument list, so all URLs are available when full
+      // logging is turned on.  We can only extract one of the URLs into
+      // arg_url; arbitrarily take the first one.

[Matt Perry, 2013/08/02 19:23:04]

What's the difference between arg_url and the id_list from args?

[mvrable, 2013/08/02 19:57:09]

arg_url is a separate database column where we can store a URL (but only one);
if we encounter a URL in the API argument list we'll extract it into arg_url so
we can more easily search through the database.

A couple of tabs APIs can include a list of tabs (id_list); when we translate
those to URLs we can't fit all of them into arg_url so the full list of URLs can
only fit in the args list.

[Matt Perry, 2013/08/02 20:26:05]

Might be worth briefly mentioning that in the comment.

[mvrable, 2013/08/02 20:54:49]

I reworked the comments to try to make this clearer.

       for (int i = 0; i < static_cast<int>(id_list->GetSize()); ++i) {
         if (id_list->GetInteger(i, &tab_id)) {
           GURL url;
           bool is_incognito;
           if (GetUrlForTabId(tab_id, profile, &url, &is_incognito) &&
-              !is_incognito)
+              !is_incognito) {
             id_list->Set(i, new base::StringValue(url.spec()));
+            if (i == 0) {
+              action->set_arg_url(url);
+              action->set_arg_incognito(is_incognito);
+            }
+          }
         } else {
           LOG(ERROR) << "The tab ID array is malformed at index " << i;
         }
       }
     }
   }
 }
 
 }  // namespace
 
@@ skipping 237 matching lines @@
            it2 != it->second.end();
            ++it2) {
         action->mutable_args()->AppendString(*it2);
       }
       LogAction(action);
     }
   }
 }
 
 }  // namespace extensions