Commit b336943f authored by Kyle Horimoto's avatar Kyle Horimoto Committed by Commit Bot

[CrOS Cellular] Remove parameter from StartActivation() call.

An earlier design planned to pass a "cellular network GUID" parameter to
StartActivation(), but this is no longer the case, since activation
should be allowed even if the device has no SIM card (and thus, no
cellular network).

This CL removes the parameter; a follow-up CL adds the implementation
for this function.

Bug: 961084
Change-Id: Ie3026c2b8400f79bd317becb0fc71c5bf740bbf0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1629124
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarAzeem Arshad <azeemarshad@chromium.org>
Auto-Submit: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664147}
parent 7cfcf54f
...@@ -37,7 +37,6 @@ CellularSetupImpl::CellularSetupImpl() = default; ...@@ -37,7 +37,6 @@ CellularSetupImpl::CellularSetupImpl() = default;
CellularSetupImpl::~CellularSetupImpl() = default; CellularSetupImpl::~CellularSetupImpl() = default;
void CellularSetupImpl::StartActivation( void CellularSetupImpl::StartActivation(
const std::string& cellular_network_guid,
mojom::ActivationDelegatePtr delegate, mojom::ActivationDelegatePtr delegate,
StartActivationCallback callback) { StartActivationCallback callback) {
// TODO(khorimoto): Actually return a CarrierPortalObserver instead of // TODO(khorimoto): Actually return a CarrierPortalObserver instead of
......
...@@ -29,8 +29,7 @@ class CellularSetupImpl : public CellularSetupBase { ...@@ -29,8 +29,7 @@ class CellularSetupImpl : public CellularSetupBase {
CellularSetupImpl(); CellularSetupImpl();
// mojom::CellularSetup: // mojom::CellularSetup:
void StartActivation(const std::string& cellular_network_guid, void StartActivation(mojom::ActivationDelegatePtr delegate,
mojom::ActivationDelegatePtr delegate,
StartActivationCallback callback) override; StartActivationCallback callback) override;
DISALLOW_COPY_AND_ASSIGN(CellularSetupImpl); DISALLOW_COPY_AND_ASSIGN(CellularSetupImpl);
......
...@@ -29,7 +29,6 @@ namespace { ...@@ -29,7 +29,6 @@ namespace {
using CarrierPortalHandlerPair = using CarrierPortalHandlerPair =
std::pair<mojom::CarrierPortalHandlerPtr, FakeCarrierPortalHandler*>; std::pair<mojom::CarrierPortalHandlerPtr, FakeCarrierPortalHandler*>;
const char kTestCellularNetworkGuid[] = "testCellularNetworkGuid";
const char kTestPaymentUrl[] = "testPaymentUrl"; const char kTestPaymentUrl[] = "testPaymentUrl";
const char kTestPaymentPostData[] = "testPaymentPostData"; const char kTestPaymentPostData[] = "testPaymentPostData";
const char kTestCarrier[] = "testCarrier"; const char kTestCarrier[] = "testCarrier";
...@@ -86,7 +85,6 @@ class CellularSetupServiceTest : public testing::Test { ...@@ -86,7 +85,6 @@ class CellularSetupServiceTest : public testing::Test {
// Calls StartActivation() and returns the fake CarrierPortalHandler and its // Calls StartActivation() and returns the fake CarrierPortalHandler and its
// associated InterfacePtr. // associated InterfacePtr.
CarrierPortalHandlerPair CallStartActivation( CarrierPortalHandlerPair CallStartActivation(
const std::string& cellular_network_guid,
FakeActivationDelegate* fake_activation_delegate) { FakeActivationDelegate* fake_activation_delegate) {
std::vector<std::unique_ptr<FakeCellularSetup::StartActivationInvocation>>& std::vector<std::unique_ptr<FakeCellularSetup::StartActivationInvocation>>&
start_activation_invocations = start_activation_invocations =
...@@ -96,15 +94,13 @@ class CellularSetupServiceTest : public testing::Test { ...@@ -96,15 +94,13 @@ class CellularSetupServiceTest : public testing::Test {
// Make StartActivation() call and propagate it to the service. // Make StartActivation() call and propagate it to the service.
cellular_setup_ptr_->StartActivation( cellular_setup_ptr_->StartActivation(
cellular_network_guid, fake_activation_delegate->GenerateInterfacePtr(), fake_activation_delegate->GenerateInterfacePtr(),
base::BindOnce(&CellularSetupServiceTest::OnStartActivationResult, base::BindOnce(&CellularSetupServiceTest::OnStartActivationResult,
base::Unretained(this), run_loop.QuitClosure())); base::Unretained(this), run_loop.QuitClosure()));
cellular_setup_ptr_.FlushForTesting(); cellular_setup_ptr_.FlushForTesting();
// Verify that the call was made successfully. // Verify that the call was made successfully.
EXPECT_EQ(num_args_before_call + 1u, start_activation_invocations.size()); EXPECT_EQ(num_args_before_call + 1u, start_activation_invocations.size());
EXPECT_EQ(cellular_network_guid,
start_activation_invocations.back()->cellular_network_guid());
// Execute the callback and retrieve the returned CarrierPortalHandler. // Execute the callback and retrieve the returned CarrierPortalHandler.
FakeCarrierPortalHandler* fake_carrier_portal_observer = FakeCarrierPortalHandler* fake_carrier_portal_observer =
...@@ -215,8 +211,8 @@ class CellularSetupServiceTest : public testing::Test { ...@@ -215,8 +211,8 @@ class CellularSetupServiceTest : public testing::Test {
TEST_F(CellularSetupServiceTest, StartActivation_Success) { TEST_F(CellularSetupServiceTest, StartActivation_Success) {
auto fake_activation_delegate = std::make_unique<FakeActivationDelegate>(); auto fake_activation_delegate = std::make_unique<FakeActivationDelegate>();
CarrierPortalHandlerPair pair = CallStartActivation( CarrierPortalHandlerPair pair =
kTestCellularNetworkGuid, fake_activation_delegate.get()); CallStartActivation(fake_activation_delegate.get());
NotifyLastDelegateThatActivationStarted(fake_activation_delegate.get()); NotifyLastDelegateThatActivationStarted(fake_activation_delegate.get());
SendCarrierPortalStatusUpdate( SendCarrierPortalStatusUpdate(
...@@ -232,8 +228,8 @@ TEST_F(CellularSetupServiceTest, StartActivation_Success) { ...@@ -232,8 +228,8 @@ TEST_F(CellularSetupServiceTest, StartActivation_Success) {
TEST_F(CellularSetupServiceTest, StartActivation_PortalFailsToLoad) { TEST_F(CellularSetupServiceTest, StartActivation_PortalFailsToLoad) {
auto fake_activation_delegate = std::make_unique<FakeActivationDelegate>(); auto fake_activation_delegate = std::make_unique<FakeActivationDelegate>();
CarrierPortalHandlerPair pair = CallStartActivation( CarrierPortalHandlerPair pair =
kTestCellularNetworkGuid, fake_activation_delegate.get()); CallStartActivation(fake_activation_delegate.get());
NotifyLastDelegateThatActivationStarted(fake_activation_delegate.get()); NotifyLastDelegateThatActivationStarted(fake_activation_delegate.get());
SendCarrierPortalStatusUpdate(mojom::CarrierPortalStatus::kPortalFailedToLoad, SendCarrierPortalStatusUpdate(mojom::CarrierPortalStatus::kPortalFailedToLoad,
...@@ -247,8 +243,8 @@ TEST_F(CellularSetupServiceTest, StartActivation_PortalFailsToLoad) { ...@@ -247,8 +243,8 @@ TEST_F(CellularSetupServiceTest, StartActivation_PortalFailsToLoad) {
TEST_F(CellularSetupServiceTest, StartActivation_ErrorDuringPayment) { TEST_F(CellularSetupServiceTest, StartActivation_ErrorDuringPayment) {
auto fake_activation_delegate = std::make_unique<FakeActivationDelegate>(); auto fake_activation_delegate = std::make_unique<FakeActivationDelegate>();
CarrierPortalHandlerPair pair = CallStartActivation( CarrierPortalHandlerPair pair =
kTestCellularNetworkGuid, fake_activation_delegate.get()); CallStartActivation(fake_activation_delegate.get());
NotifyLastDelegateThatActivationStarted(fake_activation_delegate.get()); NotifyLastDelegateThatActivationStarted(fake_activation_delegate.get());
SendCarrierPortalStatusUpdate( SendCarrierPortalStatusUpdate(
......
...@@ -13,11 +13,9 @@ namespace chromeos { ...@@ -13,11 +13,9 @@ namespace chromeos {
namespace cellular_setup { namespace cellular_setup {
FakeCellularSetup::StartActivationInvocation::StartActivationInvocation( FakeCellularSetup::StartActivationInvocation::StartActivationInvocation(
const std::string& cellular_network_guid,
mojom::ActivationDelegatePtr activation_delegate, mojom::ActivationDelegatePtr activation_delegate,
StartActivationCallback callback) StartActivationCallback callback)
: cellular_network_guid_(cellular_network_guid), : activation_delegate_(std::move(activation_delegate)),
activation_delegate_(std::move(activation_delegate)),
callback_(std::move(callback)) {} callback_(std::move(callback)) {}
FakeCellularSetup::StartActivationInvocation::~StartActivationInvocation() = FakeCellularSetup::StartActivationInvocation::~StartActivationInvocation() =
...@@ -40,7 +38,6 @@ FakeCellularSetup::FakeCellularSetup() = default; ...@@ -40,7 +38,6 @@ FakeCellularSetup::FakeCellularSetup() = default;
FakeCellularSetup::~FakeCellularSetup() = default; FakeCellularSetup::~FakeCellularSetup() = default;
void FakeCellularSetup::StartActivation( void FakeCellularSetup::StartActivation(
const std::string& cellular_network_guid,
mojom::ActivationDelegatePtr activation_delegate, mojom::ActivationDelegatePtr activation_delegate,
StartActivationCallback callback) { StartActivationCallback callback) {
DCHECK(activation_delegate); DCHECK(activation_delegate);
...@@ -48,8 +45,7 @@ void FakeCellularSetup::StartActivation( ...@@ -48,8 +45,7 @@ void FakeCellularSetup::StartActivation(
start_activation_invocations_.emplace_back( start_activation_invocations_.emplace_back(
std::make_unique<StartActivationInvocation>( std::make_unique<StartActivationInvocation>(
cellular_network_guid, std::move(activation_delegate), std::move(activation_delegate), std::move(callback)));
std::move(callback)));
} }
} // namespace cellular_setup } // namespace cellular_setup
......
...@@ -23,15 +23,10 @@ class FakeCellularSetup : public CellularSetupBase { ...@@ -23,15 +23,10 @@ class FakeCellularSetup : public CellularSetupBase {
public: public:
class StartActivationInvocation { class StartActivationInvocation {
public: public:
StartActivationInvocation(const std::string& cellular_network_guid, StartActivationInvocation(mojom::ActivationDelegatePtr activation_delegate,
mojom::ActivationDelegatePtr activation_delegate,
StartActivationCallback callback); StartActivationCallback callback);
~StartActivationInvocation(); ~StartActivationInvocation();
const std::string& cellular_network_guid() const {
return cellular_network_guid_;
}
mojom::ActivationDelegatePtr& activation_delegate() { mojom::ActivationDelegatePtr& activation_delegate() {
return activation_delegate_; return activation_delegate_;
} }
...@@ -42,7 +37,6 @@ class FakeCellularSetup : public CellularSetupBase { ...@@ -42,7 +37,6 @@ class FakeCellularSetup : public CellularSetupBase {
FakeCarrierPortalHandler* ExecuteCallback(); FakeCarrierPortalHandler* ExecuteCallback();
private: private:
std::string cellular_network_guid_;
mojom::ActivationDelegatePtr activation_delegate_; mojom::ActivationDelegatePtr activation_delegate_;
StartActivationCallback callback_; StartActivationCallback callback_;
...@@ -62,8 +56,7 @@ class FakeCellularSetup : public CellularSetupBase { ...@@ -62,8 +56,7 @@ class FakeCellularSetup : public CellularSetupBase {
private: private:
// mojom::CellularSetup: // mojom::CellularSetup:
void StartActivation(const std::string& cellular_network_guid, void StartActivation(mojom::ActivationDelegatePtr activation_delegate,
mojom::ActivationDelegatePtr activation_delegate,
StartActivationCallback callback) override; StartActivationCallback callback) override;
std::vector<std::unique_ptr<StartActivationInvocation>> std::vector<std::unique_ptr<StartActivationInvocation>>
......
...@@ -80,12 +80,8 @@ interface ActivationDelegate { ...@@ -80,12 +80,8 @@ interface ActivationDelegate {
// Interface used to set up a cellular connection. // Interface used to set up a cellular connection.
interface CellularSetup { interface CellularSetup {
// Entrypoint to the activation flow. |cellular_network_guid| corresponds to // Entrypoint to the activation flow. If |delegate| becomes disconnected
// the GUID associated with the cellular network, obtained via
// chromeos::NetworkState::guid(). |delegate|'s OnActivationFinished()
// function is guaranteed to be called. If |delegate| becomes disconnected
// during the activation process, activation is cancelled. // during the activation process, activation is cancelled.
StartActivation( StartActivation(ActivationDelegate delegate)
string cellular_network_guid, => (CarrierPortalHandler observer);
ActivationDelegate delegate) => (CarrierPortalHandler observer);
}; };
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment