Mountain Lion: use the system download progress.

chrome/browser/download/download_status_updater_mac.mm

Index: chrome/browser/download/download_status_updater_mac.mm
diff --git a/chrome/browser/download/download_status_updater_mac.mm b/chrome/browser/download/download_status_updater_mac.mm
new file mode 100644
index 0000000000000000000000000000000000000000..7464b027414c94ff602e5b7cf3fce10a7c88fe32
--- /dev/null
+++ b/chrome/browser/download/download_status_updater_mac.mm
@@ -0,0 +1,245 @@
+// Copyright (c) 2012 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/download/download_status_updater.h"
+
+#include "base/memory/scoped_nsobject.h"
+#include "base/supports_user_data.h"
+#include "base/sys_string_conversions.h"
+#include "content/public/browser/download_item.h"
+#include "googleurl/src/gurl.h"
+
+// --- Private 10.8 API for showing progress ---

[Nico, 2012/08/08 20:00:59]

Can you file a rdar about turning this into public api and then link to that
here? There's clearly a use case for it :-)

[Avi (use Gerrit), 2012/08/08 21:16:20]

Done.

+
+namespace {
+
+NSString* kNSProgressAppBundleIdentifierKey =

[Nico, 2012/08/08 20:00:59]

NSString* const

[Avi (use Gerrit), 2012/08/08 21:16:20]

Done.

+    @"NSProgressAppBundleIdentifierKey";
+NSString* kNSProgressEstimatedTimeKey =
+    @"NSProgressEstimatedTimeKey";
+NSString* kNSProgressFileCompletedCountKey =
+    @"NSProgressFileCompletedCountKey";
+NSString* kNSProgressFileContainerURLKey =
+    @"NSProgressFileContainerURLKey";
+NSString* kNSProgressFileDownloadingSourceURLKey =
+    @"NSProgressFileDownloadingSourceURLKey";
+NSString* kNSProgressFileIconKey =
+    @"NSProgressFileIconKey";
+NSString* kNSProgressFileIconOriginalRectKey =
+    @"NSProgressFileIconOriginalRectKey";
+NSString* kNSProgressFileLocationCanChangeKey =
+    @"NSProgressFileLocationCanChangeKey";
+NSString* kNSProgressFileOperationKindAirDropping =
+    @"NSProgressFileOperationKindAirDropping";
+NSString* kNSProgressFileOperationKindCopying =
+    @"NSProgressFileOperationKindCopying";
+NSString* kNSProgressFileOperationKindDecompressingAfterDownloading =
+    @"NSProgressFileOperationKindDecompressingAfterDownloading";
+NSString* kNSProgressFileOperationKindDownloading =
+    @"NSProgressFileOperationKindDownloading";
+NSString* kNSProgressFileOperationKindEncrypting =
+    @"NSProgressFileOperationKindEncrypting";
+NSString* kNSProgressFileOperationKindKey =
+    @"NSProgressFileOperationKindKey";
+NSString* kNSProgressFileTotalCountKey =
+    @"NSProgressFileTotalCountKey";
+NSString* kNSProgressFileURLKey =
+    @"NSProgressFileURLKey";
+NSString* kNSProgressIsWaitingKey =
+    @"NSProgressIsWaitingKey";
+NSString* kNSProgressKindFile =
+    @"NSProgressKindFile";
+NSString* kNSProgressThroughputKey =
+    @"NSProgressThroughputKey";
+
+NSString* ProgressString(NSString* string) {
+  static NSMutableDictionary* cache;
+  if (!cache)
+    cache = [[NSMutableDictionary alloc] init];
+  if (![cache objectForKey:string]) {
+    CFBundleRef foundation = CFBundleGetBundleWithIdentifier(
+        CFSTR("com.apple.Foundation"));
+    NSString** ref = static_cast<NSString**>(
+        CFBundleGetDataPointerForName(foundation, (CFStringRef)string));

[Nico, 2012/08/08 20:00:59]

we have a type-safe casting function somewhere in base (foundation_util.h I
think?)

[Avi (use Gerrit), 2012/08/08 21:16:20]

Done.

+    if (ref)
+      [cache setObject:*ref

[Nico, 2012/08/08 20:00:59]

1 line (or braces)

[Avi (use Gerrit), 2012/08/08 21:16:20]

Done.

+                forKey:string];
+  }
+
+  return [cache objectForKey:string];

[Nico, 2012/08/08 20:00:59]

You can avoid the two cache lookups by writing it like this:

  NSString* result = [cache objectForKey:string];
  if (!result) {
    NSString** ref = ...
    if (ref) {
      result = *ref;
      [cache setObject:result forKey:string];
    }
  }
  return result;
}

[Avi (use Gerrit), 2012/08/08 21:16:20]

Done.

+}
+
+}  // namespace
+
+@interface NSProgress : NSObject
+
+- (id)initWithParent:(id)parent userInfo:(NSDictionary*)info;
+@property(copy) NSString *kind;

[Nico, 2012/08/08 20:00:59]

space after *

[Avi (use Gerrit), 2012/08/08 21:16:20]

Done.

+
+- (void)unpublish;
+- (void)publish;
+
+- (void)setUserInfoObject:(id)object forKey:(NSString*)key;
+- (NSDictionary*)userInfo;
+
+@property(readonly) double fractionCompleted;
+// Set the totalUnitCount to -1 to indicate an indeterminate download. The dock
+// shows a non-filling progress bar; the Finder is lame and draws its progress
+// bar off the right side.
+@property(readonly, getter=isIndeterminate) BOOL indeterminate;
+@property long long completedUnitCount;
+@property long long totalUnitCount;
+
+// Pausing appears to be unimplemented in 10.8.0.
+- (void)pause;
+@property(readonly, getter=isPaused) BOOL paused;
+@property(getter=isPausable) BOOL pausable;
+- (void)setPausingHandler:(id)blockOfUnknownSignature;
+
+- (void)cancel;
+@property(readonly, getter=isCancelled) BOOL cancelled;
+@property(getter=isCancellable) BOOL cancellable;
+// Note that the cancellation handler block will be called on a random thread.
+- (void)setCancellationHandler:(void (^)())block;
+
+// Allows other applications to provide feedback as to whether the progress is
+// visible in that app. Note that the acknowledgement handler block will be
+// called on a random thread.
+// com.apple.dock => BOOL indicating whether the download target folder was
+//                   successfully "flown to" at the beginning of the download.
+//                   This primarily depends on whether the download target
+//                   folder is in the dock. Note that if the download target
+//                   folder is added or removed from the dock during the
+//                   duration of the download, it will not trigger a callback.
+//                   Note that if the "fly to the dock" keys were not set, the
+//                   callback's parameter will always be NO.
+// com.apple.Finder => always YES, no matter whether the download target
+//                     folder's window is open.
+- (void)handleAcknowledgementByAppWithBundleIdentifier:(NSString*)bundle
+                                            usingBlock:(void (^)(BOOL success))block;

[Avi (use Gerrit), 2012/08/08 19:34:28]

Yes, this is > 80. I can't think how to format it cleanly to not be so.

[Nico, 2012/08/08 20:00:59]

Just indent this line by 4 spaces instead. We do this in a few other places
already. (And/or add a typedef for the block, but I don't think that's
necessary)

[Avi (use Gerrit), 2012/08/08 21:16:20]

Done.

+
+@end
+
+// --- Private 10.8 API for showing progress ---
+
+namespace {
+
+bool NSProgressSupported() {
+  static bool supported;
+  static bool valid;
+  if (!valid) {
+    supported = NSClassFromString(@"NSProgress");
+    valid = true;
+  }
+
+  return supported;
+}
+
+}  // namespace
+
+const char kNSProgressUserDataKey[] = "NSProgressUserData";
+
+class NSProgressUserData : public base::SupportsUserData::Data {

[Nico, 2012/08/08 20:00:59]

CrNSProgessUserData?

[Avi (use Gerrit), 2012/08/08 21:16:20]

Moved to anon namespace.

[Nico, 2012/08/08 21:21:38]

I'd still call it CrNSProgressUserData, I consider all NSFoo to be in apple's
namespace. But I don't feel strongly, so keep it as is if you prefer that.

[Avi (use Gerrit), 2012/08/08 21:22:46]

Even for C++ classes?

[Nico, 2012/08/08 21:27:23]

Even for poetry.

+ public:
+  NSProgressUserData(NSProgress* progress, const FilePath& target)
+      : target_(target) {
+    progress_.reset(progress);
+  }
+  virtual ~NSProgressUserData() {}
+
+  NSProgress* progress() const { return progress_.get(); }
+  FilePath target() const { return target_; }
+  void setTarget(const FilePath& target) { target_ = target; }
+
+ private:
+  scoped_nsobject<NSProgress> progress_;
+  FilePath target_;
+};
+
+void DownloadStatusUpdater::UpdateDownloadProgressForItemAdded(
+    content::DownloadItem* download) {
+  if (!NSProgressSupported())
+    return;
+
+  NSURL* source_url = [NSURL URLWithString:
+      base::SysUTF8ToNSString(download->GetURL().spec())];

[Nico, 2012/08/08 20:00:59]

Huh, amazing that we don't have something nicer for nsurl <-> gurl conversions!
Oh well :-)

+  FilePath destination_path = download->GetFullPath();
+  NSURL* destination_url = [NSURL fileURLWithPath:
+      [NSString stringWithUTF8String:destination_path.value().c_str()]];

[Nico, 2012/08/08 20:00:59]

This gets a tiny bit shorter with base::mac::FilePathToNSString()

[Avi (use Gerrit), 2012/08/08 21:16:20]

Ooh!

+
+  // If there were an image to fly to the download folder in the dock, then
+  // the keys in the userInfo to set would be:
+  // - @"NSProgressFlyToImageKey" : NSImage
+  // - kNSProgressFileIconOriginalRectKey : NSValue of NSRect in global coords
+
+  NSDictionary* user_info = @{
+    ProgressString(kNSProgressFileDownloadingSourceURLKey) : source_url,
+    ProgressString(kNSProgressFileLocationCanChangeKey) : @true,

[Avi (use Gerrit), 2012/08/08 19:34:28]

@YES is broken for the SDK that we compile against.

[Nico, 2012/08/08 20:00:59]

@(YES) might work

(why "YES" though? Do you know what this key means?)

[Avi (use Gerrit), 2012/08/08 21:16:20]

Doesn't that determine at runtime what the type of the expression between the @(
and ) are? In any case, Safari sets that key. I'm setting it too.

Since @YES and @true are equivalent, I'm sticking to this.

+    ProgressString(kNSProgressFileOperationKindKey) :
+      ProgressString(kNSProgressFileOperationKindDownloading),
+    ProgressString(kNSProgressFileURLKey) : destination_url
+  };
+  Class progress_class = NSClassFromString(@"NSProgress");
+  NSProgress* progress = [progress_class performSelector:@selector(alloc)];
+  progress = [progress performSelector:@selector(initWithParent:userInfo:)
+                            withObject:nil
+                            withObject:user_info];
+  progress.kind = ProgressString(kNSProgressKindFile);
+
+  progress.pausable = NO;
+  progress.cancellable = YES;
+  [progress setCancellationHandler:^{
+      dispatch_async(dispatch_get_main_queue(), ^{
+          download->Cancel(true);

[Nico, 2012/08/08 20:00:59]

rdsmith: What are the guarantees for DownloadItem lifetime?

[Randy Smith (Not in Mondays), 2012/08/08 20:39:40]

If you're an observer of the DownloadItem, you'll get a notification when it's
dying.  Currently you have to catch passage through the REMOVING state, but
there's a CL in process ( http://codereview.chromium.org/10704026, but don't be
fooled by the apparent commit at the end of it--it's been landed and reverted,
and will probably be re-landed soon) to change that over to an
OnDownloadDestroyed notification to the observer.  REMOVING != IN_PROGRESS, so
this CL is good (and in general the updater should be updated as we change
that).

Avi, you should be aware of possible merge issues if the CL mentioned above goes
in before this one--it won't be a hard merge, it would just be good to make sure
it happens properly.

[Avi (use Gerrit), 2012/08/08 21:16:20]

As you noted, the DSU code would need to be updated too, and this piggybacks on
it. What should I be watching out for?

[Randy Smith (Not in Mondays), 2012/08/08 21:27:42]

A glitch in the DSU code update :-}.  The DSU code update may substantially move
where the removal from the tracked list occurs (since it's under a different
entry point), and that'll probably get a delete/change merge conflict for
you--I'm mostly just giving you a heads up about it.

(Just checked the other CL and it hadn't modified DSU at all.  Which is ok,
since we always move from IN_PROGRESS->CANCELLED/INTERRUPTED before destruction,
but I've still requested hooking OnDownloadDestroyed.)

+      });
+  }];
+
+  progress.totalUnitCount = download->GetTotalBytes();
+  progress.completedUnitCount = download->GetReceivedBytes();
+
+  [progress publish];
+
+  download->SetUserData(&kNSProgressUserDataKey,
+                        new NSProgressUserData(progress, destination_path));
+}
+
+void DownloadStatusUpdater::UpdateDownloadProgressForItemProgressed(
+    content::DownloadItem* download) {
+  if (!NSProgressSupported())
+    return;
+
+  NSProgressUserData* progress_data = static_cast<NSProgressUserData*>(
+      download->GetUserData(&kNSProgressUserDataKey));
+  if (!progress_data)
+    return;
+
+  NSProgress* progress = progress_data->progress();
+  progress.totalUnitCount = download->GetTotalBytes();
+  progress.completedUnitCount = download->GetReceivedBytes();
+
+  FilePath download_path = download->GetFullPath();
+  if (progress_data->target() != download_path) {
+    progress_data->setTarget(download_path);
+    NSURL* download_url = [NSURL fileURLWithPath:
+        [NSString stringWithUTF8String:download_path.value().c_str()]];

[Nico, 2012/08/08 20:00:59]

same as above

[Avi (use Gerrit), 2012/08/08 21:16:20]

Done.

+    [progress setUserInfoObject:download_url
+                         forKey:ProgressString(kNSProgressFileURLKey)];
+  }
+}
+
+void DownloadStatusUpdater::UpdateDownloadProgressForItemRemoved(
+    content::DownloadItem* download) {
+  if (!NSProgressSupported())
+    return;
+
+  NSProgressUserData* progress_data = static_cast<NSProgressUserData*>(
+      download->GetUserData(&kNSProgressUserDataKey));
+  if (!progress_data)
+    return;
+
+  NSProgress* progress = progress_data->progress();
+  [progress unpublish];
+
+  download->RemoveUserData(&kNSProgressUserDataKey);
+}