Reporting of policy errors via host-offline-reason: part 3

remoting/host/dns_blackhole_checker.h

 // Copyright (c) 2012 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.
 
 #ifndef REMOTING_HOST_DNS_BLACKHOLE_CHECKER_H_
 #define REMOTING_HOST_DNS_BLACKHOLE_CHECKER_H_
 
 #include "net/url_request/url_fetcher_delegate.h"
 
 #include "base/callback.h"
 
 namespace net {
 class URLRequestContextGetter;
 }  // namespace net
 
 namespace remoting {
 
 // This is the default prefix that is prepended to the kTalkGadgetUrl to form
 // the complete talkgadget URL used by the host. Policy settings allow admins
 // to change the prefix that is used.
 extern const char kDefaultHostTalkGadgetPrefix[];
 
 class DnsBlackholeChecker : public net::URLFetcherDelegate {
  public:
-  DnsBlackholeChecker(
-      scoped_refptr<net::URLRequestContextGetter> url_request_context_getter,
-      std::string talkgadget_prefix);
+  DnsBlackholeChecker(const scoped_refptr<net::URLRequestContextGetter>&
+                          url_request_context_getter,

[Wez, 2015/01/08 01:49:50]

What's the rationale for making this const scoped_refptr&?

[Łukasz Anforowicz, 2015/01/08 23:50:31]

When following your suggestion to pass network_task_runner +
url_request_context_getter directly (rather than wrapped in chromoting_context),
I've introduced parameters to HostSignalingManager::Create with const
scoped_refptr<X>& declaration.  This declaration avoids unnecessary
addref/release and ref-count churn (I am not sure now where exactly I read the
advise to pass scoped_refptr by const-ref rather than by value).

When fixing usage of these parameters in HostSignalingManager::Create, I've
noticed that we can also change the declarations in here (and a few other
places).  I've checked that in all of those cases the parameter (now changed to
const scoped_refptr<X>&) is just used in the constructor to initialize a field
of scoped_refptr<X> type, so this change seemed straightforward and safe.

[Wez, 2015/01/09 02:55:45]

Acknowledged.

+                      std::string talkgadget_prefix);

[Wez, 2015/01/08 01:49:50]

Where did this weird wrapping come from?

[Łukasz Anforowicz, 2015/01/08 23:50:31]

git cl format

The thing is that the 1st parameter won't fit on a single line even when
indented by 4 spaces (rather than continuing right after the opening paren).

I've changed "url_request_context_getter" to "request_context_getter" (this name
is already used by xmpp_signal_strategy.h) and now it fits in 80 columns.

[Wez, 2015/01/09 02:55:45]

Acknowledged.

   ~DnsBlackholeChecker() override;
 
   // net::URLFetcherDelegate interface.
   void OnURLFetchComplete(const net::URLFetcher* source) override;
 
   // Initiates a check the verify that the host talkgadget has not been "DNS
   // blackholed" to prevent connections. If this is called again before the
   // callback has been called, then the second call is ignored.
   void CheckForDnsBlackhole(const base::Callback<void(bool)>& callback);
 
@@ skipping 10 matching lines @@
 
   // Called with the results of the connection check.
   base::Callback<void(bool)> callback_;
 
   DISALLOW_COPY_AND_ASSIGN(DnsBlackholeChecker);
 };
 
 }  // namespace remoting
 
 #endif  // REMOTING_HOST_DNS_BLACKHOLE_CHECKER_H_