Set the user agent for 0.6

blimp/client/core/settings/settings_feature.cc

Index: blimp/client/core/settings/settings_feature.cc
diff --git a/blimp/client/core/settings/settings_feature.cc b/blimp/client/core/settings/settings_feature.cc
index e669415cca02fa1e8b321996ac588e7d404ede3e..21d57f7500948985aa7ad7c0c7631ae9769ad5b4 100644
--- a/blimp/client/core/settings/settings_feature.cc
+++ b/blimp/client/core/settings/settings_feature.cc
@@ -5,6 +5,7 @@
 #include "blimp/client/core/settings/settings_feature.h"
 
 #include "blimp/client/core/settings/settings.h"
+#include "blimp/client/core/settings/user_agent.h"
 #include "blimp/common/create_blimp_message.h"
 #include "blimp/common/proto/blimp_message.pb.h"
 #include "blimp/common/proto/settings.pb.h"
@@ -22,16 +23,6 @@ void SettingsFeature::set_outgoing_message_processor(
   outgoing_message_processor_ = std::move(processor);
 }
 
-// TODO(mlliu): remove this method once we set the user agent in PushSettings()
-// http://crbug.com/652032.
-void SettingsFeature::SendUserAgentOSVersionInfo(const std::string& osVersion) {
-  EngineSettingsMessage* engine_settings;
-  std::unique_ptr<BlimpMessage> message = CreateBlimpMessage(&engine_settings);
-  engine_settings->set_client_os_info(osVersion);
-  outgoing_message_processor_->ProcessMessage(std::move(message),
-                                              net::CompletionCallback());
-}
-
 void SettingsFeature::ProcessMessage(std::unique_ptr<BlimpMessage> message,
                                      const net::CompletionCallback& callback) {
   // We don't receive any messages from the engine yet.
@@ -48,12 +39,13 @@ void SettingsFeature::OnRecordWholeDocumentChanged(bool enable) {
 }
 
 void SettingsFeature::PushSettings() {
-  // TODO(mlliu): set the user agent on the proto as well after moving
-  // blimp/client/app/user_agent.* to this directory (http://crbug.com/652032).
   EngineSettingsMessage* engine_settings;
   std::unique_ptr<BlimpMessage> message = CreateBlimpMessage(&engine_settings);
-  engine_settings->set_record_whole_document(
-      settings_->IsRecordWholeDocument());
+  if (settings_) {

[Khushal, 2016/11/01 21:28:55]

Why the if check? Isn't settings_ supposed to outlive this class?

[Menglin, 2016/11/01 21:37:52]

it's for BlimpClientSession. because in BlimpClientSession
https://cs.chromium.org/chromium/src/blimp/client/app/session/blimp_client_se...,
a nullptr is passed as the settings when constructing the settings_feature_ (we
don't care about the settings object there).

so when BlimpClientSessionAndroid calls GetSettingsFeature()->PushSettings(), it
should essentially only sends the client os info.

[Khushal, 2016/11/01 23:33:06]

Hmmm, I just looked at that class and it seems like the rest of the settings
don't even work anymore. :/ Like record whole document is no longer read and
set.

Would it be possible to build a Settings object from the app? For the
PrefService, you can do it the same way we do in
BlimpBrowserContext::InitPrefService and use a PrefRegisterySimple.

+    engine_settings->set_record_whole_document(
+        settings_->IsRecordWholeDocument());
+  }
+  engine_settings->set_client_os_info(GetOSVersionInfoForUserAgent());
   outgoing_message_processor_->ProcessMessage(std::move(message),
                                               net::CompletionCallback());
 }