Resolve issue from feedback for 1417363004

chrome/browser/extensions/api/gcd_private/gcd_private_api.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/extensions/api/gcd_private/gcd_private_api.h"
 
+#include "base/command_line.h"
 #include "base/lazy_instance.h"
 #include "base/location.h"
 #include "base/memory/linked_ptr.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/stringprintf.h"
 #include "base/thread_task_runner_handle.h"
 #include "chrome/browser/local_discovery/cloud_device_list.h"
 #include "chrome/browser/local_discovery/cloud_print_printer_list.h"
 #include "chrome/browser/local_discovery/gcd_api_flow.h"
 #include "chrome/browser/local_discovery/gcd_constants.h"
 #include "chrome/browser/local_discovery/privet_device_lister_impl.h"
 #include "chrome/browser/local_discovery/privet_http_asynchronous_factory.h"
 #include "chrome/browser/local_discovery/privet_http_impl.h"
 #include "chrome/browser/local_discovery/privetv3_session.h"
 #include "chrome/browser/local_discovery/service_discovery_shared_client.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
+#include "chrome/common/chrome_switches.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
 #include "components/signin/core/browser/signin_manager.h"
 #include "components/signin/core/browser/signin_manager_base.h"
 #include "extensions/browser/event_router.h"
 #include "net/base/net_util.h"
 
 #if defined(ENABLE_WIFI_BOOTSTRAPPING)
 #include "chrome/browser/local_discovery/wifi/wifi_manager.h"
 #endif
 
@@ skipping 33 matching lines @@
                                     params.Pass()));
   return event.Pass();
 }
 
 GcdPrivateAPI::GCDApiFlowFactoryForTests* g_gcd_api_flow_factory = NULL;
 
 base::LazyInstance<BrowserContextKeyedAPIFactory<GcdPrivateAPI> > g_factory =
     LAZY_INSTANCE_INITIALIZER;
 
 scoped_ptr<local_discovery::GCDApiFlow> MakeGCDApiFlow(Profile* profile) {
   if (g_gcd_api_flow_factory) {

[Ryan Sleevi, 2015/10/30 23:17:27]

FWIW this bracing is inconsistent w/ the rest of this function/file

[Vitaly Buka (NO REVIEWS), 2015/10/31 03:55:35]

Done.

     return g_gcd_api_flow_factory->CreateGCDApiFlow();
   }
 
   ProfileOAuth2TokenService* token_service =
       ProfileOAuth2TokenServiceFactory::GetForProfile(profile);
   if (!token_service)
     return scoped_ptr<local_discovery::GCDApiFlow>();
   SigninManagerBase* signin_manager =
       SigninManagerFactory::GetInstance()->GetForProfile(profile);
   if (!signin_manager)
@@ skipping 125 matching lines @@
     EventRouter::Get(context)
         ->RegisterObserver(this, gcd_private::OnDeviceStateChanged::kEventName);
     EventRouter::Get(context)
         ->RegisterObserver(this, gcd_private::OnDeviceRemoved::kEventName);
   }
 }
 
 GcdPrivateAPIImpl::~GcdPrivateAPIImpl() {
   if (EventRouter::Get(browser_context_)) {
     EventRouter::Get(browser_context_)->UnregisterObserver(this);
   }

[Ryan Sleevi, 2015/10/30 23:17:27]

more inconsistent bracing

[Vitaly Buka (NO REVIEWS), 2015/10/31 03:55:35]

Done.

 }
 
 void GcdPrivateAPIImpl::OnListenerAdded(const EventListenerInfo& details) {
   if (details.event_name == gcd_private::OnDeviceStateChanged::kEventName ||
       details.event_name == gcd_private::OnDeviceRemoved::kEventName) {
     num_device_listeners_++;
 
     if (num_device_listeners_ == 1) {
       service_discovery_client_ =
           local_discovery::ServiceDiscoverySharedClient::GetInstance();
@@ skipping 36 matching lines @@
   if (!description.id.empty())
     device->cloud_id.reset(new std::string(description.id));
 
   known_devices_[device->device_id] = device;
 
   EventRouter::Get(browser_context_)
       ->BroadcastEvent(MakeDeviceStateChangedEvent(*device));
 }
 
 void GcdPrivateAPIImpl::DeviceRemoved(const std::string& name) {
   GCDDeviceMap::iterator found = known_devices_.find(kIDPrefixMdns + name);

[Ryan Sleevi, 2015/10/30 23:17:27]

BUG: Can't found == .end() under exceptional cases here? It seems it could,
which means 285 would be a security bug.

It's taken me quite a bit of code to figure out whether or not this information
is validated or not, but looking at
//chrome/common/local_discovery/service_discovery_client_impl and the related
thread hops, it 'seems' as if you can double-advertise a device removal and
cause mischief, based on the thread-hops involved.

[Vitaly Buka (NO REVIEWS), 2015/10/31 03:55:35]

Thanks. Quite possible.
I suspect half of this file is irrelevant now.
crbug.com/549868

On 2015/10/30 23:17:27, Ryan Sleevi wrote:

   linked_ptr<gcd_private::GCDDevice> device = found->second;
   known_devices_.erase(found);
 
   EventRouter::Get(browser_context_)
       ->BroadcastEvent(MakeDeviceRemovedEvent(device->device_id));
 }
 
 void GcdPrivateAPIImpl::DeviceCacheFlushed() {
   for (GCDDeviceMap::iterator i = known_devices_.begin();
        i != known_devices_.end();
@@ skipping 17 matching lines @@
     return false;
 
   privet_device_lister_->DiscoverNewDevices(true);
 
   return true;
 }
 
 void GcdPrivateAPIImpl::CreateSession(const std::string& service_name,
                                       const CreateSessionCallback& callback) {
   int session_id = last_session_id_++;
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnablePrivetV3)) {
+    return callback.Run(session_id, gcd_private::STATUS_SESSIONERROR,
+                        base::DictionaryValue());
+  }
   scoped_ptr<local_discovery::PrivetHTTPAsynchronousFactory> factory(
       local_discovery::PrivetHTTPAsynchronousFactory::CreateInstance(
           browser_context_->GetRequestContext()));
   auto& session_data = sessions_[session_id];
   session_data.http_resolution.reset(
       factory->CreatePrivetHTTP(service_name).release());
   session_data.http_resolution->Start(
       base::Bind(&GcdPrivateAPIImpl::OnServiceResolved, base::Unretained(this),
                  session_id, callback));
 }
@@ skipping 552 matching lines @@
 }
 
 GcdPrivateGetCommandsListFunction::~GcdPrivateGetCommandsListFunction() {
 }
 
 bool GcdPrivateGetCommandsListFunction::RunAsync() {
   return false;
 }
 
 }  // namespace extensions