Cleanup: Drop some unnecessary params from the BluetoothSocketNet constructor.

Cleanup: Drop some unnecessary params from the BluetoothSocketNet constructor.

BUG=none
TEST=none
R=keybuk@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284857

[keybuk, 2014-07-16 20:50:42]

+rpaquay, xiyuan

https://codereview.chromium.org/397123002/diff/1/device/bluetooth/bluetooth_s...
File device/bluetooth/bluetooth_socket_chromeos.cc (right):

https://codereview.chromium.org/397123002/diff/1/device/bluetooth/bluetooth_s...
device/bluetooth/bluetooth_socket_chromeos.cc:80: ui_task_runner, socket_thread,
NULL, net::NetLog::Source()) {
This is done for consistency with BluetoothSocketWin, so if we make this change
here, we should make it on that platform too - since they both share
BluetoothSocketNet as an implementation

[Ilya Sherman, 2014-07-16 21:59:28]

https://codereview.chromium.org/397123002/diff/1/device/bluetooth/bluetooth_s...
File device/bluetooth/bluetooth_socket_chromeos.cc (right):

https://codereview.chromium.org/397123002/diff/1/device/bluetooth/bluetooth_s...
device/bluetooth/bluetooth_socket_chromeos.cc:80: ui_task_runner, socket_thread,
NULL, net::NetLog::Source()) {
On 2014/07/16 20:50:42, keybuk wrote:
> This is done for consistency with BluetoothSocketWin, so if we make this
change
> here, we should make it on that platform too - since they both share
> BluetoothSocketNet as an implementation

You're right, these parameters aren't needed on Windows either.  I've now
removed them throughout.

[rpaquay, 2014-07-17 18:21:30]

https://codereview.chromium.org/397123002/diff/1/device/bluetooth/bluetooth_s...
File device/bluetooth/bluetooth_socket_chromeos.cc (right):

https://codereview.chromium.org/397123002/diff/1/device/bluetooth/bluetooth_s...
device/bluetooth/bluetooth_socket_chromeos.cc:80: ui_task_runner, socket_thread,
NULL, net::NetLog::Source()) {
On 2014/07/16 21:59:27, Ilya Sherman wrote:
> On 2014/07/16 20:50:42, keybuk wrote:
> > This is done for consistency with BluetoothSocketWin, so if we make this
> change
> > here, we should make it on that platform too - since they both share
> > BluetoothSocketNet as an implementation
> 
> You're right, these parameters aren't needed on Windows either.  I've now
> removed them throughout.

Do you guys know what there parameters are used for when passed to
net::TCPSocket? From my understanding, they are used for logging network
activity, but I could not figure out where actual (i.e. non default values) are
coming from in the "browser" layer.

I originally exposed them in the "device" API because I figured they must be
useful for something, and it didn't seem obvious to me how the "device" layer
could pass non-default meaningful values to net::TCPSocket.

[xiyuan, 2014-07-17 18:28:24]

On 2014/07/17 18:21:30, rpaquay wrote:
>
https://codereview.chromium.org/397123002/diff/1/device/bluetooth/bluetooth_s...
> File device/bluetooth/bluetooth_socket_chromeos.cc (right):
> 
>
https://codereview.chromium.org/397123002/diff/1/device/bluetooth/bluetooth_s...
> device/bluetooth/bluetooth_socket_chromeos.cc:80: ui_task_runner,
socket_thread,
> NULL, net::NetLog::Source()) {
> On 2014/07/16 21:59:27, Ilya Sherman wrote:
> > On 2014/07/16 20:50:42, keybuk wrote:
> > > This is done for consistency with BluetoothSocketWin, so if we make this
> > change
> > > here, we should make it on that platform too - since they both share
> > > BluetoothSocketNet as an implementation
> > 
> > You're right, these parameters aren't needed on Windows either.  I've now
> > removed them throughout.
> 
> Do you guys know what there parameters are used for when passed to
> net::TCPSocket? From my understanding, they are used for logging network
> activity, but I could not figure out where actual (i.e. non default values)
are
> coming from in the "browser" layer.
> 
> I originally exposed them in the "device" API because I figured they must be
> useful for something, and it didn't seem obvious to me how the "device" layer
> could pass non-default meaningful values to net::TCPSocket.

My guess is the one in browser process:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/bro...

Then passed down via ChromeContentBrowserClient::GetNetLog
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...

[xiyuan, 2014-07-17 18:34:36]

On 2014/07/17 18:28:24, xiyuan wrote:
> On 2014/07/17 18:21:30, rpaquay wrote:
> >
>
https://codereview.chromium.org/397123002/diff/1/device/bluetooth/bluetooth_s...
> > File device/bluetooth/bluetooth_socket_chromeos.cc (right):
> > 
> >
>
https://codereview.chromium.org/397123002/diff/1/device/bluetooth/bluetooth_s...
> > device/bluetooth/bluetooth_socket_chromeos.cc:80: ui_task_runner,
> socket_thread,
> > NULL, net::NetLog::Source()) {
> > On 2014/07/16 21:59:27, Ilya Sherman wrote:
> > > On 2014/07/16 20:50:42, keybuk wrote:
> > > > This is done for consistency with BluetoothSocketWin, so if we make this
> > > change
> > > > here, we should make it on that platform too - since they both share
> > > > BluetoothSocketNet as an implementation
> > > 
> > > You're right, these parameters aren't needed on Windows either.  I've now
> > > removed them throughout.
> > 
> > Do you guys know what there parameters are used for when passed to
> > net::TCPSocket? From my understanding, they are used for logging network
> > activity, but I could not figure out where actual (i.e. non default values)
> are
> > coming from in the "browser" layer.
> > 
> > I originally exposed them in the "device" API because I figured they must be
> > useful for something, and it didn't seem obvious to me how the "device"
layer
> > could pass non-default meaningful values to net::TCPSocket.
> 
> My guess is the one in browser process:
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/bro...
> 
> Then passed down via ChromeContentBrowserClient::GetNetLog
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...

And set as member of IOthread and pass along with ChromeURLRequestContext
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/bro...
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/pro...

[keybuk, 2014-07-17 18:35:09]

https://codereview.chromium.org/397123002/diff/20001/device/bluetooth/bluetoo...
File device/bluetooth/bluetooth_socket_chromeos.cc (right):

https://codereview.chromium.org/397123002/diff/20001/device/bluetooth/bluetoo...
device/bluetooth/bluetooth_socket_chromeos.cc:63: new
BluetoothSocketChromeOS(ui_task_runner, socket_thread));
ui_task_runner and socket_thread are singletons too, no?

https://codereview.chromium.org/397123002/diff/20001/device/bluetooth/bluetoo...
File device/bluetooth/bluetooth_socket_win.cc (right):

https://codereview.chromium.org/397123002/diff/20001/device/bluetooth/bluetoo...
device/bluetooth/bluetooth_socket_win.cc:82: new
BluetoothSocketWin(ui_task_runner, socket_thread));
these are both singletons too, they're passed down from the adapter right now -
but is there any reason that must be the case?

(since we're not passing net::NetLog and net::NetLog::Source down now)

[Ilya Sherman, 2014-07-17 18:41:09]

https://codereview.chromium.org/397123002/diff/20001/device/bluetooth/bluetoo...
File device/bluetooth/bluetooth_socket_win.cc (right):

https://codereview.chromium.org/397123002/diff/20001/device/bluetooth/bluetoo...
device/bluetooth/bluetooth_socket_win.cc:82: new
BluetoothSocketWin(ui_task_runner, socket_thread));
On 2014/07/17 18:35:09, keybuk wrote:
> these are both singletons too, they're passed down from the adapter right now
-
> but is there any reason that must be the case?
> 
> (since we're not passing net::NetLog and net::NetLog::Source down now)

I think it's a little different -- the other params were not singletons, but
rather just null/default values.  These two are singletons, but they're passed
from the adapter into other classes as well.  Since they're shared among
multiple classes, I think it makes sense for the adapter to keep the master
references, and pass them into the classes it manages, rather than having each
of those classes access the singleton directly.  That way, there's only one
place to update if we ever want to change e.g. how we choose the socket thread.

[keybuk, 2014-07-22 17:51:48]

lgtm

[Ilya Sherman, 2014-07-22 20:53:15]

The CQ bit was checked by isherman@chromium.org

[commit-bot: I haz the power, 2014-07-22 20:56:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/isherman@chromium.org/397123002/20001

[commit-bot: I haz the power, 2014-07-22 21:49:36]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/9...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/20...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel_swarming on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel_sw...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win8_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win8_chromium_rel/bui...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/31703)

[commit-bot: I haz the power, 2014-07-22 21:51:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-22 21:51:33]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/31741)

[Ilya Sherman, 2014-07-22 23:28:29]

The CQ bit was checked by isherman@chromium.org

[commit-bot: I haz the power, 2014-07-22 23:29:55]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/isherman@chromium.org/397123002/40001

[commit-bot: I haz the power, 2014-07-23 02:26:19]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-07-23 06:18:57]

Change committed as 284857