Revert "Delay SharedClipboardShareActivity init if DeviceInfoProvider not loaded"

This reverts commit 0b7170d7.

Reason for revert: Speculative: suspected of causing failures of
InfoCollection_basic
InfoCollection_direct_composition
https://ci.chromium.org/p/chromium/builders/ci/Win10%20x64%20Debug%20%28NVIDIA%29/829?blamelist=1#blamelist-tab

Original change's description:
> Delay SharedClipboardShareActivity init if DeviceInfoProvider not loaded
> 
> - Add a JNI method to callback when both DeviceInfoTracker and
>   LocalDeviceInfoProvider are ready
> - Also removed redundant setVibration in SharingNotificationUtil
> 
> Bug: 996318
> Change-Id: Id4df6b407ac2fb1c1622071ffb7ee8bb3a747b41
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1787379
> Commit-Queue: Alex Chau <alexchau@chromium.org>
> Reviewed-by: Marc Treib <treib@chromium.org>
> Reviewed-by: Richard Knoll <knollr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#694208}

TBR=treib@chromium.org,alexchau@chromium.org,knollr@chromium.org

Change-Id: I3837352ddf7345751b679f4065b0c00de28d5d43
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 996318
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1789234Reviewed-by: Olga Sharonova <olka@chromium.org>
Commit-Queue: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694223}

chrome/android/java/src/org/chromium/chrome/browser/sharing/SharingNotificationUtil.java

@@ -94,6 +94,7 @@ public final class SharingNotificationUtil {
                         .setColor(ApiCompatibilityUtils.getColor(context.getResources(), color))
                         .setGroup(group)
                         .setPriorityBeforeO(NotificationCompat.PRIORITY_HIGH)
+                        .setVibrate(new long[0])
                         .setSmallIcon(smallIconId)
                         .setAutoCancel(true)
                         .setDefaults(Notification.DEFAULT_ALL);
@@ -150,6 +151,7 @@ public final class SharingNotificationUtil {
                         .setColor(ApiCompatibilityUtils.getColor(
                                 context.getResources(), R.color.default_icon_color_blue))
                         .setPriorityBeforeO(NotificationCompat.PRIORITY_HIGH)
+                        .setVibrate(new long[0])
                         .setSmallIcon(R.drawable.ic_devices_16dp)
                         .setProgress(/*max=*/0, /*percentage=*/0, true)
                         .setOngoing(true)
@@ -195,6 +197,7 @@ public final class SharingNotificationUtil {
                         .setGroup(group)
                         .setColor(ApiCompatibilityUtils.getColor(resources, R.color.google_red_600))
                         .setPriorityBeforeO(NotificationCompat.PRIORITY_HIGH)
+                        .setVibrate(new long[0])
                         .setSmallIcon(R.drawable.ic_error_outline_red_24dp)
                         .setContentText(contentText)
                         .setDefaults(Notification.DEFAULT_ALL)

chrome/android/java/src/org/chromium/chrome/browser/sharing/SharingServiceProxy.java

@@ -95,7 +95,7 @@ public class SharingServiceProxy {
 
     /**
      * Returns a list of devices for with given capabilities.
-     * @param capabilities A bitmask of capabilities created from.
+     * @param capability A bitmask of capabilities created from.
      *         org.chromium.chrome.browser.sharing.SharingDeviceCapability enum values.
      */
     public ArrayList<DeviceInfo> getDeviceCandidates(int capabilities) {
@@ -109,16 +109,6 @@ public class SharingServiceProxy {
         return deviceInfo;
     }
 
-    public void addDeviceCandidatesInitializedObserver(Runnable callback) {
-        if (sNativeSharingServiceProxyAndroid == 0) {
-            callback.run();
-            return;
-        }
-
-        Natives jni = SharingServiceProxyJni.get();
-        jni.addDeviceCandidatesInitializedObserver(sNativeSharingServiceProxyAndroid, callback);
-    }
-
     @NativeMethods
     interface Natives {
         void initSharingService(Profile profile);
@@ -126,7 +116,5 @@ public class SharingServiceProxy {
                 String text, Callback<Integer> callback);
         void getDeviceCandidates(long nativeSharingServiceProxyAndroid,
                 ArrayList<DeviceInfo> deviceInfo, int capabilities);
-        void addDeviceCandidatesInitializedObserver(
-                long nativeSharingServiceProxyAndroid, Runnable runnable);
     }
 }

chrome/android/java/src/org/chromium/chrome/browser/sharing/shared_clipboard/SharedClipboardShareActivity.java

@@ -23,7 +23,6 @@ import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.init.AsyncInitializationActivity;
 import org.chromium.chrome.browser.sharing.SharingAdapter;
 import org.chromium.chrome.browser.sharing.SharingDeviceCapability;
-import org.chromium.chrome.browser.sharing.SharingServiceProxy;
 import org.chromium.chrome.browser.sharing.SharingServiceProxy.DeviceInfo;
 
 /**
@@ -83,12 +82,6 @@ public class SharedClipboardShareActivity
         onInitialLayoutInflationComplete();
     }
 
-    @Override
-    public void startNativeInitialization() {
-        SharingServiceProxy.getInstance().addDeviceCandidatesInitializedObserver(
-                this::finishNativeInitialization);
-    }
-
     @Override
     public void finishNativeInitialization() {
         super.finishNativeInitialization();

chrome/browser/sharing/sharing_service.cc

@@ -31,6 +31,7 @@
 #include "components/gcm_driver/gcm_driver.h"
 #include "components/sync/driver/sync_service.h"
 #include "components/sync_device_info/device_info.h"
+#include "components/sync_device_info/device_info_tracker.h"
 #include "components/sync_device_info/local_device_info_provider.h"
 #include "content/public/browser/browser_task_traits.h"
 
@@ -54,8 +55,7 @@ SharingService::SharingService(
       local_device_info_provider_(local_device_info_provider),
       sync_service_(sync_service),
       backoff_entry_(&kRetryBackoffPolicy),
-      state_(State::DISABLED),
-      is_observing_device_info_tracker_(false) {
+      state_(State::DISABLED) {
   // Remove old encryption info with empty authrozed_entity to avoid DCHECK.
   // See http://crbug/987591
   if (gcm_driver) {
@@ -131,14 +131,11 @@ std::unique_ptr<syncer::DeviceInfo> SharingService::GetDeviceByGuid(
 std::vector<std::unique_ptr<syncer::DeviceInfo>>
 SharingService::GetDeviceCandidates(int required_capabilities) const {
   std::vector<std::unique_ptr<syncer::DeviceInfo>> device_candidates;
-  std::vector<std::unique_ptr<syncer::DeviceInfo>> all_devices =
-      device_info_tracker_->GetAllDeviceInfo();
-  const syncer::DeviceInfo* local_device_info =
-      local_device_info_provider_->GetLocalDeviceInfo();
-
-  if (IsSyncDisabled() || all_devices.empty() || !local_device_info)
+  if (IsSyncDisabled())
     return device_candidates;
 
+  std::vector<std::unique_ptr<syncer::DeviceInfo>> all_devices =
+      device_info_tracker_->GetAllDeviceInfo();
   std::map<std::string, SharingSyncPreference::Device> synced_devices =
       sync_prefs_->GetSyncedDevices();
 
@@ -152,6 +149,8 @@ SharingService::GetDeviceCandidates(int required_capabilities) const {
             });
 
   std::unordered_set<std::string> device_names;
+  const syncer::DeviceInfo* local_device_info =
+      local_device_info_provider_->GetLocalDeviceInfo();
   for (auto& device : all_devices) {
     // If the current device is considered expired for our purposes, stop here
     // since the next devices in the vector are at least as expired than this
@@ -159,8 +158,10 @@ SharingService::GetDeviceCandidates(int required_capabilities) const {
     if (device->last_updated_timestamp() < min_updated_time)
       break;
 
-    if (local_device_info->client_name() == device->client_name())
+    if (local_device_info &&
+        (local_device_info->client_name() == device->client_name())) {
       continue;
+    }
 
     auto synced_device = synced_devices.find(device->guid());
     if (synced_device == synced_devices.end())
@@ -182,36 +183,6 @@ SharingService::GetDeviceCandidates(int required_capabilities) const {
   return device_candidates;
 }
 
-void SharingService::AddDeviceCandidatesInitializedObserver(
-    base::OnceClosure callback) {
-  if (IsSyncDisabled()) {
-    std::move(callback).Run();
-    return;
-  }
-
-  bool is_device_info_tracker_ready = device_info_tracker_->IsSyncing();
-  bool is_local_device_info_ready =
-      local_device_info_provider_->GetLocalDeviceInfo();
-  if (is_device_info_tracker_ready && is_local_device_info_ready) {
-    std::move(callback).Run();
-    return;
-  }
-
-  device_candidates_initialized_callbacks_.emplace_back(std::move(callback));
-
-  if (!is_device_info_tracker_ready && !is_observing_device_info_tracker_) {
-    device_info_tracker_->AddObserver(this);
-    is_observing_device_info_tracker_ = true;
-  }
-
-  if (!is_local_device_info_ready && !local_device_info_ready_subscription_) {
-    local_device_info_ready_subscription_ =
-        local_device_info_provider_->RegisterOnInitializedCallback(
-            base::BindRepeating(&SharingService::OnDeviceInfoChange,
-                                weak_ptr_factory_.GetWeakPtr()));
-  }
-}
-
 void SharingService::SendMessageToDevice(
     const std::string& device_guid,
     base::TimeDelta time_to_live,
@@ -290,22 +261,6 @@ void SharingService::InvokeSendMessageCallback(
   LogSendSharingMessageResult(result);
 }
 
-void SharingService::OnDeviceInfoChange() {
-  if (!device_info_tracker_->IsSyncing() ||
-      !local_device_info_provider_->GetLocalDeviceInfo()) {
-    return;
-  }
-
-  device_info_tracker_->RemoveObserver(this);
-  is_observing_device_info_tracker_ = false;
-  local_device_info_ready_subscription_.reset();
-
-  for (base::OnceClosure& callback : device_candidates_initialized_callbacks_) {
-    std::move(callback).Run();
-  }
-  device_candidates_initialized_callbacks_.clear();
-}
-
 void SharingService::RegisterHandler(
     chrome_browser_sharing::SharingMessage::PayloadCase payload_type,
     SharingMessageHandler* handler) {}

chrome/browser/sharing/sharing_service.h

@@ -22,7 +22,6 @@
 #include "components/gcm_driver/web_push_common.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/sync/driver/sync_service_observer.h"
-#include "components/sync_device_info/device_info_tracker.h"
 #include "net/base/backoff_entry.h"
 
 #if defined(OS_ANDROID)
@@ -38,6 +37,7 @@ class GCMDriver;
 
 namespace syncer {
 class DeviceInfo;
+class DeviceInfoTracker;
 class LocalDeviceInfoProvider;
 class SyncService;
 }  // namespace syncer
@@ -53,8 +53,7 @@ enum class SharingDeviceRegistrationResult;
 // sharing messages to other devices.
 class SharingService : public KeyedService,
                        syncer::SyncServiceObserver,
-                       AckMessageHandler::AckMessageObserver,
-                       syncer::DeviceInfoTracker::Observer {
+                       AckMessageHandler::AckMessageObserver {
  public:
   using SendMessageCallback =
       base::OnceCallback<void(SharingSendMessageResult)>;
@@ -93,10 +92,6 @@ class SharingService : public KeyedService,
   virtual std::vector<std::unique_ptr<syncer::DeviceInfo>> GetDeviceCandidates(
       int required_capabilities) const;
 
-  // Register |callback| so it will be invoked after all dependencies of
-  // GetDeviceCandidates are ready.
-  void AddDeviceCandidatesInitializedObserver(base::OnceClosure callback);
-
   // Sends a message to the device specified by GUID.
   // |callback| will be invoked with message_id if synchronous operation
   // succeeded, or base::nullopt if operation failed.
@@ -132,9 +127,6 @@ class SharingService : public KeyedService,
   // AckMessageHandler::AckMessageObserver override.
   void OnAckReceived(const std::string& message_id) override;
 
-  // syncer::DeviceInfoTracker::Observer.
-  void OnDeviceInfoChange() override;
-
   void RegisterDevice();
 
   void UnregisterDevice();
@@ -166,10 +158,6 @@ class SharingService : public KeyedService,
   PingMessageHandler ping_message_handler_;
   net::BackoffEntry backoff_entry_;
   State state_;
-  std::vector<base::OnceClosure> device_candidates_initialized_callbacks_;
-  bool is_observing_device_info_tracker_;
-  std::unique_ptr<syncer::LocalDeviceInfoProvider::Subscription>
-      local_device_info_ready_subscription_;
 
   // Map of random GUID to SendMessageCallback.
   std::map<std::string, SendMessageCallback> send_message_callbacks_;

chrome/browser/sharing/sharing_service_proxy_android.cc

@@ -77,11 +77,3 @@ void SharingServiceProxyAndroid::GetDeviceCandidates(
         device_info->last_updated_timestamp().ToJavaTime());
   }
 }
-
-void SharingServiceProxyAndroid::AddDeviceCandidatesInitializedObserver(
-    JNIEnv* env,
-    const base::android::JavaParamRef<jobject>& j_runnable) {
-  sharing_service_->AddDeviceCandidatesInitializedObserver(
-      base::BindOnce(base::android::RunRunnableAndroid,
-                     base::android::ScopedJavaGlobalRef<jobject>(j_runnable)));
-}

chrome/browser/sharing/sharing_service_proxy_android.h

@@ -33,10 +33,6 @@ class SharingServiceProxyAndroid {
       const base::android::JavaParamRef<jobject>& j_device_info,
       jint j_capabilities);
 
-  void AddDeviceCandidatesInitializedObserver(
-      JNIEnv* env,
-      const base::android::JavaParamRef<jobject>& j_runnable);
-
  private:
   SharingService* sharing_service_ = nullptr;
   ClickToCallMessageHandler click_to_call_message_handler_;

chrome/browser/sharing/sharing_service_unittest.cc

@@ -12,6 +12,7 @@
 #include "base/optional.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/task_environment.h"
+#include "chrome/browser/sharing/fake_local_device_info_provider.h"
 #include "chrome/browser/sharing/features.h"
 #include "chrome/browser/sharing/proto/sharing_message.pb.h"
 #include "chrome/browser/sharing/sharing_constants.h"
@@ -28,7 +29,6 @@
 #include "components/sync/driver/test_sync_service.h"
 #include "components/sync_device_info/device_info.h"
 #include "components/sync_device_info/fake_device_info_tracker.h"
-#include "components/sync_device_info/local_device_info_provider.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -49,7 +49,6 @@ const char kDeviceName[] = "other_name";
 const char kMessageId[] = "message_id";
 const char kAuthorizedEntity[] = "authorized_entity";
 constexpr base::TimeDelta kTtl = base::TimeDelta::FromSeconds(10);
-const char kSenderGuid[] = "test_sender_guid";
 
 class FakeGCMDriver : public gcm::FakeGCMDriver {
  public:
@@ -165,46 +164,6 @@ class FakeSharingDeviceRegistration : public SharingDeviceRegistration {
   int unregistration_attempts_ = 0;
 };
 
-class FakeLocalDeviceInfoProvider : public syncer::LocalDeviceInfoProvider {
- public:
-  FakeLocalDeviceInfoProvider()
-      : local_device_info_(kSenderGuid,
-                           "name",
-                           "chrome_version",
-                           "user_agent",
-                           sync_pb::SyncEnums_DeviceType_TYPE_LINUX,
-                           "device_id",
-                           base::Time::Now(),
-                           false) {}
-  ~FakeLocalDeviceInfoProvider() override {}
-
-  version_info::Channel GetChannel() const override {
-    return version_info::Channel::UNKNOWN;
-  }
-
-  const syncer::DeviceInfo* GetLocalDeviceInfo() const override {
-    return ready_ ? &local_device_info_ : nullptr;
-  }
-
-  std::unique_ptr<syncer::LocalDeviceInfoProvider::Subscription>
-  RegisterOnInitializedCallback(
-      const base::RepeatingClosure& callback) override {
-    return callback_list_.Add(callback);
-  }
-
-  void SetReady(bool ready) {
-    bool got_ready = !ready_ && ready;
-    ready_ = ready;
-    if (got_ready)
-      callback_list_.Notify();
-  }
-
- private:
-  syncer::DeviceInfo local_device_info_;
-  bool ready_ = true;
-  base::CallbackList<void(void)> callback_list_;
-};
-
 class SharingServiceTest : public testing::Test {
  public:
   SharingServiceTest() {
@@ -228,10 +187,6 @@ class SharingServiceTest : public testing::Test {
     return send_message_result_;
   }
 
-  void OnDeviceCandidatesInitialized() {
-    device_candidates_initialized_ = true;
-  }
-
  protected:
   static std::unique_ptr<syncer::DeviceInfo> CreateFakeDeviceInfo(
       const std::string& id,
@@ -279,7 +234,6 @@ class SharingServiceTest : public testing::Test {
   VapidKeyManager* vapid_key_manager_;
   FakeSharingDeviceRegistration* sharing_device_registration_;
   SharingFCMSender* fcm_sender_;
-  bool device_candidates_initialized_ = false;
 
  private:
   std::unique_ptr<SharingService> sharing_service_ = nullptr;
@@ -683,35 +637,3 @@ TEST_F(SharingServiceTest, NoDevicesWhenSyncDisabled) {
 
   ASSERT_EQ(0u, candidates.size());
 }
-
-TEST_F(SharingServiceTest, DeviceCandidatesAlreadyReady) {
-  std::string id = base::GenerateGUID();
-  std::unique_ptr<syncer::DeviceInfo> device_info =
-      CreateFakeDeviceInfo(id, kDeviceName);
-  device_info_tracker_.Add(device_info.get());
-  fake_local_device_info_provider_.SetReady(true);
-
-  GetSharingService()->AddDeviceCandidatesInitializedObserver(
-      base::BindOnce(&SharingServiceTest::OnDeviceCandidatesInitialized,
-                     base::Unretained(this)));
-
-  ASSERT_TRUE(device_candidates_initialized_);
-}
-
-TEST_F(SharingServiceTest, DeviceCandidatesReadyAfterAddObserver) {
-  fake_local_device_info_provider_.SetReady(false);
-
-  GetSharingService()->AddDeviceCandidatesInitializedObserver(
-      base::BindOnce(&SharingServiceTest::OnDeviceCandidatesInitialized,
-                     base::Unretained(this)));
-
-  ASSERT_FALSE(device_candidates_initialized_);
-
-  std::string id = base::GenerateGUID();
-  std::unique_ptr<syncer::DeviceInfo> device_info =
-      CreateFakeDeviceInfo(id, kDeviceName);
-  device_info_tracker_.Add(device_info.get());
-  fake_local_device_info_provider_.SetReady(true);
-
-  ASSERT_TRUE(device_candidates_initialized_);
-}

components/sync_device_info/fake_device_info_tracker.cc

@@ -49,11 +49,11 @@ FakeDeviceInfoTracker::GetAllDeviceInfo() const {
 }
 
 void FakeDeviceInfoTracker::AddObserver(Observer* observer) {
-  observers_.AddObserver(observer);
+  NOTREACHED();
 }
 
 void FakeDeviceInfoTracker::RemoveObserver(Observer* observer) {
-  observers_.RemoveObserver(observer);
+  NOTREACHED();
 }
 
 int FakeDeviceInfoTracker::CountActiveDevices() const {
@@ -76,14 +76,10 @@ bool FakeDeviceInfoTracker::IsRecentLocalCacheGuid(
 
 void FakeDeviceInfoTracker::Add(const DeviceInfo* device) {
   devices_.push_back(device);
-  for (auto& observer : observers_)
-    observer.OnDeviceInfoChange();
 }
 
 void FakeDeviceInfoTracker::OverrideActiveDeviceCount(int count) {
   active_device_count_ = count;
-  for (auto& observer : observers_)
-    observer.OnDeviceInfoChange();
 }
 
 }  // namespace syncer

components/sync_device_info/fake_device_info_tracker.h

@@ -10,7 +10,6 @@
 #include <vector>
 
 #include "base/macros.h"
-#include "base/observer_list.h"
 #include "base/optional.h"
 #include "components/sync_device_info/device_info_tracker.h"
 
@@ -46,8 +45,6 @@ class FakeDeviceInfoTracker : public DeviceInfoTracker {
   // DeviceInfo stored here are not owned.
   std::vector<const DeviceInfo*> devices_;
   base::Optional<int> active_device_count_;
-  // Registered observers, not owned.
-  base::ObserverList<Observer, true>::Unchecked observers_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeDeviceInfoTracker);
 };