Hello andreip,
I'd like you to do a code review. Please execute
g4 diff -c 9877173
or point your web browser to
http://mondrian/9877173
to review the following code:
Change 9877173 by stevebl...@steveblock-gears1 on 2009/01/27 15:56:31 *pending*
Modifies CabUpdater to pass a BrowsingContext through to the AsyncTask
used to check for updates and to download the CAB. Mailed on behalf of Opera.
R=andreip
[email protected],[email protected],[email protected]
DELTA=57 (34 added, 1 deleted, 22 changed)
OCL=9877173
Affected files ...
...
//depot/googleclient/gears/opensource/gears/installer/common/cab_updater.cc#2
edit
...
//depot/googleclient/gears/opensource/gears/installer/common/cab_updater.h#2
edit
...
//depot/googleclient/gears/opensource/gears/installer/common/download_task.cc#2
edit
...
//depot/googleclient/gears/opensource/gears/installer/common/download_task.h#2
edit
...
//depot/googleclient/gears/opensource/gears/installer/common/periodic_checker.cc#3
edit
...
//depot/googleclient/gears/opensource/gears/installer/common/periodic_checker.h#4
edit
57 delta lines: 34 added, 1 deleted, 22 changed
Also consider running:
g4 lint -c 9877173
which verifies that the changelist doesn't introduce new style violations.
If you can't do the review, please let me know as soon as possible. During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately. Visit
http://www/eng/code_review.html for more information.
This is a semiautomated message from "g4 mail". Complaints or suggestions?
Mail [email protected].
Change 9877173 by stevebl...@steveblock-gears1 on 2009/01/27 15:56:31 *pending*
Modifies CabUpdater to pass a BrowsingContext through to the AsyncTask
used to check for updates and to download the CAB. Mailed on behalf of Opera.
Affected files ...
...
//depot/googleclient/gears/opensource/gears/installer/common/cab_updater.cc#2
edit
...
//depot/googleclient/gears/opensource/gears/installer/common/cab_updater.h#2
edit
...
//depot/googleclient/gears/opensource/gears/installer/common/download_task.cc#2
edit
...
//depot/googleclient/gears/opensource/gears/installer/common/download_task.h#2
edit
...
//depot/googleclient/gears/opensource/gears/installer/common/periodic_checker.cc#3
edit
...
//depot/googleclient/gears/opensource/gears/installer/common/periodic_checker.h#4
edit
====
//depot/googleclient/gears/opensource/gears/installer/common/cab_updater.cc#2 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/installer/common/cab_updater.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/installer/common/cab_updater.cc
2009-01-27 17:14:38.000000000 +0000
+++ googleclient/gears/opensource/gears/installer/common/cab_updater.cc
2009-01-27 15:56:32.000000000 +0000
@@ -40,15 +40,20 @@
const char16 *kGearsCabName = L"gears-wince-opt.cab";
// The delay before the first update.
+#ifdef BROWSER_OPERA
+const int32 kFirstUpdatePeriod = (1000 * 2); // 2 seconds
+#else
const int32 kFirstUpdatePeriod = (1000 * 60 * 2); // 2 minutes
+#endif
// The time interval between the rest of the update checks.
const int32 kUpdatePeriod = (1000 * 60 * 60 * 24); // 24 hours
// A grace period after a connection event. Setting up a new connection
// can take time so we wait a little after the event.
const int32 kGracePeriod = (1000 * 30); // 30 seconds
-CabUpdater::CabUpdater()
- : checker_(PeriodicChecker::CreateChecker()),
+CabUpdater::CabUpdater(BrowsingContext *browsing_context)
+ : checker_(PeriodicChecker::CreateChecker(browsing_context)),
+ browsing_context_(browsing_context),
is_showing_update_dialog_(false),
download_task_(NULL) {
}
@@ -124,7 +129,12 @@
}
bool CabUpdater::ShowUpdateAvailableDialog() {
+#ifdef BROWSER_OPERA
+ HMODULE module = GetModuleHandle(TEXT("gearsop")); // [naming]
+#else
HMODULE module = GetModuleHandle(PRODUCT_SHORT_NAME);
+#endif
+
if (!module) return false;
// Load the dialog strings
LPCTSTR message = reinterpret_cast<LPCTSTR>(
@@ -147,7 +157,12 @@
}
bool CabUpdater::ShowInstallationFailedDialog() {
+#ifdef BROWSER_OPERA
+ HMODULE module = GetModuleHandle(TEXT("gearsop")); // [naming]
+#else
HMODULE module = GetModuleHandle(PRODUCT_SHORT_NAME);
+#endif
+
if (!module) return false;
// Load the dialog strings
LPCTSTR message = reinterpret_cast<LPCTSTR>(
@@ -177,7 +192,7 @@
if (File::GetBaseTemporaryDirectory(&temp_file_path_)) {
temp_file_path_ += kGearsCabName;
download_task_ =
- DownloadTask::Create(url.c_str(), temp_file_path_.c_str(), this);
+ DownloadTask::Create(url.c_str(), temp_file_path_.c_str(), this,
browsing_context_.get());
}
}
}
====
//depot/googleclient/gears/opensource/gears/installer/common/cab_updater.h#2 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/installer/common/cab_updater.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/installer/common/cab_updater.h
2009-01-27 17:14:39.000000000 +0000
+++ googleclient/gears/opensource/gears/installer/common/cab_updater.h
2009-01-27 15:56:35.000000000 +0000
@@ -49,7 +49,8 @@
public PeriodicChecker::ListenerInterface,
public DownloadTask::ListenerInterface {
public:
- CabUpdater();
+ CabUpdater(BrowsingContext *browsing_context = NULL);
+
virtual ~CabUpdater();
// Starts the updater. guid is the GUID used in the request query parameters.
@@ -93,6 +94,8 @@
// The temporary file that will be used for the downloaded CAB.
std::string16 temp_file_path_;
+ scoped_refptr<BrowsingContext> browsing_context_;
+
DISALLOW_EVIL_CONSTRUCTORS(CabUpdater);
};
====
//depot/googleclient/gears/opensource/gears/installer/common/download_task.cc#2
-
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/installer/common/download_task.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/installer/common/download_task.cc
2009-01-27 17:14:39.000000000 +0000
+++ googleclient/gears/opensource/gears/installer/common/download_task.cc
2009-01-27 15:56:37.000000000 +0000
@@ -34,8 +34,9 @@
// static
DownloadTask *DownloadTask::Create(const char16* url,
const char16* save_path,
- ListenerInterface *listener) {
- scoped_ptr<DownloadTask> task(new DownloadTask(url, save_path, listener));
+ ListenerInterface *listener,
+
BrowsingContext *browsing_context) {
+ scoped_ptr<DownloadTask> task(new DownloadTask(url, save_path, listener,
browsing_context));
assert(task.get());
if (!task.get() || !task->Init() || !task->Start()) {
return NULL;
@@ -50,8 +51,9 @@
DownloadTask::DownloadTask(const char16* url,
const char16* save_path,
- ListenerInterface *listener)
- : AsyncTask(NULL),
+ ListenerInterface *listener,
+ BrowsingContext
*browsing_context)
+ : AsyncTask(browsing_context),
url_(url),
save_path_(save_path),
listener_(listener) {
====
//depot/googleclient/gears/opensource/gears/installer/common/download_task.h#2
-
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/installer/common/download_task.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/installer/common/download_task.h
2009-01-27 17:14:39.000000000 +0000
+++ googleclient/gears/opensource/gears/installer/common/download_task.h
2009-01-27 15:56:40.000000000 +0000
@@ -43,14 +43,16 @@
static DownloadTask *Create(const char16 *url,
const char16 *save_path,
- ListenerInterface *listener);
+ ListenerInterface *listener,
+ BrowsingContext
*browsing_context = NULL);
void StopThreadAndDelete();
private:
// Use Create and StopThreadAndDelete to create and destroy
DownloadTask(const char16 *url,
const char16 *save_path,
- ListenerInterface *listener);
+ ListenerInterface *listener,
+ BrowsingContext *browsing_context = NULL);
virtual ~DownloadTask() {}
// AsyncTask implementation
====
//depot/googleclient/gears/opensource/gears/installer/common/periodic_checker.cc#3
-
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/installer/common/periodic_checker.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/installer/common/periodic_checker.cc
2009-01-27 17:14:39.000000000 +0000
+++ googleclient/gears/opensource/gears/installer/common/periodic_checker.cc
2009-01-27 15:56:42.000000000 +0000
@@ -33,6 +33,8 @@
#include "gears/localserver/common/async_task.h"
#include "gears/localserver/common/http_constants.h"
#include "genfiles/product_constants.h"
+#include "gears/base/common/browsing_context.h"
+
const char16* kUpgradeUrl = L"http://tools.google.com/service/update2/ff?";
L"version=" PRODUCT_VERSION_STRING
@@ -73,7 +75,8 @@
public:
// Factory method
static VersionFetchTask *Create(const std::string16 &guid,
- HWND listener_window);
+ HWND listener_window,
+
BrowsingContext *browsing_context = NULL);
// Signals the worker thread to stop and asynchronously deletes the object.
// No further messages will be sent to the listener once a call to this
method
@@ -85,7 +88,8 @@
private:
// Use Create to create a new instance and StopThreadAndDelete to destroy.
- VersionFetchTask(const std::string16 &guid, HWND listener_window);
+ VersionFetchTask(const std::string16 &guid, HWND listener_window,
BrowsingContext *browsing_context = NULL);
+
virtual ~VersionFetchTask() {}
// AsyncTask implementation
@@ -107,8 +111,8 @@
// static
-PeriodicChecker* PeriodicChecker::CreateChecker() {
- return new PeriodicChecker();
+PeriodicChecker* PeriodicChecker::CreateChecker(BrowsingContext
*browsing_context) {
+ return new PeriodicChecker(browsing_context);
}
bool PeriodicChecker::Init(int32 first_period,
@@ -125,7 +129,7 @@
(TRUE == thread_complete_event_.Create(NULL, FALSE, FALSE, NULL));
}
-PeriodicChecker::PeriodicChecker()
+PeriodicChecker::PeriodicChecker(BrowsingContext *browsing_context)
: first_period_(0),
normal_period_(0),
grace_period_(0),
@@ -136,6 +140,7 @@
timer_(1),
task_(NULL),
window_(NULL),
+ browsing_context_(browsing_context),
normal_timer_event_(false) {
}
@@ -239,7 +244,7 @@
// Depending on how the periodic checker is configured, this can fire
// while a task is running. We ignore such events.
if (!task_) {
- task_ = VersionFetchTask::Create(guid_, window_);
+ task_ = VersionFetchTask::Create(guid_, window_, browsing_context_.get());
}
handled = TRUE;
return TRUE;
@@ -300,9 +305,12 @@
// static
VersionFetchTask *VersionFetchTask::Create(const std::string16 &guid,
- HWND listener_window) {
+ HWND listener_window,
+
BrowsingContext *browsing_context) {
+
VersionFetchTask *task =
- new VersionFetchTask(guid, listener_window);
+ new VersionFetchTask(guid, listener_window, browsing_context);
+
if (!task) {
assert(false);
return NULL;
@@ -315,8 +323,9 @@
}
VersionFetchTask::VersionFetchTask(const std::string16 &guid,
- HWND listener_window)
- : AsyncTask(NULL),
+ HWND listener_window,
+
BrowsingContext* browsing_context)
+ : AsyncTask(browsing_context),
guid_(guid),
listener_window_(listener_window) {
assert(listener_window_);
====
//depot/googleclient/gears/opensource/gears/installer/common/periodic_checker.h#4
-
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/installer/common/periodic_checker.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/installer/common/periodic_checker.h
2009-01-27 17:14:40.000000000 +0000
+++ googleclient/gears/opensource/gears/installer/common/periodic_checker.h
2009-01-27 15:56:45.000000000 +0000
@@ -36,6 +36,7 @@
#include "gears/base/common/atl_headers_win32.h"
#include <gears/base/common/string16.h>
#include "gears/base/common/wince_compatibility.h"
+#include "gears/base/common/browsing_context.h"
// An implemnentation detail of PeriodicChecker.
class VersionFetchTask;
@@ -48,7 +49,7 @@
};
// Simple factory method.
- static PeriodicChecker* CreateChecker();
+ static PeriodicChecker* CreateChecker(BrowsingContext *browsing_context =
NULL);
// Initializes the checker and sets the desired times between update checks.
// Also sets the application ID sent to the server for update checks and the
// listener for callbacks.
@@ -76,8 +77,7 @@
END_MSG_MAP()
// Internal
- PeriodicChecker();
-
+ PeriodicChecker(BrowsingContext *browsing_context = NULL);
// Message handlers.
LRESULT OnTimer(UINT message, WPARAM unused1, LPARAM unused2, BOOL &handled);
LRESULT OnFetchTaskComplete(UINT message, WPARAM success, LPARAM unused,
@@ -124,6 +124,8 @@
// The message ID used for listening to connection status events.
uint32 con_mgr_msg_;
+ scoped_refptr<BrowsingContext> browsing_context_;
+
DISALLOW_EVIL_CONSTRUCTORS(PeriodicChecker);
};
