Fix URL duplication when SetArgsForURL is called mutliple times.
R=ppi@chromium.org
Review URL: https://codereview.chromium.org/1467263002 .
diff --git a/shell/application_manager/application_manager.cc b/shell/application_manager/application_manager.cc
index 4f4af48..8453f1c 100644
--- a/shell/application_manager/application_manager.cc
+++ b/shell/application_manager/application_manager.cc
@@ -45,6 +45,14 @@
return result;
}
+void AppendArgsForURL(const GURL& url,
+ const std::vector<std::string>& args,
+ std::vector<std::string>* target) {
+ if (target->empty())
+ target->push_back(url.spec());
+ target->insert(target->end(), args.begin(), args.end());
+}
+
} // namespace
class ApplicationManager::ContentHandlerConnection {
@@ -457,20 +465,15 @@
void ApplicationManager::SetArgsForURL(const std::vector<std::string>& args,
const GURL& url) {
GURL base_url = GetBaseURLAndQuery(url, nullptr);
- url_to_args_[base_url].insert(url_to_args_[base_url].end(), args.begin(),
- args.end());
+ AppendArgsForURL(base_url, args, &url_to_args_[base_url]);
GURL mapped_url = delegate_->ResolveMappings(base_url);
DCHECK(!mapped_url.has_query());
- if (mapped_url != url) {
- url_to_args_[mapped_url].insert(url_to_args_[mapped_url].end(),
- args.begin(), args.end());
- }
+ if (mapped_url != base_url)
+ AppendArgsForURL(mapped_url, args, &url_to_args_[mapped_url]);
GURL resolved_url = delegate_->ResolveMojoURL(mapped_url);
DCHECK(!resolved_url.has_query());
- if (resolved_url != mapped_url) {
- url_to_args_[resolved_url].insert(url_to_args_[resolved_url].end(),
- args.begin(), args.end());
- }
+ if (resolved_url != mapped_url)
+ AppendArgsForURL(resolved_url, args, &url_to_args_[resolved_url]);
}
NativeApplicationOptions* ApplicationManager::GetNativeApplicationOptionsForURL(
diff --git a/shell/application_manager/application_manager_unittest.cc b/shell/application_manager/application_manager_unittest.cc
index 341aea1..de6fdc5 100644
--- a/shell/application_manager/application_manager_unittest.cc
+++ b/shell/application_manager/application_manager_unittest.cc
@@ -522,9 +522,9 @@
test_client.Test("test");
loop_.Run();
std::vector<std::string> app_args = loader->GetArgs();
- ASSERT_EQ(args.size(), app_args.size());
- EXPECT_EQ(args[0], app_args[0]);
- EXPECT_EQ(args[1], app_args[1]);
+ ASSERT_EQ(args.size() + 1, app_args.size());
+ EXPECT_EQ(args[0], app_args[1]);
+ EXPECT_EQ(args[1], app_args[2]);
}
// Confirm that arguments are sent to an application in the presence of query
@@ -545,8 +545,33 @@
test_client.Test("test");
loop_.Run();
std::vector<std::string> app_args = loader->GetArgs();
- ASSERT_EQ(args.size(), app_args.size());
- EXPECT_EQ(args[0], app_args[0]);
+ ASSERT_EQ(args.size() + 1, app_args.size());
+ EXPECT_EQ(args[0], app_args[1]);
+}
+
+// Confirm that the URL is not duplicated when arguments are added in multiple
+// phases.
+TEST_F(ApplicationManagerTest, ArgsMultipleCalls) {
+ ApplicationManager am(ApplicationManager::Options(), &test_delegate_);
+ GURL test_url("test:test");
+ std::vector<std::string> args1;
+ args1.push_back("test_arg1");
+ am.SetArgsForURL(args1, test_url);
+ std::vector<std::string> args2;
+ args2.push_back("test_arg2");
+ am.SetArgsForURL(args2, test_url);
+ TestApplicationLoader* loader = new TestApplicationLoader;
+ loader->set_context(&context_);
+ am.SetLoaderForURL(scoped_ptr<ApplicationLoader>(loader), test_url);
+ TestServicePtr test_service;
+ am.ConnectToService(test_url, &test_service);
+ TestClient test_client(test_service.Pass());
+ test_client.Test("test");
+ loop_.Run();
+ std::vector<std::string> app_args = loader->GetArgs();
+ ASSERT_EQ(args1.size() + args2.size() + 1, app_args.size());
+ EXPECT_EQ(args1[0], app_args[1]);
+ EXPECT_EQ(args2[0], app_args[2]);
}
// Confirm that arguments are aggregated through mappings.
@@ -574,11 +599,11 @@
test_client.Test("test");
loop_.Run();
std::vector<std::string> app_args = loader->GetArgs();
- ASSERT_EQ(args.size() + args2.size(), app_args.size());
- EXPECT_EQ(args[0], app_args[0]);
- EXPECT_EQ(args[1], app_args[1]);
- EXPECT_EQ(args2[0], app_args[2]);
- EXPECT_EQ(args2[1], app_args[3]);
+ ASSERT_EQ(args.size() + args2.size() + 1, app_args.size());
+ EXPECT_EQ(args[0], app_args[1]);
+ EXPECT_EQ(args[1], app_args[2]);
+ EXPECT_EQ(args2[0], app_args[3]);
+ EXPECT_EQ(args2[1], app_args[4]);
}
{
// Connext to the target url
@@ -588,11 +613,11 @@
test_client.Test("test");
loop_.Run();
std::vector<std::string> app_args = loader->GetArgs();
- ASSERT_EQ(args.size() + args2.size(), app_args.size());
- EXPECT_EQ(args[0], app_args[0]);
- EXPECT_EQ(args[1], app_args[1]);
- EXPECT_EQ(args2[0], app_args[2]);
- EXPECT_EQ(args2[1], app_args[3]);
+ ASSERT_EQ(args.size() + args2.size() + 1, app_args.size());
+ EXPECT_EQ(args[0], app_args[1]);
+ EXPECT_EQ(args[1], app_args[2]);
+ EXPECT_EQ(args2[0], app_args[3]);
+ EXPECT_EQ(args2[1], app_args[4]);
}
}
diff --git a/shell/command_line_util.cc b/shell/command_line_util.cc
index 10d7a1e..dfa2986 100644
--- a/shell/command_line_util.cc
+++ b/shell/command_line_util.cc
@@ -21,7 +21,7 @@
std::vector<std::string> args;
GURL app_url = GetAppURLAndArgs(context, app_url_and_args, &args);
- if (args.size() > 1)
+ if (!args.empty())
context->application_manager()->SetArgsForURL(args, app_url);
return app_url;
}
@@ -58,8 +58,7 @@
LOG(ERROR) << "Error: invalid URL: " << (*args)[0];
return app_url;
}
- if (args->size() == 1)
- args->clear();
+ args->erase(args->begin());
return app_url;
}
diff --git a/shell/command_line_util_unittest.cc b/shell/command_line_util_unittest.cc
index d676155..58d7649 100644
--- a/shell/command_line_util_unittest.cc
+++ b/shell/command_line_util_unittest.cc
@@ -67,8 +67,6 @@
EXPECT_EQ(GURL(expectation.url), result);
std::vector<std::string> expected_args;
if (expectation.values) {
- if (*expectation.values)
- expected_args.push_back(expectation.url);
for (const char** value = expectation.values; *value; ++value)
expected_args.push_back(*value);
}
