Rewrite implementation of ClipboardRecentContent in Objective C.

components/open_from_clipboard/clipboard_recent_content_ios_impl.mm

Index: components/open_from_clipboard/clipboard_recent_content_ios_impl.mm
diff --git a/components/open_from_clipboard/clipboard_recent_content_ios_impl.mm b/components/open_from_clipboard/clipboard_recent_content_ios_impl.mm
new file mode 100644
index 0000000000000000000000000000000000000000..9ad6f14a1fe5be618e487ac156863d4dca5bf035
--- /dev/null
+++ b/components/open_from_clipboard/clipboard_recent_content_ios_impl.mm
@@ -0,0 +1,233 @@
+// Copyright 2017 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.
+
+#import "components/open_from_clipboard/clipboard_recent_content_ios_impl.h"
+
+#import <CommonCrypto/CommonDigest.h>
+#import <UIKit/UIKit.h>
+
+#include "base/strings/sys_string_conversions.h"
+#include "base/sys_info.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+namespace {
+// Key used to store the pasteboard's current change count. If when resuming
+// chrome the pasteboard's change count is different from the stored one, then
+// it means that the pasteboard's content has changed.
+NSString* kPasteboardChangeCountKey = @"PasteboardChangeCount";
+// Key used to store the last date at which it was detected that the pasteboard
+// changed. It is used to evaluate the age of the pasteboard's content.
+NSString* kPasteboardChangeDateKey = @"PasteboardChangeDate";
+// Key used to store the hash of the content of the pasteboard. Whenever the
+// hash changed, the pasteboard content is considered to have changed.
+NSString* kPasteboardEntryMD5Key = @"PasteboardEntryMD5";
+// Maximum age of clipboard in seconds.
+NSTimeInterval kMaximumAgeOfClipboard = 3 * 60 * 60;
+
+// Compute a hash consisting of the first 4 bytes of the MD5 hash of |string|.
+// This value is used to detect pasteboard content change. Keeping only 4 bytes
+// is a privacy requirement to introduce collision and allow deniability of
+// having copied a given string.
+NSData* WeakMD5FromNSString(NSString* string) {
+  unsigned char hash[CC_MD5_DIGEST_LENGTH];
+  const std::string clipboard = base::SysNSStringToUTF8(string);
+  const char* c_string = clipboard.c_str();
+  CC_MD5(c_string, strlen(c_string), hash);
+  NSData* data = [NSData dataWithBytes:hash length:4];
+  return data;
+}
+
+}  // namespace
+
+@interface ClipboardRecentContentIOSImpl ()
+
+// The user defaults from the app group used to optimize the pasteboard change
+// detection.
+@property(nonatomic, strong) NSUserDefaults* sharedUserDefaults;
+// The pasteboard's change count. Increases everytime the pasteboard changes.
+@property(nonatomic) NSInteger lastPasteboardChangeCount;
+// MD5 hash of the last registered pasteboard entry.
+@property(nonatomic, strong) NSData* lastPasteboardEntryMD5;
+// Contains the authorized schemes for URLs.
+@property(nonatomic, strong) NSArray* authorizedSchemes;
+// Delegate for metrics.
+@property(nonatomic, strong) id<ClipboardRecentContentMetricsDelegate> delegate;
+
+// If the content of the pasteboard has changed, updates the change count,
+// change date, and md5 of the latest pasteboard entry if necessary.
+- (void)updateIfNeeded;
+
+// Returns whether the pasteboard changed since the last time a pasteboard
+// change was detected.
+- (BOOL)hasPasteboardChanged;
+
+// Loads information from the user defaults about the latest pasteboard entry.
+- (void)loadFromUserDefaults;
+
+// Returns the URL contained in the clipboard (if any).
+- (NSURL*)URLFromPasteboard;
+
+// Returns the uptime.
+- (NSTimeInterval)uptime;
+
+@end
+
+@implementation ClipboardRecentContentIOSImpl
+
+@synthesize lastPasteboardChangeCount = _lastPasteboardChangeCount;
+@synthesize lastPasteboardChangeDate = _lastPasteboardChangeDate;
+@synthesize lastPasteboardEntryMD5 = _lastPasteboardEntryMD5;
+@synthesize sharedUserDefaults = _sharedUserDefaults;
+@synthesize authorizedSchemes = _authorizedSchemes;
+@synthesize delegate = _delegate;
+
+- (instancetype)initWithDelegate:
+                    (id<ClipboardRecentContentMetricsDelegate>)delegate
+               authorizedSchemes:(NSArray*)authorizedSchemes
+                    userDefaults:(NSUserDefaults*)groupUserDefaults {
+  self = [super init];
+  if (self) {
+    _delegate = delegate;
+    _authorizedSchemes = authorizedSchemes;
+    _sharedUserDefaults = groupUserDefaults;
+
+    _lastPasteboardChangeCount = NSIntegerMax;
+    [self loadFromUserDefaults];
+    [self updateIfNeeded];
+
+    // Makes sure |last_pasteboard_change_count_| was properly initialized.
+    DCHECK_NE(_lastPasteboardChangeCount, NSIntegerMax);
+    [[NSNotificationCenter defaultCenter]
+        addObserver:self
+           selector:@selector(didBecomeActive:)
+               name:UIApplicationDidBecomeActiveNotification
+             object:nil];
+  }
+  return self;
+}
+
+- (void)dealloc {
+  [[NSNotificationCenter defaultCenter] removeObserver:self];
+}
+
+- (void)didBecomeActive:(NSNotification*)notification {
+  [self loadFromUserDefaults];
+  [self updateIfNeeded];
+}
+
+- (BOOL)hasPasteboardChanged {
+  // If |MD5Changed|, we know for sure there has been at least one pasteboard
+  // copy since last time it was checked.
+  // If the pasteboard content is still the same but the device was not
+  // rebooted, the change count can be checked to see if it changed.
+  // Note: due to a mismatch between the actual behavior and documentation, and
+  // lack of consistency on different reboot scenarios, the change count cannot
+  // be checked after a reboot.
+  // See radar://21833556 for more information.
+  NSInteger changeCount = [UIPasteboard generalPasteboard].changeCount;

[sdefresne, 2017/04/03 09:27:17]

It looks like changeCount and changeCountChanged are only used if "notRebooted"
is true, maybe move them into the corresponding if block.

[lody, 2017/04/04 13:42:18]

Done.

+  bool changeCountChanged = changeCount != self.lastPasteboardChangeCount;
+
+  bool notRebooted = [self uptime] > [self getClipboardContentAge];

[sdefresne, 2017/04/03 09:27:17]

Avoid using negative names for you variables. Instead you can name if
"deviceRebooted" by changing the test:

  bool deviceRebooted = [self getClipboardContentAge] >= [self uptime];
  if (!deviceRebooted)
    return changeCountChanged;

[lody, 2017/04/04 13:42:18]

Done.

+  if (notRebooted)
+    return changeCountChanged;
+
+  NSString* pasteboardString = [UIPasteboard generalPasteboard].string;
+  if (!pasteboardString) {

[sdefresne, 2017/04/03 09:27:17]

No need to check for nil here, WeakMD5FromNSString supports nil NSString* (as
base::SysNSStringToUTF8 does).

[lody, 2017/04/04 13:42:18]

Done.

+    pasteboardString = @"";
+  }
+  NSData* md5 = WeakMD5FromNSString(pasteboardString);
+  BOOL md5Changed = ![md5 isEqualToData:self.lastPasteboardEntryMD5];
+
+  return md5Changed;
+}
+
+- (NSURL*)getRecentURLFromClipboard {
+  [self updateIfNeeded];
+  if ([self getClipboardContentAge] > kMaximumAgeOfClipboard) {
+    return nil;
+  }
+
+  NSURL* urlFromPasteboard = [self URLFromPasteboard];
+  if (urlFromPasteboard) {
+    return urlFromPasteboard;
+  }
+  return nil;
+}
+
+- (NSTimeInterval)getClipboardContentAge {
+  return -[self.lastPasteboardChangeDate timeIntervalSinceNow];
+}
+
+- (void)suppressClipboardContent {
+  // User cleared the user data. The pasteboard entry must be removed from the
+  // omnibox list. Force entry expiration by setting copy date to 1970.
+  self.lastPasteboardChangeDate =
+      [[NSDate alloc] initWithTimeIntervalSince1970:0];
+  [self saveToUserDefaults];
+}
+
+- (void)updateIfNeeded {
+  if (![self hasPasteboardChanged])
+    return;
+
+  [self.delegate onClipboardChanged];
+
+  self.lastPasteboardChangeDate = [NSDate date];
+  self.lastPasteboardChangeCount = [UIPasteboard generalPasteboard].changeCount;
+  NSString* pasteboardString = [UIPasteboard generalPasteboard].string;

[sdefresne, 2017/04/03 09:27:17]

This is duplicated with line 138-142. Can you extract to a separate method.

[lody, 2017/04/04 13:42:18]

Done.

+  if (!pasteboardString) {

[sdefresne, 2017/04/03 09:27:17]

ditto

[lody, 2017/04/04 13:42:19]

Done.

+    pasteboardString = @"";
+  }
+  NSData* MD5 = WeakMD5FromNSString(pasteboardString);
+  self.lastPasteboardEntryMD5 = [MD5 init];

[sdefresne, 2017/04/03 09:27:17]

You're sending -init to an already initialised Objective-C object. I think this
is incorrect. Did you want to use -copy?

[lody, 2017/04/04 13:42:19]

Done.

+  [self saveToUserDefaults];
+}
+
+- (NSURL*)URLFromPasteboard {
+  NSString* clipboardString = [UIPasteboard generalPasteboard].string;
+  if (!clipboardString) {

[sdefresne, 2017/04/03 09:27:17]

Braces for simple blocks are optional (simple blocks are defined as both
condition and block being single line). However, they should be consistent in a
given file. Here you use braces while you don't at line 174. Please make all
consistents.

[lody, 2017/04/04 13:42:19]

Done.

+    return nil;
+  }
+
+  NSURL* url = [NSURL URLWithString:clipboardString];
+  if (url) {

[sdefresne, 2017/04/03 09:27:17]

You can use an early return here to avoid deep nesting:

  if (!url)
    return nil;

  for (NSString* scheme in self.authorizedSchemes) {
    if ([url.scheme isEqualToString:scheme]) {
      return url;
    }
  }

BTW, it looks like self.authorizedScheme is only used to check whether
url.scheme is a member of the collection. Maybe instead change this to use
-indexOfObject:, or even better, change the collection from a NSArray* to a
NSSet*, and use -containsObject:.

  @property (nonatomic, readonly) NSSet<NSString*>* authorizedSchemes;

  if (![self.authorizedSchemes containsObject:url.scheme])
    return nil;
  return url;

[lody, 2017/04/04 13:42:19]

Done.

+    for (NSString* scheme in self.authorizedSchemes) {
+      if ([url.scheme isEqualToString:scheme]) {
+        return url;
+      }
+    }
+  }
+
+  return nil;
+}
+
+- (void)loadFromUserDefaults {
+  self.lastPasteboardChangeCount =
+      [self.sharedUserDefaults integerForKey:kPasteboardChangeCountKey];
+  self.lastPasteboardChangeDate =
+      [self.sharedUserDefaults objectForKey:kPasteboardChangeDateKey];

[sdefresne, 2017/04/03 09:27:17]

You can use base::ObjCCastStrict here and remove the DCHECK below:

  self.lastPasteboardChangeDate =
      base::ObjCCastStrict<NSDate>(
          [self.sharedUserDefaults objectForKey:kPasteboardChangeDateKey]);

[lody, 2017/04/04 13:42:19]

Done.

+  self.lastPasteboardEntryMD5 =
+      [self.sharedUserDefaults objectForKey:kPasteboardEntryMD5Key];
+
+  DCHECK(!self.lastPasteboardChangeDate ||
+         [self.lastPasteboardChangeDate isKindOfClass:[NSDate class]]);
+}
+
+- (void)saveToUserDefaults {
+  [self.sharedUserDefaults setInteger:self.lastPasteboardChangeCount
+                               forKey:kPasteboardChangeCountKey];
+  [self.sharedUserDefaults setObject:self.lastPasteboardChangeDate
+                              forKey:kPasteboardChangeDateKey];
+  [self.sharedUserDefaults setObject:self.lastPasteboardEntryMD5
+                              forKey:kPasteboardEntryMD5Key];
+}
+
+- (NSTimeInterval)uptime {
+  return base::SysInfo::Uptime().InSecondsF();
+}
+
+@end