chrome/browser/component_updater/component_updater_service.cc - Issue 18516010: Implemented completion pings for component updates.

Unified Diff: chrome/browser/component_updater/component_updater_service.cc

Issue 18516010: Implemented completion pings for component updates. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: Created 7 years, 5 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« chrome/browser/component_updater/component_updater_ping_manager.cc ('K') | « chrome/browser/component_updater/component_updater_service.h ('k') | chrome/browser/component_updater/component_updater_service_observer.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/browser/component_updater/component_updater_service.cc

diff --git a/chrome/browser/component_updater/component_updater_service.cc b/chrome/browser/component_updater/component_updater_service.cc

index ab9fb21e6a2b16f779e9bdeec9152ac0a7df765f..6760bd9119d8f50e1510b82c95f5fbd710590883 100644

--- a/chrome/browser/component_updater/component_updater_service.cc

+++ b/chrome/browser/component_updater/component_updater_service.cc

@@ -15,6 +15,7 @@

#include "base/files/file_path.h"

#include "base/logging.h"

#include "base/memory/scoped_ptr.h"

+#include "base/observer_list.h"

#include "base/stl_util.h"

#include "base/strings/string_number_conversions.h"

#include "base/strings/string_piece.h"

@@ -25,6 +26,9 @@

#include "chrome/browser/chrome_notification_types.h"

#include "chrome/browser/component_updater/component_patcher.h"

#include "chrome/browser/component_updater/component_unpacker.h"

+#include "chrome/browser/component_updater/component_updater_ping_manager.h"

+#include "chrome/browser/component_updater/component_updater_service_observer.h"

+#include "chrome/browser/component_updater/crx_update_item.h"

#include "chrome/common/chrome_utility_messages.h"

#include "chrome/common/chrome_version_info.h"

#include "chrome/common/extensions/extension.h"

@@ -41,6 +45,7 @@

#include "net/url_request/url_request_status.h"

#include "url/gurl.h"

+using component_updater::ComponentUpdaterServiceObserver;

using content::BrowserThread;

using content::UtilityProcessHost;

using content::UtilityProcessHostClient;

@@ -107,14 +112,6 @@ static std::string HexStringToID(const std::string& hexstr) {

return id;

}

-// Returns given a crx id it returns a small number, less than 100, that has a

-// decent chance of being unique among the registered components. It also has

-// the nice property that can be trivially computed by hand.

-static int CrxIdtoUMAId(const std::string& id) {

- CHECK_GT(id.size(), 2U);

- return id[0] + id[1] + id[2] - ('a' * 3);

// Helper to do version check for components.

bool IsVersionNewer(const Version& current, const std::string& proposed) {

Version proposed_ver(proposed);

@@ -172,97 +169,28 @@ bool FetchSuccess(const net::URLFetcher& fetcher) {

(fetcher.GetResponseCode() == 200);

}

-// This is the one and only per-item state structure. Designed to be hosted

-// in a std::vector or a std::list. The two main members are |component|

-// which is supplied by the the component updater client and |status| which

-// is modified as the item is processed by the update pipeline. The expected

-// transition graph is:

-//

-// kNew

-// |

-// V

-// +----------------------> kChecking -<---------+-----<-------+

-// | | | |

-// | error V no | |

-// kNoUpdate <---------------- [update?] ->---- kUpToDate kUpdated

-// ^ | ^

-// | yes | |

-// | diff=false V |

-// | +-----------> kCanUpdate |

-// | | | |

-// | | V no |

-// | | [differential update?]->----+ |

-// | | | | |

-// | | yes | | |

-// | | error V | |

-// | +---------<- kDownloadingDiff | |

-// | | | | |

-// | | error V | |

-// | +---------<- kUpdatingDiff ->--------|-----------+ success

-// | | |

-// | error V |

-// +----------------------------------------- kDownloading |

-// | | |

-// | error V |

-// +------------------------------------------ kUpdating ->----+ success

-//

-struct CrxUpdateItem {

- enum Status {

- kNew,

- kChecking,

- kCanUpdate,

- kDownloadingDiff,

- kDownloading,

- kUpdatingDiff,

- kUpdating,

- kUpdated,

- kUpToDate,

- kNoUpdate,

- kLastStatus

- };

+// Returns the error code which occured during the fetch.The function returns 0

+// if the fetch was successful. If errors happen, the function could return a

+// network error, an http response code, or the status of the fetch, if the

+// fetch is pending or canceled.

+int GetFetchError(const net::URLFetcher& fetcher) {

+ if (FetchSuccess(fetcher))

+ return 0;

- Status status;

- std::string id;

- CrxComponent component;

- base::Time last_check;

- // These members are initialized with their corresponding values from the

- // update server response.

- GURL crx_url;

- GURL diff_crx_url;

- int size;

- int diff_size;

- // The from/to version and fingerprint values.

- Version previous_version;

- Version next_version;

- std::string previous_fp;

- std::string next_fp;

- // True if the differential update failed for any reason.

- bool diff_update_failed;

- CrxUpdateItem()

- : status(kNew),

- size(0),

- diff_size(0),

- diff_update_failed(false) {

- }

+ const net::URLRequestStatus::Status status(fetcher.GetStatus().status());

+ if (status == net::URLRequestStatus::FAILED)

+ return fetcher.GetStatus().error();

- // Function object used to find a specific component.

- class FindById {

- public:

- explicit FindById(const std::string& id) : id_(id) {}

+ if (status == net::URLRequestStatus::IO_PENDING ||

+ status == net::URLRequestStatus::CANCELED)

+ return status;

- bool operator() (CrxUpdateItem* item) const {

- return (item->id == id_);

- }

- private:

- const std::string& id_;

- };

-};

+ const int response_code(fetcher.GetResponseCode());

+ if (status == net::URLRequestStatus::SUCCESS && response_code != 200)

+ return response_code;

+ return -1;

+ }

// Returns true if a differential update is available for the update item.

bool IsDiffUpdateAvailable(const CrxUpdateItem* update_item) {

@@ -278,7 +206,21 @@ bool CanTryDiffUpdate(const CrxUpdateItem* update_item,

config.DeltasEnabled();

}

-} // namespace.

+} // namespace

+CrxUpdateItem::CrxUpdateItem()

+ : status(kNew),

+ diff_update_failed(false),

+ error_category(0),

+ error_code(0),

+ extra_code1(0),

+ diff_error_category(0),

+ diff_error_code(0),

+ diff_extra_code1(0) {

+CrxUpdateItem::~CrxUpdateItem() {

CrxComponent::CrxComponent()

: installer(NULL) {

@@ -370,9 +312,15 @@ class CrxUpdateService : public ComponentUpdateService {

CRXContext* context);

private:

+ enum ErrorCategory {

+ kErrorNone = 0,

+ kNetworkError,

+ kUnpackError,

+ kInstallError,

+ };

// See ManifestParserBridge.

- void OnParseUpdateManifestSucceeded(

- const UpdateManifest::Results& results);

+ void OnParseUpdateManifestSucceeded(const UpdateManifest::Results& results);

// See ManifestParserBridge.

void OnParseUpdateManifestFailed(const std::string& error_message);

@@ -402,6 +350,10 @@ class CrxUpdateService : public ComponentUpdateService {

scoped_ptr<net::URLFetcher> url_fetcher_;

+ ObserverList<component_updater::ComponentUpdaterServiceObserver> observers_;

+ scoped_ptr<component_updater::PingManager> ping_manager_;

// A collection of every work item.

typedef std::vector<CrxUpdateItem*> UpdateItems;

UpdateItems work_items_;

@@ -424,10 +376,14 @@ class CrxUpdateService : public ComponentUpdateService {

CrxUpdateService::CrxUpdateService(ComponentUpdateService::Configurator* config)

: config_(config),

component_patcher_(config->CreateComponentPatcher()),

+ ping_manager_(new component_updater::PingManager(

+ config->PingUrl(),

+ config->RequestContext())),

chrome_version_(chrome::VersionInfo().Version()),

prod_id_(chrome::OmahaQueryParams::GetProdIdString(

chrome::OmahaQueryParams::CHROME)),

running_(false) {

+ observers_.AddObserver(ping_manager_.get());

}

CrxUpdateService::~CrxUpdateService() {

@@ -436,6 +392,7 @@ CrxUpdateService::~CrxUpdateService() {

// flight in other threads.

Stop();

STLDeleteElements(&work_items_);

+ observers_.RemoveObserver(ping_manager_.get());

}

ComponentUpdateService::Status CrxUpdateService::Start() {

@@ -586,6 +543,12 @@ bool CrxUpdateService::AddItemToUpdateCheck(CrxUpdateItem* item,

item->previous_fp = item->component.fingerprint;

item->next_fp.clear();

item->diff_update_failed = false;

+ item->error_category = 0;

+ item->error_code = 0;

+ item->extra_code1 = 0;

+ item->diff_error_category = 0;

+ item->diff_error_code = 0;

+ item->diff_extra_code1 = 0;

return true;

}

@@ -801,8 +764,6 @@ void CrxUpdateService::OnParseUpdateManifestSucceeded(

if (crx->status != CrxUpdateItem::kChecking)

continue; // Not updating this component now.

- config_->OnEvent(Configurator::kManifestCheck, CrxIdtoUMAId(crx->id));

if (it->version.empty()) {

// No version means no update available.

crx->status = CrxUpdateItem::kNoUpdate;

@@ -823,9 +784,7 @@ void CrxUpdateService::OnParseUpdateManifestSucceeded(

// All test passed. Queue an upgrade for this component and fire the

// notifications.

crx->crx_url = it->crx_url;

- crx->size = it->size;

crx->diff_crx_url = it->diff_crx_url;

- crx->diff_size = it->diff_size;

crx->status = CrxUpdateItem::kCanUpdate;

crx->next_version = Version(it->version);

crx->next_fp = it->package_fingerprint;

@@ -850,7 +809,6 @@ void CrxUpdateService::OnParseUpdateManifestFailed(

DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

size_t count = ChangeItemStatus(CrxUpdateItem::kChecking,

CrxUpdateItem::kNoUpdate);

- config_->OnEvent(Configurator::kManifestError, static_cast<int>(count));

DCHECK_GT(count, 0ul);

ScheduleNextRun(false);

}

@@ -869,19 +827,29 @@ void CrxUpdateService::OnURLFetchComplete(const net::URLFetcher* source,

if (source->FileErrorOccurred(&error_code) || !FetchSuccess(*source)) {

if (crx->status == CrxUpdateItem::kDownloadingDiff) {

+ crx->diff_error_category = kNetworkError;

+ crx->diff_error_code = GetFetchError(*source);

crx->diff_update_failed = true;

size_t count = ChangeItemStatus(CrxUpdateItem::kDownloadingDiff,

CrxUpdateItem::kCanUpdate);

DCHECK_EQ(count, 1ul);

+ url_fetcher_.reset();

ScheduleNextRun(true);

return;

}

+ crx->error_category = kNetworkError;

+ crx->error_code = GetFetchError(*source);

size_t count = ChangeItemStatus(CrxUpdateItem::kDownloading,

CrxUpdateItem::kNoUpdate);

DCHECK_EQ(count, 1ul);

- config_->OnEvent(Configurator::kNetworkError, CrxIdtoUMAId(context->id));

url_fetcher_.reset();

+ // At this point, since both the differential and the full downloads failed,

+ // the update for this component has finished with an error.

+ FOR_EACH_OBSERVER(component_updater::ComponentUpdaterServiceObserver,

+ observers_, OnUpdateComplete(crx));

ScheduleNextRun(false);

} else {

base::FilePath temp_crx_path;

@@ -942,15 +910,32 @@ void CrxUpdateService::Install(const CRXContext* context,

}

// Installation has been completed. Adjust the component status and

-// schedule the next check.

+// schedule the next check. Schedule a short delay before trying the full

+// update when the differential update failed.

void CrxUpdateService::DoneInstalling(const std::string& component_id,

ComponentUnpacker::Error error,

int extra_code) {

DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

+ ErrorCategory error_category = kErrorNone;

+ switch (error) {

+ case ComponentUnpacker::kNone:

+ break;

+ case ComponentUnpacker::kInstallerError:

+ error_category = kInstallError;

+ break;

+ default:

+ error_category = kUnpackError;

+ break;

+ }

+ const bool is_success = error == ComponentUnpacker::kNone;

CrxUpdateItem* item = FindUpdateItemById(component_id);

- if (item->status == CrxUpdateItem::kUpdatingDiff) {

- if (error != ComponentUnpacker::kNone) {

+ if (item->status == CrxUpdateItem::kUpdatingDiff && !is_success) {

+ item->diff_error_category = error_category;

+ item->diff_error_code = error;

+ item->diff_extra_code1 = extra_code;

item->diff_update_failed = true;

size_t count = ChangeItemStatus(CrxUpdateItem::kUpdatingDiff,

CrxUpdateItem::kCanUpdate);

@@ -958,29 +943,21 @@ void CrxUpdateService::DoneInstalling(const std::string& component_id,

ScheduleNextRun(true);

return;

}

- }

- item->status = (error == ComponentUnpacker::kNone) ? CrxUpdateItem::kUpdated :

- CrxUpdateItem::kNoUpdate;

- if (item->status == CrxUpdateItem::kUpdated) {

+ if (is_success) {

+ item->status = CrxUpdateItem::kUpdated;

waffles 2013/07/12 20:48:23 Do we need to use ChangeItemStatus here? I'm not

Sorin Jianu 2013/07/12 21:01:57 I kept the code that matched what we already had a

item->component.version = item->next_version;

item->component.fingerprint = item->next_fp;

+ } else {

+ item->status = CrxUpdateItem::kNoUpdate;

+ item->error_category = error_category;

+ item->error_code = error;

+ item->extra_code1 = extra_code;

}

- Configurator::Events event;

- switch (error) {

- case ComponentUnpacker::kNone:

- event = Configurator::kComponentUpdated;

- break;

- case ComponentUnpacker::kInstallerError:

- event = Configurator::kInstallerError;

- break;

- default:

- event = Configurator::kUnpackError;

- break;

- }

+ FOR_EACH_OBSERVER(component_updater::ComponentUpdaterServiceObserver,

+ observers_, OnUpdateComplete(item));

- config_->OnEvent(event, CrxIdtoUMAId(component_id));

ScheduleNextRun(false);

}