Migrate ProximityAuth to the uWeave BLE protocol |
||||
Issue descriptionUserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.63 Safari/537.36 Steps to reproduce the problem: 1. Create packet generator and receiver using the uWeave protocol. 2. Submit a new client that communicate to the server with uWeave protocol 3. Swap out the legacy client with the new one What is the expected behavior? What went wrong? Need to migrate the current application specific protocol to a common protocol Did this work before? N/A Chrome version: 51.0.2704.63 Channel: n/a OS Version: Flash Version: Shockwave Flash 21.0 r0
,
Jun 9 2016
,
Jun 15 2016
,
Jun 24 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6742a47fff173bfa1bf0a4ec3a4655436c9c03e4 commit 6742a47fff173bfa1bf0a4ec3a4655436c9c03e4 Author: jingxuy <jingxuy@google.com> Date: Fri Jun 24 22:07:06 2016 Weave Packet Generator This CL adds a packet generator that respects the uWeave.BLE protocol. The packet generator accepts raw bytes as messages to be sent and wraps the message with the uWeave protocol. The uWeave packets include control packets of request/response/close of a connection and data packets that are the original message chopped up to fixed size packets with uWeave protocol headers. Design Doc of ProximityAuth uWeave Migration: https://docs.google.com/a/google.com/document/d/1HJ1fipxsNQwIxmKs-5_6TjroCAcs6qRhAYvOnIr6mQw/edit?usp=sharing BUG= 617238 Review-Url: https://codereview.chromium.org/2031953003 Cr-Commit-Position: refs/heads/master@{#401993} [modify] https://crrev.com/6742a47fff173bfa1bf0a4ec3a4655436c9c03e4/components/components_tests.gyp [modify] https://crrev.com/6742a47fff173bfa1bf0a4ec3a4655436c9c03e4/components/proximity_auth.gypi [modify] https://crrev.com/6742a47fff173bfa1bf0a4ec3a4655436c9c03e4/components/proximity_auth/ble/BUILD.gn [add] https://crrev.com/6742a47fff173bfa1bf0a4ec3a4655436c9c03e4/components/proximity_auth/ble/bluetooth_low_energy_weave_packet_generator.cc [add] https://crrev.com/6742a47fff173bfa1bf0a4ec3a4655436c9c03e4/components/proximity_auth/ble/bluetooth_low_energy_weave_packet_generator.h [add] https://crrev.com/6742a47fff173bfa1bf0a4ec3a4655436c9c03e4/components/proximity_auth/ble/bluetooth_low_energy_weave_packet_generator_unittest.cc
,
Jun 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/714da71c4b3b8a763ed61b00ee7f8efa83cc0347 commit 714da71c4b3b8a763ed61b00ee7f8efa83cc0347 Author: jingxuy <jingxuy@google.com> Date: Tue Jun 28 01:04:55 2016 Move weave packet to common location b/t generator and receiver BUG= 617238 Review-Url: https://codereview.chromium.org/2096103003 Cr-Commit-Position: refs/heads/master@{#402355} [modify] https://crrev.com/714da71c4b3b8a763ed61b00ee7f8efa83cc0347/components/proximity_auth/ble/BUILD.gn [add] https://crrev.com/714da71c4b3b8a763ed61b00ee7f8efa83cc0347/components/proximity_auth/ble/bluetooth_low_energy_weave_defines.h [modify] https://crrev.com/714da71c4b3b8a763ed61b00ee7f8efa83cc0347/components/proximity_auth/ble/bluetooth_low_energy_weave_packet_generator.cc [modify] https://crrev.com/714da71c4b3b8a763ed61b00ee7f8efa83cc0347/components/proximity_auth/ble/bluetooth_low_energy_weave_packet_generator.h [modify] https://crrev.com/714da71c4b3b8a763ed61b00ee7f8efa83cc0347/components/proximity_auth/ble/bluetooth_low_energy_weave_packet_generator_unittest.cc
,
Jun 29 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/15d29c37855d219d1d015a01639a3ac9c00fda1c commit 15d29c37855d219d1d015a01639a3ac9c00fda1c Author: jingxuy <jingxuy@google.com> Date: Wed Jun 29 02:26:51 2016 Current generator's functions are not virtual and doesn't allow the subclass to have polymorphism for the mocks in the test. This CL makes those functions virtual and overridable. BUG= 617238 Review-Url: https://codereview.chromium.org/2105103002 Cr-Commit-Position: refs/heads/master@{#402673} [modify] https://crrev.com/15d29c37855d219d1d015a01639a3ac9c00fda1c/components/proximity_auth/ble/bluetooth_low_energy_weave_packet_generator.h
,
Jun 29 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/688334563b18b7d80b3207da6c0d30d17323b351 commit 688334563b18b7d80b3207da6c0d30d17323b351 Author: jingxuy <jingxuy@google.com> Date: Wed Jun 29 18:58:28 2016 Weave Packet Generator This CL adds a packet receiver that respects the uWeave.BLE protocol. The packet receiver expects this chain of events: 1 control_request/response -> 0+ data packets -> 0 or 1 control_close. The packet receiver keeps track of the receiving state and will jump to ERROR state when anything unexpected happens. (receiving packets out of order, invalid packet content, etc) The expected user of packet receiver should feed a packet into receiver, check the state, and take appropriate action. If actions are not taken in the appropriate state, the receiver will crash. Data message should be retrieved promptly when the state reaches DATA_READY or else the data will be lost when the next packet arrives. Design Doc for ProximityAuth uWeave Migration: https://docs.google.com/a/google.com/document/d/1HJ1fipxsNQwIxmKs-5_6TjroCAcs6qRhAYvOnIr6mQw/edit?usp=sharing BUG= 617238 Review-Url: https://codereview.chromium.org/2053013002 Cr-Commit-Position: refs/heads/master@{#402884} [modify] https://crrev.com/688334563b18b7d80b3207da6c0d30d17323b351/components/components_tests.gyp [modify] https://crrev.com/688334563b18b7d80b3207da6c0d30d17323b351/components/proximity_auth.gypi [modify] https://crrev.com/688334563b18b7d80b3207da6c0d30d17323b351/components/proximity_auth/ble/BUILD.gn [add] https://crrev.com/688334563b18b7d80b3207da6c0d30d17323b351/components/proximity_auth/ble/bluetooth_low_energy_weave_packet_receiver.cc [add] https://crrev.com/688334563b18b7d80b3207da6c0d30d17323b351/components/proximity_auth/ble/bluetooth_low_energy_weave_packet_receiver.h [add] https://crrev.com/688334563b18b7d80b3207da6c0d30d17323b351/components/proximity_auth/ble/bluetooth_low_energy_weave_packet_receiver_unittest.cc
,
Jul 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f4f0a5cde8eff464de5359281560444ff442f920 commit f4f0a5cde8eff464de5359281560444ff442f920 Author: jingxuy <jingxuy@google.com> Date: Tue Jul 26 00:42:49 2016 Using the uWeave packet generator and receiver to re-implement the BluetoothLowEnergyConnection class Major changes: SendMesssageImpl which instead of using the old protocol, it will just generate the message with the WeavePacketGenerator. GattCharacteristicValueChanged instead of parsing messages received with the old protocol, will now use the WeavePacketReceiver and just switch on the state of the receiver and take appropriate action. Design Doc for ProximityAuth uWeave Migration: https://docs.google.com/a/google.com/document/d/1HJ1fipxsNQwIxmKs-5_6TjroCAcs6qRhAYvOnIr6mQw/edit?usp=sharing BUG= 617238 Review-Url: https://codereview.chromium.org/2075313002 Cr-Commit-Position: refs/heads/master@{#407655} [modify] https://crrev.com/f4f0a5cde8eff464de5359281560444ff442f920/components/components_tests.gyp [modify] https://crrev.com/f4f0a5cde8eff464de5359281560444ff442f920/components/proximity_auth.gypi [modify] https://crrev.com/f4f0a5cde8eff464de5359281560444ff442f920/components/proximity_auth/ble/BUILD.gn [add] https://crrev.com/f4f0a5cde8eff464de5359281560444ff442f920/components/proximity_auth/ble/bluetooth_low_energy_weave_client_connection.cc [add] https://crrev.com/f4f0a5cde8eff464de5359281560444ff442f920/components/proximity_auth/ble/bluetooth_low_energy_weave_client_connection.h [add] https://crrev.com/f4f0a5cde8eff464de5359281560444ff442f920/components/proximity_auth/ble/bluetooth_low_energy_weave_client_connection_unittest.cc
,
Jul 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/932d272591d6c54513a837f70d0ebb182f5cc19c commit 932d272591d6c54513a837f70d0ebb182f5cc19c Author: benwells <benwells@chromium.org> Date: Thu Jul 28 07:04:42 2016 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. Original issue's description: > Using the uWeave packet generator and receiver to re-implement the BluetoothLowEnergyConnection class > > Major changes: > SendMesssageImpl which instead of using the old protocol, it will just generate the message with the WeavePacketGenerator. > > GattCharacteristicValueChanged instead of parsing messages received with the old protocol, will now use the WeavePacketReceiver and just switch on the state of the receiver and take appropriate action. > > Design Doc for ProximityAuth uWeave Migration: > https://docs.google.com/a/google.com/document/d/1HJ1fipxsNQwIxmKs-5_6TjroCAcs6qRhAYvOnIr6mQw/edit?usp=sharing > > BUG= 617238 > > Committed: https://crrev.com/f4f0a5cde8eff464de5359281560444ff442f920 > Cr-Commit-Position: refs/heads/master@{#407655} TBR=khorimoto@chromium.org,tengs@chromium.org,sacomoto@chromium.org,msarda@chromium.org,jingxuy@google.com # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 617238 Review-Url: https://codereview.chromium.org/2189913002 Cr-Commit-Position: refs/heads/master@{#408351} [modify] https://crrev.com/932d272591d6c54513a837f70d0ebb182f5cc19c/components/components_tests.gyp [modify] https://crrev.com/932d272591d6c54513a837f70d0ebb182f5cc19c/components/proximity_auth.gypi [modify] https://crrev.com/932d272591d6c54513a837f70d0ebb182f5cc19c/components/proximity_auth/ble/BUILD.gn [delete] https://crrev.com/d8b8acff3d2d57e66b833a5632e3a993eb405277/components/proximity_auth/ble/bluetooth_low_energy_weave_client_connection.cc [delete] https://crrev.com/d8b8acff3d2d57e66b833a5632e3a993eb405277/components/proximity_auth/ble/bluetooth_low_energy_weave_client_connection.h [delete] https://crrev.com/d8b8acff3d2d57e66b833a5632e3a993eb405277/components/proximity_auth/ble/bluetooth_low_energy_weave_client_connection_unittest.cc
,
Aug 3 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2af35dd45f0a075fa0ce835302aeab0af3a4a392 commit 2af35dd45f0a075fa0ce835302aeab0af3a4a392 Author: jingxuy <jingxuy@google.com> Date: Wed Aug 03 21:04:14 2016 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 Review-Url: https://codereview.chromium.org/2188053003 Cr-Commit-Position: refs/heads/master@{#409618} [modify] https://crrev.com/2af35dd45f0a075fa0ce835302aeab0af3a4a392/components/components_tests.gyp [modify] https://crrev.com/2af35dd45f0a075fa0ce835302aeab0af3a4a392/components/proximity_auth.gypi [modify] https://crrev.com/2af35dd45f0a075fa0ce835302aeab0af3a4a392/components/proximity_auth/ble/BUILD.gn [add] https://crrev.com/2af35dd45f0a075fa0ce835302aeab0af3a4a392/components/proximity_auth/ble/bluetooth_low_energy_weave_client_connection.cc [add] https://crrev.com/2af35dd45f0a075fa0ce835302aeab0af3a4a392/components/proximity_auth/ble/bluetooth_low_energy_weave_client_connection.h [add] https://crrev.com/2af35dd45f0a075fa0ce835302aeab0af3a4a392/components/proximity_auth/ble/bluetooth_low_energy_weave_client_connection_unittest.cc [modify] https://crrev.com/2af35dd45f0a075fa0ce835302aeab0af3a4a392/components/proximity_auth/ble/bluetooth_low_energy_weave_packet_generator.cc [modify] https://crrev.com/2af35dd45f0a075fa0ce835302aeab0af3a4a392/components/proximity_auth/ble/bluetooth_low_energy_weave_packet_generator.h [modify] https://crrev.com/2af35dd45f0a075fa0ce835302aeab0af3a4a392/components/proximity_auth/ble/bluetooth_low_energy_weave_packet_receiver.cc [modify] https://crrev.com/2af35dd45f0a075fa0ce835302aeab0af3a4a392/components/proximity_auth/ble/bluetooth_low_energy_weave_packet_receiver.h
,
Jan 10
Archiving issues older than 2 years with no owner or component. |
||||
►
Sign in to add a comment |
||||
Comment 1 by sheriffbot@chromium.org
, Jun 4 2016