Notify Files App when ARC++ app is installed/removed

chrome/browser/chromeos/extensions/file_manager/event_router.cc

 // Copyright 2014 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/chromeos/extensions/file_manager/event_router.h"
 
 #include <stddef.h>
 
 #include <utility>
 
@@ skipping 19 matching lines @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/login/login_state.h"
 #include "chromeos/network/network_handler.h"
 #include "chromeos/network/network_state_handler.h"
+#include "components/arc/intent_helper/arc_intent_helper_bridge.h"

[Yusuke Sato, 2016/11/17 02:29:54]

nit: Remove. You can rely on the header file in this case.

https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes
reads "However, any includes present in the related header do not need to be
included again in the related cc (i.e., foo.cc can rely on foo.h's includes)."

[oka, 2016/11/21 07:02:15]

Done.

 #include "components/drive/chromeos/file_system_interface.h"
 #include "components/drive/drive_pref_names.h"
 #include "components/drive/file_change.h"
 #include "components/drive/service/drive_service_interface.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/storage_partition.h"
 #include "extensions/browser/event_router.h"
@@ skipping 343 matching lines @@
           base::Bind(&EventRouter::DispatchDirectoryChangeEventImpl,
                      base::Unretained(this))),
       weak_factory_(this) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   ObserveEvents();
 }
 
 EventRouter::~EventRouter() {
 }
 
+void EventRouter::OnAppsUpdated() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  BroadcastEvent(profile_,
+                 extensions::events::FILE_MANAGER_PRIVATE_ON_APPS_UPDATED,
+                 file_manager_private::OnAppsUpdated::kEventName,
+                 file_manager_private::OnAppsUpdated::Create());
+}
+
 void EventRouter::Shutdown() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   chromeos::system::TimezoneSettings::GetInstance()->RemoveObserver(this);
 
   DLOG_IF(WARNING, !file_watchers_.empty())
       << "Not all file watchers are "
       << "removed. This can happen when Files.app is open during shutdown.";
   file_watchers_.clear();
   if (!profile_) {
     NOTREACHED();
@@ skipping 72 matching lines @@
       base::Bind(&EventRouter::OnFileManagerPrefsChanged,
                  weak_factory_.GetWeakPtr());
   pref_change_registrar_->Add(drive::prefs::kDisableDriveOverCellular,
                               callback);
   pref_change_registrar_->Add(drive::prefs::kDisableDriveHostedFiles, callback);
   pref_change_registrar_->Add(drive::prefs::kDisableDrive, callback);
   pref_change_registrar_->Add(prefs::kSearchSuggestEnabled, callback);
   pref_change_registrar_->Add(prefs::kUse24HourClock, callback);
 
   chromeos::system::TimezoneSettings::GetInstance()->AddObserver(this);
+
+  arc::ArcIntentHelperBridge* bridge = arc::ArcIntentHelperBridge::Get();
+  DCHECK(bridge);

[kinaba, 2016/11/17 01:47:19]

When ARC is not enabled (either by config or when the device does not support it
at all),
|bridge| may be null, IIUC.

[Yusuke Sato, 2016/11/17 02:29:54]

Yeah, and you might have to retry later to avoid introducing a race. I'll
comment more on this later.

[Yusuke Sato, 2016/11/17 04:22:37]

Sorry I misread your code. ArcIntentHelperBridge instance is created in
ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun() regardless of whether
ARC is supported or not, so you code doesn't seem to have a race.

hidehiko:
Can |bridge| be nullptr here? I don't remember if ArcServiceManager's Shutdown()
is actually called on production code.

[hidehiko, 2016/11/17 07:34:19]

I think no. In past, the launcher creation was guarded by a flag IIRC, but
nowadays it is created always. I'm not sure it is ensured in all test cases,
though.
It is.
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/chrome_browser_m...

Note that the Shutdown is called before the BrowserContextKeyedService is
destructed.

[Yusuke Sato, 2016/11/17 09:15:27]

Got it, thanks for refreshing my memory.

+  bridge->AddObserver(this);

[Yusuke Sato, 2016/11/17 02:29:54]

AddObserver should almost always be paired with RemoveObserver. For other
observer interfaces, EventRouter::Shutdown() seems to call RemoveObserver in the
reverse order. Shouldn't we do that too?

[oka, 2016/11/21 07:02:15]

Done.

 }
 
 // File watch setup routines.
 void EventRouter::AddFileWatch(const base::FilePath& local_path,
                                const base::FilePath& virtual_path,
                                const std::string& extension_id,
                                const BoolCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(!callback.is_null());
 
@@ skipping 479 matching lines @@
 void EventRouter::SetDispatchDirectoryChangeEventImplForTesting(
     const DispatchDirectoryChangeEventImplCallback& callback) {
   dispatch_directory_change_event_impl_ = callback;
 }
 
 base::WeakPtr<EventRouter> EventRouter::GetWeakPtr() {
   return weak_factory_.GetWeakPtr();
 }
 
 }  // namespace file_manager