Changed the update protocol for component updater from v2 to v3.

chrome/browser/component_updater/component_updater_ping_manager.cc

 // Copyright 2013 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/component_updater/component_updater_ping_manager.h"
 #include "base/guid.h"
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
-#include "base/sys_info.h"
-#include "base/win/windows_version.h"
+#include "chrome/browser/component_updater/component_updater_utils.h"
 #include "chrome/browser/component_updater/crx_update_item.h"
-#include "chrome/common/chrome_version_info.h"
-#include "chrome/common/omaha_query_params/omaha_query_params.h"
-#include "net/base/load_flags.h"
-#include "net/base/net_errors.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_fetcher_delegate.h"
-#include "net/url_request/url_request_status.h"
 
 namespace {
 
 // Returns true if the |update_item| contains a valid differential update url.
 bool HasDiffUpdate(const CrxUpdateItem* update_item) {
   return update_item->diff_crx_url.is_valid();
 }
 
 }  // namespace
 
@@ skipping 32 matching lines @@
 
 void PingSender::SendPing(
     const GURL& ping_url,
     net::URLRequestContextGetter* url_request_context_getter,
     const CrxUpdateItem* item) {
   DCHECK(item);
 
   if (!ping_url.is_valid())
     return;
 
-  url_fetcher_.reset(net::URLFetcher::Create(0,
-                                             ping_url,
-                                             net::URLFetcher::POST,
-                                             this));
-
-  url_fetcher_->SetUploadData("application/xml", BuildPing(item));
-  url_fetcher_->SetRequestContext(url_request_context_getter);
-  url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
-                             net::LOAD_DO_NOT_SAVE_COOKIES |
-                             net::LOAD_DISABLE_CACHE);
-  url_fetcher_->SetAutomaticallyRetryOn5xx(false);
-  url_fetcher_->Start();
+  url_fetcher_.reset(SendProtocolRequest(ping_url,
+                                         BuildPing(item),
+                                         this,
+                                         url_request_context_getter));
 }
 
 // Builds a ping message for the specified update item.
 std::string PingSender::BuildPing(const CrxUpdateItem* item) {
-  const std::string prod_id(chrome::OmahaQueryParams::GetProdIdString(
-      chrome::OmahaQueryParams::CHROME));
+  const char app_element_format[] =
+      "<app appid=\"%s\" version=\"%s\" nextversion=\"%s\">"
+      "%s"
+      "</app>";
+  const std::string app_element(base::StringPrintf(
+      app_element_format,
+      item->id.c_str(),                               // "appid"
+      item->previous_version.GetString().c_str(),     // "version"
+      item->next_version.GetString().c_str(),         // "nextversion"

[waffles, 2013/11/18 22:15:59]

What is the benefit of sending previous_version and next_version at the <app>
level rather than at the <event> level? Simply to avoid sending one of the
versions twice?

We briefly discussed Friday:

What is the outcome version of the update?
Well, if <event eventresult represents a success, it is <app nextversion>,
otherwise it is <app version>.
I don't like this, and I don't like the fact that it makes various elements of
our protocol uncombinable (e.g. you can never send an <event> and an
<updatecheck> together).

(Sidenote: it's frustrating that we don't have a prescriptive definition of our
protocol. The document we have is primarily descriptive but lacks detail. Note
that our protocol document never explicitly states that an <event> and an
<updatecheck> may not be sent in the same request.)

[Sorin Jianu, 2013/11/19 04:25:45]

Let's discuss and see if there is a way to fix that.

On 2013/11/18 22:15:59, waffles wrote:

+      BuildPingEventElement(item).c_str()));
 
-  const char request_format[] =
-      "<o:gupdate xmlns:o=\"http://www.google.com/update2/request\" "
-      "protocol=\"2.0\" version=\"%s-%s\" requestid=\"{%s}\" "
-      "updaterchannel=\"%s\"> "
-      "<o:os platform=\"%s\" version=\"%s\"/> "
-      "<o:app appid=\"%s\" version=\"%s\">"
-      "%s"
-      "</o:app></o:gupdate>";
-  const std::string request(
-      base::StringPrintf(request_format,
-                         prod_id.c_str(),
-                         chrome::VersionInfo().Version().c_str(),
-                         base::GenerateGUID().c_str(),
-                         chrome::OmahaQueryParams::GetChannelString(),
-                         chrome::VersionInfo().OSType().c_str(),
-                         base::SysInfo().OperatingSystemVersion().c_str(),
-                         item->id.c_str(),
-                         item->component.version.GetString().c_str(),
-                         BuildPingEventElement(item).c_str()));
-  return request;
+  return BuildProtocolRequest(app_element);
 }
 
 // Returns a string representing one ping event xml element for an update item.
 std::string PingSender::BuildPingEventElement(const CrxUpdateItem* item) {
   DCHECK(item->status == CrxUpdateItem::kNoUpdate ||
          item->status == CrxUpdateItem::kUpdated);
 
   using base::StringAppendF;
 
-  std::string ping_event("<o:event eventtype=\"3\"");
+  std::string ping_event("<event eventtype=\"3\"");

[waffles, 2013/11/18 22:15:59]

Any thoughts about sending eventtype="2" for updates from 0.0.0.0? I guess I'm
inclined to say that we shouldn't. Even such an update is still an "update", not
really an "install".

[Sorin Jianu, 2013/11/19 04:25:45]

If could add that is you think it can help with the queries. I would like to
avoid it but I will make the change if you'd like.

[waffles, 2013/11/19 18:33:58]

Let's avoid it for now. The distinction between install and update is kind of
arbitrary in this case.

   const int event_result = item->status == CrxUpdateItem::kUpdated;
   StringAppendF(&ping_event, " eventresult=\"%d\"", event_result);
-  StringAppendF(&ping_event, " previousversion=\"%s\"",
-                item->previous_version.GetString().c_str());
-  StringAppendF(&ping_event, " nextversion=\"%s\"",
-                item->next_version.GetString().c_str());
   if (item->error_category)
     StringAppendF(&ping_event, " errorcat=\"%d\"", item->error_category);
   if (item->error_code)
     StringAppendF(&ping_event, " errorcode=\"%d\"", item->error_code);
   if (item->extra_code1)
     StringAppendF(&ping_event, " extracode1=\"%d\"", item->extra_code1);
   if (HasDiffUpdate(item))
     StringAppendF(&ping_event, " diffresult=\"%d\"", !item->diff_update_failed);
   if (item->diff_error_category)
     StringAppendF(&ping_event,
@@ skipping 26 matching lines @@
 
 // Sends a fire and forget ping when the updates are complete. The ping
 // sender object self-deletes after sending the ping.
 void PingManager::OnUpdateComplete(const CrxUpdateItem* item) {
   component_updater::PingSender* ping_sender(new PingSender);
   ping_sender->SendPing(ping_url_, url_request_context_getter_, item);
 }
 
 }  // namespace component_updater