ClientConnection Unittest memory leak fixed

Revert of Revert of Substituting legacy protocol with uWeave protocol (patchset #1 id:1 of https://codereview.chromium.org/2189913002/ )

Reason for revert:
The memory leak that caused Substituting legacy protocol with uWeave protocol to be reverted is fixed.

First build showing the error:
n/a

Error text:
n/a

Original issue's description:
>Revert of Substituting legacy protocol with uWeave protocol (patchset #33 id:630001 of https://codereview.chromium.org/2075313002/ )
>
>Reason for revert:
>This CL has a memory problem in the unit tests, as reported by DrMemory.
>
>First build showing the error: >https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%29/builds/5364
>
>Note, that build doesn't include this change as there was a problem introduced earlier that caused the whole build to fail.
>
>Error text:
>UNADDRESSABLE ACCESS of freed memory: reading 0x069fa848-0x069fa84c 4 byte(s)
># 0 >proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance [components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator.cc:33]
># 1 >proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody [components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator_unittest.cc:54]
># 2 >testing::internal::HandleExceptionsInMethodIfSupported<>                   [testing\gtest\src\gtest.cc:2458]
>Note: @0:05:46.506 in thread 13092
>Note: 0x069fa848-0x069fa84c overlaps memory 0x069fa5d8-0x069fa8c0 that was freed here:
>Note: # 0 replace_operator_delete_nothrow                                            [d:\drmemory_package\common\alloc_replace.c:2974]
>Note: # 1 >proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeaveClientConnectionTest_ConnectAfterADelayWhenThrottled_Test::`scalar deleting destructor'
>Note: # 2 testing::Test::DeleteSelf_                                                 [testing\gtest\include\gtest\gtest.h:453]
>Note: # 3 testing::TestInfo::Run                                                     [testing\gtest\src\gtest.cc:2661]
>Note: instruction: mov    (%eax) -> %edx
>Suppression (error hash=#E743E83C1B0FB1CF#):
>For more info on using suppressions see >http://dev.chromium.org/developers/how-tos/using-drmemory#TOC-Suppressing-error-reports-from-the-
>{
>UNADDRESSABLE ACCESS
>name=<insert_a_suppression_name_here>
>*!proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance
>*!proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody
>*!testing::internal::HandleExceptionsInMethodIfSupported<>
>}
>
>The problem is real, to fix this SetInstanceForTesting should keep the previous factory around somewhere, and there should be a method to clear the testing instance. Or something like that.

BUG=617238
Committed: https://crrev.com/2af35dd45f0a075fa0ce835302aeab0af3a4a392
Cr-Commit-Position: refs/heads/master@{#409618}

[jingxuy, 2016-07-28 18:33:46]

Description was changed from

==========
memory leak fix

BUG=
==========

to

==========
CL 2075313002 Substituting legacy protocol with uWeave protocol is reverted due
to memory leak in the ClientConnection's unittest.

The leak is due to the generator and receiver factory's SetInstanceForTesting
function. The function sets the factory_instance_ pointer to a new pointer
without deallocating the old pointer.

This is fixed by shared_ptr memory management of factory_instance_.

BUG=617238
==========

[jingxuy, 2016-07-28 18:33:47]

jingxuy@google.com changed reviewers:
+ khorimoto@chromium.org, msarda@chromium.org, sacomoto@chromium.org,
tengs@chromium.org

[Kyle Horimoto, 2016-07-28 18:37:45]

Your CL description should use the "revert" format like the CL which reverted
your change did: https://codereview.chromium.org/2189913002/. Please use that
format.

Regarding your actual CL, can you please add a CL comment (not in the code, just
in this CL) which shows the fix that you made to fix the memory leak? Thanks!

[jingxuy, 2016-07-28 18:51:10]

Patchset #1 (id:1) has been deleted

[jingxuy, 2016-07-28 19:03:22]

Description was changed from

==========
CL 2075313002 Substituting legacy protocol with uWeave protocol is reverted due
to memory leak in the ClientConnection's unittest.

The leak is due to the generator and receiver factory's SetInstanceForTesting
function. The function sets the factory_instance_ pointer to a new pointer
without deallocating the old pointer.

This is fixed by shared_ptr memory management of factory_instance_.

BUG=617238
==========

to

==========
Revert of Revert of Substituting legacy protocol with uWeave protocol (patchset
#1 id:1 of https://codereview.chromium.org/2189913002/ )

Reason for revert:
The memory leak that caused Substituting legacy protocol with uWeave protocol to
be reverted is fixed.

First build showing the error: 
n/a

Error text:
n/a
                          
Original issue's description:
TODO(jingxuy) move description here.

BUG=617238
==========

[jingxuy, 2016-07-28 21:13:43]

Description was changed from

==========
Revert of Revert of Substituting legacy protocol with uWeave protocol (patchset
#1 id:1 of https://codereview.chromium.org/2189913002/ )

Reason for revert:
The memory leak that caused Substituting legacy protocol with uWeave protocol to
be reverted is fixed.

First build showing the error: 
n/a

Error text:
n/a
                          
Original issue's description:
TODO(jingxuy) move description here.

BUG=617238
==========

to

==========
Revert of Revert of Substituting legacy protocol with uWeave protocol (patchset
#1 id:1 of https://codereview.chromium.org/2189913002/ )

Reason for revert:
The memory leak that caused Substituting legacy protocol with uWeave protocol to
be reverted is fixed.

First build showing the error: 
n/a

Error text:
n/a
                          
Original issue's description:
Revert of Substituting legacy protocol with uWeave protocol (patchset #33
id:630001 of https://codereview.chromium.org/2075313002/ )

Reason for revert:
This CL has a memory problem in the unit tests, as reported by DrMemory.

First build showing the error:
https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%2...

Note, that build doesn't include this change as there was a problem introduced
earlier that caused the whole build to fail.

Error text:
UNADDRESSABLE ACCESS of freed memory: reading 0x069fa848-0x069fa84c 4 byte(s)
# 0
proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance
[components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator.cc:33]
# 1
proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody
[components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator_unittest.cc:54]
# 2 testing::internal::HandleExceptionsInMethodIfSupported<>                  
[testing\gtest\src\gtest.cc:2458]
Note: @0:05:46.506 in thread 13092
Note: 0x069fa848-0x069fa84c overlaps memory 0x069fa5d8-0x069fa8c0 that was freed
here:
Note: # 0 replace_operator_delete_nothrow                                       
    [d:\drmemory_package\common\alloc_replace.c:2974]
Note: # 1
proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeaveClientConnectionTest_ConnectAfterADelayWhenThrottled_Test::`scalar
deleting destructor'
Note: # 2 testing::Test::DeleteSelf_                                            
    [testing\gtest\include\gtest\gtest.h:453]
Note: # 3 testing::TestInfo::Run                                                
    [testing\gtest\src\gtest.cc:2661]
Note: instruction: mov    (%eax) -> %edx
Suppression (error hash=#E743E83C1B0FB1CF#):
For more info on using suppressions see
http://dev.chromium.org/developers/how-tos/using-drmemory#TOC-Suppressing-err...
{
UNADDRESSABLE ACCESS
name=<insert_a_suppression_name_here>
*!proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance
*!proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody
*!testing::internal::HandleExceptionsInMethodIfSupported<>
}

The problem is real, to fix this SetInstanceForTesting should keep the previous
factory around somewhere, and there should be a method to clear the testing
instance. Or something like that.


BUG=617238
==========

[jingxuy, 2016-07-28 21:17:55]

Description was changed from

==========
Revert of Revert of Substituting legacy protocol with uWeave protocol (patchset
#1 id:1 of https://codereview.chromium.org/2189913002/ )

Reason for revert:
The memory leak that caused Substituting legacy protocol with uWeave protocol to
be reverted is fixed.

First build showing the error: 
n/a

Error text:
n/a
                          
Original issue's description:
Revert of Substituting legacy protocol with uWeave protocol (patchset #33
id:630001 of https://codereview.chromium.org/2075313002/ )

Reason for revert:
This CL has a memory problem in the unit tests, as reported by DrMemory.

First build showing the error:
https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%2...

Note, that build doesn't include this change as there was a problem introduced
earlier that caused the whole build to fail.

Error text:
UNADDRESSABLE ACCESS of freed memory: reading 0x069fa848-0x069fa84c 4 byte(s)
# 0
proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance
[components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator.cc:33]
# 1
proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody
[components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator_unittest.cc:54]
# 2 testing::internal::HandleExceptionsInMethodIfSupported<>                  
[testing\gtest\src\gtest.cc:2458]
Note: @0:05:46.506 in thread 13092
Note: 0x069fa848-0x069fa84c overlaps memory 0x069fa5d8-0x069fa8c0 that was freed
here:
Note: # 0 replace_operator_delete_nothrow                                       
    [d:\drmemory_package\common\alloc_replace.c:2974]
Note: # 1
proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeaveClientConnectionTest_ConnectAfterADelayWhenThrottled_Test::`scalar
deleting destructor'
Note: # 2 testing::Test::DeleteSelf_                                            
    [testing\gtest\include\gtest\gtest.h:453]
Note: # 3 testing::TestInfo::Run                                                
    [testing\gtest\src\gtest.cc:2661]
Note: instruction: mov    (%eax) -> %edx
Suppression (error hash=#E743E83C1B0FB1CF#):
For more info on using suppressions see
http://dev.chromium.org/developers/how-tos/using-drmemory#TOC-Suppressing-err...
{
UNADDRESSABLE ACCESS
name=<insert_a_suppression_name_here>
*!proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance
*!proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody
*!testing::internal::HandleExceptionsInMethodIfSupported<>
}

The problem is real, to fix this SetInstanceForTesting should keep the previous
factory around somewhere, and there should be a method to clear the testing
instance. Or something like that.


BUG=617238
==========

to

==========
Revert of Revert of Substituting legacy protocol with uWeave protocol (patchset
#1 id:1 of https://codereview.chromium.org/2189913002/ )

Reason for revert:
The memory leak that caused Substituting legacy protocol with uWeave protocol to
be reverted is fixed.

First build showing the error: 
n/a

Error text:
n/a
                          
Original issue's description:
>Revert of Substituting legacy protocol with uWeave protocol (patchset #33
id:630001 of https://codereview.chromium.org/2075313002/ )
>
>Reason for revert:
>This CL has a memory problem in the unit tests, as reported by DrMemory.
>
>First build showing the error:
>https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%29/builds/5364
>
>Note, that build doesn't include this change as there was a problem introduced
earlier that caused the whole build to fail.
>
>Error text:
>UNADDRESSABLE ACCESS of freed memory: reading 0x069fa848-0x069fa84c 4 byte(s)
># 0
>proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance
[components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator.cc:33]
># 1
>proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody
[components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator_unittest.cc:54]
># 2 >testing::internal::HandleExceptionsInMethodIfSupported<>                  
[testing\gtest\src\gtest.cc:2458]
>Note: @0:05:46.506 in thread 13092
>Note: 0x069fa848-0x069fa84c overlaps memory 0x069fa5d8-0x069fa8c0 that was
freed here:
>Note: # 0 replace_operator_delete_nothrow                                      
     [d:\drmemory_package\common\alloc_replace.c:2974]
>Note: # 1
>proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeaveClientConnectionTest_ConnectAfterADelayWhenThrottled_Test::`scalar
deleting destructor'
>Note: # 2 testing::Test::DeleteSelf_                                           
     [testing\gtest\include\gtest\gtest.h:453]
>Note: # 3 testing::TestInfo::Run                                               
     [testing\gtest\src\gtest.cc:2661]
>Note: instruction: mov    (%eax) -> %edx
>Suppression (error hash=#E743E83C1B0FB1CF#):
>For more info on using suppressions see
>http://dev.chromium.org/developers/how-tos/using-drmemory#TOC-Suppressing-error-reports-from-the-
>{
>UNADDRESSABLE ACCESS
>name=<insert_a_suppression_name_here>
>*!proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance
>*!proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody
>*!testing::internal::HandleExceptionsInMethodIfSupported<>
>}
>
>The problem is real, to fix this SetInstanceForTesting should keep the previous
factory around somewhere, and there should be a method to clear the testing
instance. Or something like that.


BUG=617238
==========

[Kyle Horimoto, 2016-08-02 00:04:46]

lgtm

https://codereview.chromium.org/2188053003/diff/40001/components/proximity_au...
File
components/proximity_auth/ble/bluetooth_low_energy_weave_packet_generator.cc
(right):

https://codereview.chromium.org/2188053003/diff/40001/components/proximity_au...
components/proximity_auth/ble/bluetooth_low_energy_weave_packet_generator.cc:30:
if (!factory_instance_) {
nit: Please keep the == nullptr comparison. This is more type-safe than
comparing with NULL. Same in other class.

[sacomoto, 2016-08-02 17:37:35]

lgtm

[jingxuy, 2016-08-03 18:04:38]

https://codereview.chromium.org/2188053003/diff/40001/components/proximity_au...
File
components/proximity_auth/ble/bluetooth_low_energy_weave_packet_generator.cc
(right):

https://codereview.chromium.org/2188053003/diff/40001/components/proximity_au...
components/proximity_auth/ble/bluetooth_low_energy_weave_packet_generator.cc:30:
if (!factory_instance_) {
On 2016/08/02 00:04:45, Kyle Horimoto wrote:
> nit: Please keep the == nullptr comparison. This is more type-safe than
> comparing with NULL. Same in other class.

I had the same question. The pointer is not a raw pointer but a shared pointer.
I looked up how to check for null for shared_ptr and apparently bool is supposed
to be the right way.

http://www.cplusplus.com/reference/memory/shared_ptr/operator%20bool/

[Kyle Horimoto, 2016-08-03 18:24:06]

https://codereview.chromium.org/2188053003/diff/40001/components/proximity_au...
File
components/proximity_auth/ble/bluetooth_low_energy_weave_packet_generator.cc
(right):

https://codereview.chromium.org/2188053003/diff/40001/components/proximity_au...
components/proximity_auth/ble/bluetooth_low_energy_weave_packet_generator.cc:23:
std::shared_ptr<BluetoothLowEnergyWeavePacketGenerator::Factory>
Now that you're using a class instead of a pointer, this assignment is
unnecessary. shared_ptr initializes its managed pointer to nullptr.

https://codereview.chromium.org/2188053003/diff/40001/components/proximity_au...
components/proximity_auth/ble/bluetooth_low_energy_weave_packet_generator.cc:30:
if (!factory_instance_) {
On 2016/08/03 18:04:38, jingxuy wrote:
> On 2016/08/02 00:04:45, Kyle Horimoto wrote:
> > nit: Please keep the == nullptr comparison. This is more type-safe than
> > comparing with NULL. Same in other class.
> 
> I had the same question. The pointer is not a raw pointer but a shared
pointer.
> I looked up how to check for null for shared_ptr and apparently bool is
supposed
> to be the right way.
> 
> http://www.cplusplus.com/reference/memory/shared_ptr/operator%20bool/

I meant to use `if (factory_instance_.get() == nullptr)`, not direct comparison.
But sure, this is fine too.

[jingxuy, 2016-08-03 18:29:50]

The CQ bit was checked by jingxuy@google.com

[commit-bot: I haz the power, 2016-08-03 18:30:31]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-03 18:39:29]

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

[commit-bot: I haz the power, 2016-08-03 18:39:30]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Tim Song, 2016-08-03 20:18:31]

lgtm

[jingxuy, 2016-08-03 20:21:10]

The CQ bit was checked by jingxuy@google.com

[commit-bot: I haz the power, 2016-08-03 20:21:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-03 21:04:33]

Description was changed from

==========
Revert of Revert of Substituting legacy protocol with uWeave protocol (patchset
#1 id:1 of https://codereview.chromium.org/2189913002/ )

Reason for revert:
The memory leak that caused Substituting legacy protocol with uWeave protocol to
be reverted is fixed.

First build showing the error: 
n/a

Error text:
n/a
                          
Original issue's description:
>Revert of Substituting legacy protocol with uWeave protocol (patchset #33
id:630001 of https://codereview.chromium.org/2075313002/ )
>
>Reason for revert:
>This CL has a memory problem in the unit tests, as reported by DrMemory.
>
>First build showing the error:
>https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%29/builds/5364
>
>Note, that build doesn't include this change as there was a problem introduced
earlier that caused the whole build to fail.
>
>Error text:
>UNADDRESSABLE ACCESS of freed memory: reading 0x069fa848-0x069fa84c 4 byte(s)
># 0
>proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance
[components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator.cc:33]
># 1
>proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody
[components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator_unittest.cc:54]
># 2 >testing::internal::HandleExceptionsInMethodIfSupported<>                  
[testing\gtest\src\gtest.cc:2458]
>Note: @0:05:46.506 in thread 13092
>Note: 0x069fa848-0x069fa84c overlaps memory 0x069fa5d8-0x069fa8c0 that was
freed here:
>Note: # 0 replace_operator_delete_nothrow                                      
     [d:\drmemory_package\common\alloc_replace.c:2974]
>Note: # 1
>proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeaveClientConnectionTest_ConnectAfterADelayWhenThrottled_Test::`scalar
deleting destructor'
>Note: # 2 testing::Test::DeleteSelf_                                           
     [testing\gtest\include\gtest\gtest.h:453]
>Note: # 3 testing::TestInfo::Run                                               
     [testing\gtest\src\gtest.cc:2661]
>Note: instruction: mov    (%eax) -> %edx
>Suppression (error hash=#E743E83C1B0FB1CF#):
>For more info on using suppressions see
>http://dev.chromium.org/developers/how-tos/using-drmemory#TOC-Suppressing-error-reports-from-the-
>{
>UNADDRESSABLE ACCESS
>name=<insert_a_suppression_name_here>
>*!proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance
>*!proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody
>*!testing::internal::HandleExceptionsInMethodIfSupported<>
>}
>
>The problem is real, to fix this SetInstanceForTesting should keep the previous
factory around somewhere, and there should be a method to clear the testing
instance. Or something like that.


BUG=617238
==========

to

==========
Revert of Revert of Substituting legacy protocol with uWeave protocol (patchset
#1 id:1 of https://codereview.chromium.org/2189913002/ )

Reason for revert:
The memory leak that caused Substituting legacy protocol with uWeave protocol to
be reverted is fixed.

First build showing the error: 
n/a

Error text:
n/a
                          
Original issue's description:
>Revert of Substituting legacy protocol with uWeave protocol (patchset #33
id:630001 of https://codereview.chromium.org/2075313002/ )
>
>Reason for revert:
>This CL has a memory problem in the unit tests, as reported by DrMemory.
>
>First build showing the error:
>https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%29/builds/5364
>
>Note, that build doesn't include this change as there was a problem introduced
earlier that caused the whole build to fail.
>
>Error text:
>UNADDRESSABLE ACCESS of freed memory: reading 0x069fa848-0x069fa84c 4 byte(s)
># 0
>proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance
[components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator.cc:33]
># 1
>proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody
[components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator_unittest.cc:54]
># 2 >testing::internal::HandleExceptionsInMethodIfSupported<>                  
[testing\gtest\src\gtest.cc:2458]
>Note: @0:05:46.506 in thread 13092
>Note: 0x069fa848-0x069fa84c overlaps memory 0x069fa5d8-0x069fa8c0 that was
freed here:
>Note: # 0 replace_operator_delete_nothrow                                      
     [d:\drmemory_package\common\alloc_replace.c:2974]
>Note: # 1
>proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeaveClientConnectionTest_ConnectAfterADelayWhenThrottled_Test::`scalar
deleting destructor'
>Note: # 2 testing::Test::DeleteSelf_                                           
     [testing\gtest\include\gtest\gtest.h:453]
>Note: # 3 testing::TestInfo::Run                                               
     [testing\gtest\src\gtest.cc:2661]
>Note: instruction: mov    (%eax) -> %edx
>Suppression (error hash=#E743E83C1B0FB1CF#):
>For more info on using suppressions see
>http://dev.chromium.org/developers/how-tos/using-drmemory#TOC-Suppressing-error-reports-from-the-
>{
>UNADDRESSABLE ACCESS
>name=<insert_a_suppression_name_here>
>*!proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance
>*!proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody
>*!testing::internal::HandleExceptionsInMethodIfSupported<>
>}
>
>The problem is real, to fix this SetInstanceForTesting should keep the previous
factory around somewhere, and there should be a method to clear the testing
instance. Or something like that.


BUG=617238
==========

[commit-bot: I haz the power, 2016-08-03 21:04:35]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-08-03 21:06:44]

Description was changed from

==========
Revert of Revert of Substituting legacy protocol with uWeave protocol (patchset
#1 id:1 of https://codereview.chromium.org/2189913002/ )

Reason for revert:
The memory leak that caused Substituting legacy protocol with uWeave protocol to
be reverted is fixed.

First build showing the error: 
n/a

Error text:
n/a
                          
Original issue's description:
>Revert of Substituting legacy protocol with uWeave protocol (patchset #33
id:630001 of https://codereview.chromium.org/2075313002/ )
>
>Reason for revert:
>This CL has a memory problem in the unit tests, as reported by DrMemory.
>
>First build showing the error:
>https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%29/builds/5364
>
>Note, that build doesn't include this change as there was a problem introduced
earlier that caused the whole build to fail.
>
>Error text:
>UNADDRESSABLE ACCESS of freed memory: reading 0x069fa848-0x069fa84c 4 byte(s)
># 0
>proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance
[components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator.cc:33]
># 1
>proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody
[components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator_unittest.cc:54]
># 2 >testing::internal::HandleExceptionsInMethodIfSupported<>                  
[testing\gtest\src\gtest.cc:2458]
>Note: @0:05:46.506 in thread 13092
>Note: 0x069fa848-0x069fa84c overlaps memory 0x069fa5d8-0x069fa8c0 that was
freed here:
>Note: # 0 replace_operator_delete_nothrow                                      
     [d:\drmemory_package\common\alloc_replace.c:2974]
>Note: # 1
>proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeaveClientConnectionTest_ConnectAfterADelayWhenThrottled_Test::`scalar
deleting destructor'
>Note: # 2 testing::Test::DeleteSelf_                                           
     [testing\gtest\include\gtest\gtest.h:453]
>Note: # 3 testing::TestInfo::Run                                               
     [testing\gtest\src\gtest.cc:2661]
>Note: instruction: mov    (%eax) -> %edx
>Suppression (error hash=#E743E83C1B0FB1CF#):
>For more info on using suppressions see
>http://dev.chromium.org/developers/how-tos/using-drmemory#TOC-Suppressing-error-reports-from-the-
>{
>UNADDRESSABLE ACCESS
>name=<insert_a_suppression_name_here>
>*!proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance
>*!proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody
>*!testing::internal::HandleExceptionsInMethodIfSupported<>
>}
>
>The problem is real, to fix this SetInstanceForTesting should keep the previous
factory around somewhere, and there should be a method to clear the testing
instance. Or something like that.


BUG=617238
==========

to

==========
Revert of Revert of Substituting legacy protocol with uWeave protocol (patchset
#1 id:1 of https://codereview.chromium.org/2189913002/ )

Reason for revert:
The memory leak that caused Substituting legacy protocol with uWeave protocol to
be reverted is fixed.

First build showing the error:
n/a

Error text:
n/a

Original issue's description:
>Revert of Substituting legacy protocol with uWeave protocol (patchset #33
id:630001 of https://codereview.chromium.org/2075313002/ )
>
>Reason for revert:
>This CL has a memory problem in the unit tests, as reported by DrMemory.
>
>First build showing the error:
>https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%29/builds/5364
>
>Note, that build doesn't include this change as there was a problem introduced
earlier that caused the whole build to fail.
>
>Error text:
>UNADDRESSABLE ACCESS of freed memory: reading 0x069fa848-0x069fa84c 4 byte(s)
># 0
>proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance
[components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator.cc:33]
># 1
>proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody
[components\proximity_auth\ble\bluetooth_low_energy_weave_packet_generator_unittest.cc:54]
># 2 >testing::internal::HandleExceptionsInMethodIfSupported<>                  
[testing\gtest\src\gtest.cc:2458]
>Note: @0:05:46.506 in thread 13092
>Note: 0x069fa848-0x069fa84c overlaps memory 0x069fa5d8-0x069fa8c0 that was
freed here:
>Note: # 0 replace_operator_delete_nothrow                                      
     [d:\drmemory_package\common\alloc_replace.c:2974]
>Note: # 1
>proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeaveClientConnectionTest_ConnectAfterADelayWhenThrottled_Test::`scalar
deleting destructor'
>Note: # 2 testing::Test::DeleteSelf_                                           
     [testing\gtest\include\gtest\gtest.h:453]
>Note: # 3 testing::TestInfo::Run                                               
     [testing\gtest\src\gtest.cc:2661]
>Note: instruction: mov    (%eax) -> %edx
>Suppression (error hash=#E743E83C1B0FB1CF#):
>For more info on using suppressions see
>http://dev.chromium.org/developers/how-tos/using-drmemory#TOC-Suppressing-error-reports-from-the-
>{
>UNADDRESSABLE ACCESS
>name=<insert_a_suppression_name_here>
>*!proximity_auth::weave::BluetoothLowEnergyWeavePacketGenerator::Factory::NewInstance
>*!proximity_auth::weave::ProximityAuthBluetoothLowEnergyWeavePacketGeneratorTest_CreateConnectionRequestTest_Test::TestBody
>*!testing::internal::HandleExceptionsInMethodIfSupported<>
>}
>
>The problem is real, to fix this SetInstanceForTesting should keep the previous
factory around somewhere, and there should be a method to clear the testing
instance. Or something like that.

BUG=617238

Committed: https://crrev.com/2af35dd45f0a075fa0ce835302aeab0af3a4a392
Cr-Commit-Position: refs/heads/master@{#409618}
==========

[commit-bot: I haz the power, 2016-08-03 21:06:45]

Patchset 2 (id:??) landed as
https://crrev.com/2af35dd45f0a075fa0ce835302aeab0af3a4a392
Cr-Commit-Position: refs/heads/master@{#409618}