Rewrite implementation of ClipboardRecentContent in Objective C.

components/open_from_clipboard/clipboard_recent_content_ios_unittest.mm

 // Copyright 2015 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 "components/open_from_clipboard/clipboard_recent_content_ios.h"
 
 #import <CoreGraphics/CoreGraphics.h>
 #import <UIKit/UIKit.h>
 
 #include <memory>
 
+#import "components/open_from_clipboard/clipboard_recent_content_ios_impl.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
 namespace {
 
 UIImage* TestUIImage() {
   CGRect frame = CGRectMake(0, 0, 1.0, 1.0);
   UIGraphicsBeginImageContext(frame.size);
 
   CGContextRef context = UIGraphicsGetCurrentContext();
@@ skipping 16 matching lines @@
       forPasteboardType:@"public.plain-text"];
 }
 const char kUnrecognizedURL[] = "ftp://foo/";
 const char kRecognizedURL[] = "http://bar/";
 const char kRecognizedURL2[] = "http://bar/2";
 const char kAppSpecificURL[] = "test://qux/";
 const char kAppSpecificScheme[] = "test";
 NSTimeInterval kSevenHours = 60 * 60 * 7;
 }  // namespace
 
+@interface ClipboardRecentContentIOSImplWithFakeUptime
+    : ClipboardRecentContentIOSImpl
+@property(nonatomic) NSTimeInterval fakeUptime;
+
+- (instancetype)initWithDelegate:
+                    (id<ClipboardRecentContentMetricsDelegate>)delegate
+               authorizedSchemes:(NSArray*)authorizedSchemes
+                    userDefaults:(NSUserDefaults*)groupUserDefaults
+                          uptime:(NSTimeInterval)uptime;
+
+@end
+
+@implementation ClipboardRecentContentIOSImplWithFakeUptime
+
+@synthesize fakeUptime = _fakeUptime;
+
+- (instancetype)initWithDelegate:
+                    (id<ClipboardRecentContentMetricsDelegate>)delegate
+               authorizedSchemes:(NSArray*)authorizedSchemes
+                    userDefaults:(NSUserDefaults*)groupUserDefaults
+                          uptime:(NSTimeInterval)uptime {
+  self = [super initWithDelegate:delegate
+               authorizedSchemes:authorizedSchemes
+                    userDefaults:groupUserDefaults];
+  if (self) {
+    _fakeUptime = uptime;
+  }
+  return self;
+}
+
+- (NSTimeInterval)uptime {

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

I would change this to a public property instead (see comment line 121).

+  return self.fakeUptime;
+}
+
+@end
+
 class ClipboardRecentContentIOSWithFakeUptime
     : public ClipboardRecentContentIOS {
  public:
   ClipboardRecentContentIOSWithFakeUptime(const std::string& application_scheme,
                                           NSUserDefaults* group_user_defaults)
       : ClipboardRecentContentIOS(application_scheme, group_user_defaults) {}
-  // Sets the uptime.
-  void SetUptime(base::TimeDelta uptime) { uptime_ = uptime; }
-
- protected:
-  base::TimeDelta Uptime() const override { return uptime_; }
-
- private:
-  base::TimeDelta uptime_;
 };
 
 class ClipboardRecentContentIOSTest : public ::testing::Test {
  protected:
   ClipboardRecentContentIOSTest() {
     // By default, set that the device booted 10 days ago.
     ResetClipboardRecentContent(kAppSpecificScheme,
                                 base::TimeDelta::FromDays(10));
   }
 
   void SimulateDeviceRestart() {
     ResetClipboardRecentContent(kAppSpecificScheme,
                                 base::TimeDelta::FromSeconds(0));
   }
 
   void ResetClipboardRecentContent(const std::string& application_scheme,
                                    base::TimeDelta time_delta) {
     clipboard_content_.reset(new ClipboardRecentContentIOSWithFakeUptime(
         application_scheme, [NSUserDefaults standardUserDefaults]));
-    clipboard_content_->SetUptime(time_delta);
+
+    clipboard_content_->implementation_.reset(

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

This is exactly how you should not write test. Instead provide a constructor
that takes a ClipboardRecentContentIOSImpl* and use that constructor when
creating your ClipboardRecentContentIOS.

[sdefresne, 2017/04/03 09:37:32]

Sorry, this comment tone is too negative. I should have remove the first
sentence. Please read this as:

I think injecting the ClipboardRecentContentIOSImpl via the constructor would
make the test easier to write as it would not require accessing private member
fields of ClipboardRecentContentIOS.

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

Done.

+        [[ClipboardRecentContentIOSImplWithFakeUptime alloc]
+             initWithDelegate:nil
+            authorizedSchemes:clipboard_content_->GetAuthorizedSchemeList(

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

Just inject a know list/set of schemes here, no need to access this private
method.

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

Done.

+                                  application_scheme)
+                 userDefaults:[NSUserDefaults standardUserDefaults]
+                       uptime:time_delta.InSecondsF()]);
   }
 
-  void SetStoredPasteboardChangeDate(NSDate* changeDate) {
-    clipboard_content_->last_pasteboard_change_date_.reset([changeDate copy]);
+  void SetStoredPasteboardChangeDate(NSDate* change_date) {
+    clipboard_content_->SetLastPasteboardChangeDate([change_date copy]);

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

Can't you simulate this by instead manipulating the fake uptime? \

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

I don't think so? There's two things, one is the uptime (if the uptime is
shorter than last change date md5 is used to check how many changes have occured
rather than change count) and the changedate (if the changedate is more than 3
hours then nothing is returned). I can't see how to use only uptime to simulate
this.

     clipboard_content_->SaveToUserDefaults();
   }
 
-  void SetStoredPasteboardChangeCount(NSInteger newChangeCount) {
-    clipboard_content_->last_pasteboard_change_count_ = newChangeCount;
-    clipboard_content_->SaveToUserDefaults();
-  }
-
  protected:
   std::unique_ptr<ClipboardRecentContentIOSWithFakeUptime> clipboard_content_;
 };
 
 TEST_F(ClipboardRecentContentIOSTest, SchemeFiltering) {
   GURL gurl;
 
   // Test unrecognized URL.
   SetPasteboardContent(kUnrecognizedURL);
   EXPECT_FALSE(clipboard_content_->GetRecentURLFromClipboard(&gurl));
@@ skipping 53 matching lines @@
   // Check that the pasteboard content is suppressed.
   EXPECT_FALSE(clipboard_content_->GetRecentURLFromClipboard(&gurl));
 
   // Create a new clipboard content to test persistence.
   ResetClipboardRecentContent(kAppSpecificScheme,
                               base::TimeDelta::FromDays(10));
 
   // Check that the pasteboard content is still suppressed.
   EXPECT_FALSE(clipboard_content_->GetRecentURLFromClipboard(&gurl));
 
-  // Check that the even if the device is restarted, pasteboard content is
+  // Check that even if the device is restarted, pasteboard content is
   // still suppressed.
   SimulateDeviceRestart();
   EXPECT_FALSE(clipboard_content_->GetRecentURLFromClipboard(&gurl));
 
   // Check that if the pasteboard changes, the new content is not
   // supressed anymore.
   SetPasteboardContent(kRecognizedURL2);
   EXPECT_TRUE(clipboard_content_->GetRecentURLFromClipboard(&gurl));
 }
 
@@ skipping 12 matching lines @@
   SetPasteboardImage(TestUIImage());
 
   // Pasteboard should appear empty.
   EXPECT_FALSE(clipboard_content_->GetRecentURLFromClipboard(&gurl));
 
   // Tests that if URL is added again, pasteboard provides it normally.
   SetPasteboardContent(kRecognizedURL);
   EXPECT_TRUE(clipboard_content_->GetRecentURLFromClipboard(&gurl));
   EXPECT_STREQ(kRecognizedURL, gurl.spec().c_str());
 }