Hello andreip,
I'd like you to do a code review. Please execute
g4 diff -c 9107700
or point your web browser to
http://mondrian/9107700
(this changelist has been uploaded to Mondrian)
to review the following code:
Change 9107700 by [EMAIL PROTECTED] on 2008/11/21 15:59:26 *pending*
Add release logging of auto-updates on Android.
This adds RELEASE_LOG() which is defined for both debug and
non-debug builds. Only rare/critical items should use
this. For now, this is:
* Start of an auto-update download (if new version found).
* Successful installation of an auto-update (unzip).
* NP_Shutdown() being called (only happens when on-the-fly
plugin update occurs)
* Stale directory clean-up occurs (which implies a new version
just started for the first time).
PRESUBMIT=passed
R=andreip
[EMAIL PROTECTED]
DELTA=23 (21 added, 0 deleted, 2 changed)
OCL=9107700
Affected files ...
... //depot/googleclient/gears/opensource/gears/base/common/common_np.h#7 edit
... //depot/googleclient/gears/opensource/gears/base/npapi/module_android.cc#1
edit
...
//depot/googleclient/gears/opensource/gears/installer/android/lib_updater.cc#4
edit
23 delta lines: 21 added, 0 deleted, 2 changed
Also consider running:
g4 lint -c 9107700
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 9107700 by [EMAIL PROTECTED] on 2008/11/21 15:59:26 *pending*
Add release logging of auto-updates on Android.
This adds RELEASE_LOG() which is defined for both debug and
non-debug builds. Only rare/critical items should use
this. For now, this is:
* Start of an auto-update download (if new version found).
* Successful installation of an auto-update (unzip).
* NP_Shutdown() being called (only happens when on-the-fly
plugin update occurs)
* Stale directory clean-up occurs (which implies a new version
just started for the first time).
OCL=9107700
Affected files ...
... //depot/googleclient/gears/opensource/gears/base/common/common_np.h#7 edit
... //depot/googleclient/gears/opensource/gears/base/npapi/module_android.cc#1
edit
...
//depot/googleclient/gears/opensource/gears/installer/android/lib_updater.cc#4
edit
==== //depot/googleclient/gears/opensource/gears/base/common/common_np.h#7 -
/usr/local/google/home/jripley/gears-trunk/googleclient/gears/opensource/gears/base/common/common_np.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/common_np.h 2008-11-21
15:59:32.000000000 +0000
+++ googleclient/gears/opensource/gears/base/common/common_np.h 2008-11-21
15:24:36.000000000 +0000
@@ -82,6 +82,10 @@
#endif
+// Logging even in release builds for rare and/or critical items.
+#define RELEASE_LOG(args) \
+ do { LOG_INNER args ; } while (0)
+
#else // !ANDROID
//-----------------------------------------------------------------------------
==== //depot/googleclient/gears/opensource/gears/base/npapi/module_android.cc#1
-
/usr/local/google/home/jripley/gears-trunk/googleclient/gears/opensource/gears/base/npapi/module_android.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/npapi/module_android.cc
2008-11-21 15:59:32.000000000 +0000
+++ googleclient/gears/opensource/gears/base/npapi/module_android.cc
2008-11-21 15:21:40.000000000 +0000
@@ -237,6 +237,11 @@
}
NPError NP_Shutdown() {
+ // This shutdown function is not normally called as the Browser
+ // never exits unless killed. Auto-update is the only likely reason
+ // for this to be called, in which case we want to print a log
+ // message even in release builds.
+ RELEASE_LOG(("Gears shutting down\n"));
// Stop the update checks.
LibUpdater::StopUpdateChecks();
// Shutdown URL interception.
====
//depot/googleclient/gears/opensource/gears/installer/android/lib_updater.cc#4
-
/usr/local/google/home/jripley/gears-trunk/googleclient/gears/opensource/gears/installer/android/lib_updater.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/installer/android/lib_updater.cc
2008-11-21 15:59:32.000000000 +0000
+++ googleclient/gears/opensource/gears/installer/android/lib_updater.cc
2008-11-21 15:23:53.000000000 +0000
@@ -123,7 +123,10 @@
if (Inflate(download_task_->Filename())) {
// Move the files and update the symlink. This should not leave
// any stale Gears directory behind if it fails.
- if (!MoveFromTempAndUpdateSymlink(download_task_->Version())) {
+ if (MoveFromTempAndUpdateSymlink(download_task_->Version())) {
+ // Print a message even in release builds.
+ RELEASE_LOG(("Installed Gears %s\n", download_task_->Version()));
+ } else {
LOG(("Failed to move files"));
}
} else {
@@ -215,7 +218,8 @@
// Set ourselves as the listener.
download_task_->SetListener(this);
- // Start the task.
+ // Start the task. Print a message even in release builds.
+ RELEASE_LOG(("Downloading Gears %s\n", version_check_task_->Version()));
return download_task_->Start();
}
@@ -482,6 +486,14 @@
result &= File::DeleteRecursively(dirs_to_delete[i].c_str());
}
+ if (!dirs_to_delete.empty()) {
+ // If we deleted any stale directories, we've just got initialized
+ // after an auto-update. Print this important fact even in release
+ // builds.
+ RELEASE_LOG(("New Gears running, version %s\n",
+ PRODUCT_VERSION_STRING_ASCII));
+ }
+
return result;
}
