Stop using UIPasteboardChangedNotification.

iOS10 does not trigger UIPasteboardChangedNotification notifications
on pasteboard change ( filed bug with Apple: rdr://27477065 ).
This CL is a workaround where we check if the pasteboard changed
when we attempt to display its content.

BUG=628592

Review-Url: https://codereview.chromium.org/2230983002
Cr-Commit-Position: refs/heads/master@{#412788}

components/open_from_clipboard/clipboard_recent_content.h

@@ -31,10 +31,7 @@ class ClipboardRecentContent {
   // Returns true if the clipboard contains a recent URL that has not been
   // supressed, and copies it in |url|. Otherwise, returns false. |url| must not
   // be null.
-  virtual bool GetRecentURLFromClipboard(GURL* url) const = 0;
-
-  // Reports that the URL contained in the pasteboard was displayed.
-  virtual void RecentURLDisplayed() = 0;
+  virtual bool GetRecentURLFromClipboard(GURL* url) = 0;
 
   // Returns how old the content of the clipboard is.
   virtual base::TimeDelta GetClipboardContentAge() const = 0;

components/open_from_clipboard/clipboard_recent_content_ios.h

@@ -13,7 +13,7 @@
 
 @class NSDate;
 @class NSUserDefaults;
-@class PasteboardNotificationListenerBridge;
+@class ApplicationDidBecomeActiveNotificationListenerBridge;
 
 class ClipboardRecentContentIOSTest;
 
@@ -30,39 +30,29 @@ class ClipboardRecentContentIOS : public ClipboardRecentContent {
                                      NSUserDefaults* group_user_defaults);
   ~ClipboardRecentContentIOS() override;
 
-  // Notifies that the content of the pasteboard may have changed.
-  void PasteboardChanged();
+  // 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();
 
-  // Checks if pasteboard changed since last time a pasteboard change was
-  // registered.
-  bool HasPasteboardChanged(base::TimeDelta uptime);
-
-  // Gets the current URL in the clipboard. If the cache is out of date, updates
-  // it.
-  bool GetCurrentURLFromClipboard(GURL* url);
+  // Returns whether the pasteboard changed since the last time a pasteboard
+  // change was detected.
+  bool HasPasteboardChanged() const;
 
   // Loads information from the user defaults about the latest pasteboard entry.
   void LoadFromUserDefaults();
 
   // ClipboardRecentContent implementation.
-  bool GetRecentURLFromClipboard(GURL* url) const override;
-
+  bool GetRecentURLFromClipboard(GURL* url) override;
   base::TimeDelta GetClipboardContentAge() const override;
   void SuppressClipboardContent() override;
-  void RecentURLDisplayed() override;
+
+ protected:
+  // Returns the uptime. Override in tests to return custom value.
+  virtual base::TimeDelta Uptime() const;
 
  private:
   friend class ClipboardRecentContentIOSTest;
 
-  // Helper constructor for testing. |uptime| is how long ago the device has
-  // started, while |application_scheme| has the same meaning as the public
-  // constructor.
-  ClipboardRecentContentIOS(const std::string& application_scheme,
-                            base::TimeDelta uptime);
-
-  // Initializes the object. |uptime| is how long ago the device has started.
-  void Init(base::TimeDelta uptime);
-
   // Saves information to the user defaults about the latest pasteboard entry.
   void SaveToUserDefaults();
 
@@ -75,14 +65,10 @@ class ClipboardRecentContentIOS : public ClipboardRecentContent {
   NSInteger last_pasteboard_change_count_;
   // Estimation of the date when the pasteboard changed.
   base::scoped_nsobject<NSDate> last_pasteboard_change_date_;
-  // Estimation of the copy date of the last displayed URL.
-  base::scoped_nsobject<NSDate> last_displayed_pasteboard_entry_;
   // MD5 hash of the last registered pasteboard entry.
   base::scoped_nsobject<NSData> last_pasteboard_entry_md5_;
-  // Cache of the GURL contained in the pasteboard (if any).
-  GURL url_from_pasteboard_cache_;
-  // Bridge to receive notification when the pasteboard changes.
-  base::scoped_nsobject<PasteboardNotificationListenerBridge>
+  // Bridge to receive notifications when the application becomes active.
+  base::scoped_nsobject<ApplicationDidBecomeActiveNotificationListenerBridge>
       notification_bridge_;
   // The user defaults from the app group used to optimize the pasteboard change
   // detection.

components/open_from_clipboard/clipboard_recent_content_ios.mm

@@ -18,11 +18,12 @@
 #include "url/gurl.h"
 #include "url/url_constants.h"
 
-// Bridge that forwards pasteboard change notifications to its delegate.
-@interface PasteboardNotificationListenerBridge : NSObject
+// Bridge that forwards UIApplicationDidBecomeActiveNotification notifications
+// to its delegate.
+@interface ApplicationDidBecomeActiveNotificationListenerBridge : NSObject
 
-// Initialize the PasteboardNotificationListenerBridge with |delegate| which
-// must not be null.
+// Initialize the ApplicationDidBecomeActiveNotificationListenerBridge with
+// |delegate| which must not be null.
 - (instancetype)initWithDelegate:(ClipboardRecentContentIOS*)delegate
     NS_DESIGNATED_INITIALIZER;
 
@@ -30,7 +31,7 @@
 
 @end
 
-@implementation PasteboardNotificationListenerBridge {
+@implementation ApplicationDidBecomeActiveNotificationListenerBridge {
   ClipboardRecentContentIOS* _delegate;
 }
 
@@ -44,11 +45,6 @@
   self = [super init];
   if (self) {
     _delegate = delegate;
-    [[NSNotificationCenter defaultCenter]
-        addObserver:self
-           selector:@selector(pasteboardChangedNotification:)
-               name:UIPasteboardChangedNotification
-             object:[UIPasteboard generalPasteboard]];
     [[NSNotificationCenter defaultCenter]
         addObserver:self
            selector:@selector(didBecomeActive:)
@@ -63,18 +59,10 @@
   [super dealloc];
 }
 
-- (void)pasteboardChangedNotification:(NSNotification*)notification {
-  if (_delegate) {
-    _delegate->PasteboardChanged();
-  }
-}
-
 - (void)didBecomeActive:(NSNotification*)notification {
   if (_delegate) {
     _delegate->LoadFromUserDefaults();
-    if (_delegate->HasPasteboardChanged(base::SysInfo::Uptime())) {
-      _delegate->PasteboardChanged();
-    }
+    _delegate->UpdateIfNeeded();
   }
 }
 
@@ -95,9 +83,6 @@ 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";
-// Key used to store the date of the latest pasteboard entry displayed in the
-// omnibox. This is used to report metrics on pasteboard change.
-NSString* kLastDisplayedPasteboardEntryKey = @"LastDisplayedPasteboardEntry";
 base::TimeDelta kMaximumAgeOfClipboard = base::TimeDelta::FromHours(3);
 // Schemes accepted by the ClipboardRecentContentIOS.
 const char* kAuthorizedSchemes[] = {
@@ -121,14 +106,16 @@ NSData* WeakMD5FromNSString(NSString* string) {
 
 }  // namespace
 
-bool ClipboardRecentContentIOS::GetRecentURLFromClipboard(GURL* url) const {
+bool ClipboardRecentContentIOS::GetRecentURLFromClipboard(GURL* url) {
   DCHECK(url);
+  UpdateIfNeeded();
   if (GetClipboardContentAge() > kMaximumAgeOfClipboard) {
     return false;
   }
 
-  if (url_from_pasteboard_cache_.is_valid()) {
-    *url = url_from_pasteboard_cache_;
+  GURL url_from_pasteboard = URLFromPasteboard();
+  if (url_from_pasteboard.is_valid()) {
+    *url = url_from_pasteboard;
     return true;
   }
   return false;
@@ -147,12 +134,13 @@ void ClipboardRecentContentIOS::SuppressClipboardContent() {
   SaveToUserDefaults();
 }
 
-void ClipboardRecentContentIOS::PasteboardChanged() {
-  url_from_pasteboard_cache_ = URLFromPasteboard();
-  if (!url_from_pasteboard_cache_.is_empty()) {
-    base::RecordAction(
-        base::UserMetricsAction("MobileOmniboxClipboardChanged"));
-  }
+void ClipboardRecentContentIOS::UpdateIfNeeded() {
+  if (!HasPasteboardChanged())
+    return;
+
+  base::RecordAction(base::UserMetricsAction("MobileOmniboxClipboardChanged"));
+
+  GURL url_from_pasteboard = URLFromPasteboard();
   last_pasteboard_change_date_.reset([[NSDate date] retain]);
   last_pasteboard_change_count_ = [UIPasteboard generalPasteboard].changeCount;
   NSString* pasteboard_string = [[UIPasteboard generalPasteboard] string];
@@ -169,18 +157,19 @@ ClipboardRecentContentIOS::ClipboardRecentContentIOS(
     NSUserDefaults* group_user_defaults)
     : application_scheme_(application_scheme),
       shared_user_defaults_([group_user_defaults retain]) {
-  Init(base::SysInfo::Uptime());
-}
+  last_pasteboard_change_count_ = NSIntegerMax;
+  LoadFromUserDefaults();
 
-ClipboardRecentContentIOS::ClipboardRecentContentIOS(
-    const std::string& application_scheme,
-    base::TimeDelta uptime)
-    : application_scheme_(application_scheme),
-      shared_user_defaults_([[NSUserDefaults standardUserDefaults] retain]) {
-  Init(uptime);
+  UpdateIfNeeded();
+
+  // Makes sure |last_pasteboard_change_count_| was properly initialized.
+  DCHECK_NE(last_pasteboard_change_count_, NSIntegerMax);
+  notification_bridge_.reset(
+      [[ApplicationDidBecomeActiveNotificationListenerBridge alloc]
+          initWithDelegate:this]);
 }
 
-bool ClipboardRecentContentIOS::HasPasteboardChanged(base::TimeDelta uptime) {
+bool ClipboardRecentContentIOS::HasPasteboardChanged() const {
   // 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
@@ -192,7 +181,7 @@ bool ClipboardRecentContentIOS::HasPasteboardChanged(base::TimeDelta uptime) {
   NSInteger change_count = [UIPasteboard generalPasteboard].changeCount;
   bool change_count_changed = change_count != last_pasteboard_change_count_;
 
-  bool not_rebooted = uptime > GetClipboardContentAge();
+  bool not_rebooted = Uptime() > GetClipboardContentAge();
   if (not_rebooted)
     return change_count_changed;
 
@@ -206,27 +195,6 @@ bool ClipboardRecentContentIOS::HasPasteboardChanged(base::TimeDelta uptime) {
   return md5_changed;
 }
 
-bool ClipboardRecentContentIOS::GetCurrentURLFromClipboard(GURL* url) {
-  if (HasPasteboardChanged(base::SysInfo::Uptime())) {
-    PasteboardChanged();
-  }
-  return GetRecentURLFromClipboard(url);
-}
-
-void ClipboardRecentContentIOS::Init(base::TimeDelta uptime) {
-  last_pasteboard_change_count_ = NSIntegerMax;
-  url_from_pasteboard_cache_ = URLFromPasteboard();
-  LoadFromUserDefaults();
-
-  if (HasPasteboardChanged(uptime))
-    PasteboardChanged();
-
-  // Makes sure |last_pasteboard_change_count_| was properly initialized.
-  DCHECK_NE(last_pasteboard_change_count_, NSIntegerMax);
-  notification_bridge_.reset(
-      [[PasteboardNotificationListenerBridge alloc] initWithDelegate:this]);
-}
-
 ClipboardRecentContentIOS::~ClipboardRecentContentIOS() {
   [notification_bridge_ disconnect];
 }
@@ -252,16 +220,6 @@ GURL ClipboardRecentContentIOS::URLFromPasteboard() {
   return GURL::EmptyGURL();
 }
 
-void ClipboardRecentContentIOS::RecentURLDisplayed() {
-  if ([last_pasteboard_change_date_
-          isEqualToDate:last_displayed_pasteboard_entry_.get()]) {
-    return;
-  }
-  base::RecordAction(base::UserMetricsAction("MobileOmniboxClipboardChanged"));
-  last_pasteboard_change_date_ = last_displayed_pasteboard_entry_;
-  SaveToUserDefaults();
-}
-
 void ClipboardRecentContentIOS::LoadFromUserDefaults() {
   last_pasteboard_change_count_ =
       [shared_user_defaults_ integerForKey:kPasteboardChangeCountKey];
@@ -269,8 +227,6 @@ void ClipboardRecentContentIOS::LoadFromUserDefaults() {
       [[shared_user_defaults_ objectForKey:kPasteboardChangeDateKey] retain]);
   last_pasteboard_entry_md5_.reset(
       [[shared_user_defaults_ objectForKey:kPasteboardEntryMD5Key] retain]);
-  last_displayed_pasteboard_entry_.reset([[shared_user_defaults_
-      objectForKey:kLastDisplayedPasteboardEntryKey] retain]);
 
   DCHECK(!last_pasteboard_change_date_ ||
          [last_pasteboard_change_date_ isKindOfClass:[NSDate class]]);
@@ -283,6 +239,8 @@ void ClipboardRecentContentIOS::SaveToUserDefaults() {
                             forKey:kPasteboardChangeDateKey];
   [shared_user_defaults_ setObject:last_pasteboard_entry_md5_
                             forKey:kPasteboardEntryMD5Key];
-  [shared_user_defaults_ setObject:last_displayed_pasteboard_entry_
-                            forKey:kLastDisplayedPasteboardEntryKey];
+}
+
+base::TimeDelta ClipboardRecentContentIOS::Uptime() const {
+  return base::SysInfo::Uptime();
 }

components/open_from_clipboard/clipboard_recent_content_ios_unittest.mm

@@ -39,11 +39,28 @@ void SetPasteboardContent(const char* data) {
 }
 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
 
+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() {
@@ -59,8 +76,9 @@ class ClipboardRecentContentIOSTest : public ::testing::Test {
 
   void ResetClipboardRecentContent(const std::string& application_scheme,
                                    base::TimeDelta time_delta) {
-    clipboard_content_.reset(
-        new ClipboardRecentContentIOS(application_scheme, time_delta));
+    clipboard_content_.reset(new ClipboardRecentContentIOSWithFakeUptime(
+        application_scheme, [NSUserDefaults standardUserDefaults]));
+    clipboard_content_->SetUptime(time_delta);
   }
 
   void SetStoredPasteboardChangeDate(NSDate* changeDate) {
@@ -74,7 +92,7 @@ class ClipboardRecentContentIOSTest : public ::testing::Test {
   }
 
  protected:
-  std::unique_ptr<ClipboardRecentContentIOS> clipboard_content_;
+  std::unique_ptr<ClipboardRecentContentIOSWithFakeUptime> clipboard_content_;
 };
 
 TEST_F(ClipboardRecentContentIOSTest, SchemeFiltering) {
@@ -153,7 +171,7 @@ TEST_F(ClipboardRecentContentIOSTest, SupressedPasteboard) {
 
   // Check that if the pasteboard changes, the new content is not
   // supressed anymore.
-  SetPasteboardContent(kRecognizedURL);
+  SetPasteboardContent(kRecognizedURL2);
   EXPECT_TRUE(clipboard_content_->GetRecentURLFromClipboard(&gurl));
 }
 

components/open_from_clipboard/fake_clipboard_recent_content.cc

@@ -9,7 +9,7 @@ FakeClipboardRecentContent::FakeClipboardRecentContent()
 
 FakeClipboardRecentContent::~FakeClipboardRecentContent() {}
 
-bool FakeClipboardRecentContent::GetRecentURLFromClipboard(GURL* url) const {
+bool FakeClipboardRecentContent::GetRecentURLFromClipboard(GURL* url) {
   if (suppress_content_)
     return false;
 
@@ -28,8 +28,6 @@ void FakeClipboardRecentContent::SuppressClipboardContent() {
   suppress_content_ = true;
 }
 
-void FakeClipboardRecentContent::RecentURLDisplayed() {}
-
 void FakeClipboardRecentContent::SetClipboardContent(
     const GURL& url,
     base::TimeDelta content_age) {

components/open_from_clipboard/fake_clipboard_recent_content.h

@@ -18,10 +18,9 @@ class FakeClipboardRecentContent : public ClipboardRecentContent {
   ~FakeClipboardRecentContent() override;
 
   // ClipboardRecentContent implementation.
-  bool GetRecentURLFromClipboard(GURL* url) const override;
+  bool GetRecentURLFromClipboard(GURL* url) override;
   base::TimeDelta GetClipboardContentAge() const override;
   void SuppressClipboardContent() override;
-  void RecentURLDisplayed() override;
 
   // Sets the URL and clipboard content age.
   void SetClipboardContent(const GURL& url, base::TimeDelta content_age);