Modified components ui to address concern of all the time disabled check update button.

chrome/browser/ui/webui/components_ui.cc

Index: chrome/browser/ui/webui/components_ui.cc
diff --git a/chrome/browser/ui/webui/components_ui.cc b/chrome/browser/ui/webui/components_ui.cc
index e5cce27f49a3874dd03e8144820f3d736545d4e2..5c68d2e946a75f5efc483d56f1f405a672a848c7 100644
--- a/chrome/browser/ui/webui/components_ui.cc
+++ b/chrome/browser/ui/webui/components_ui.cc
@@ -11,6 +11,7 @@
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/component_updater/component_updater_service.h"
+#include "chrome/browser/component_updater/crx_update_item.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/common/chrome_paths.h"
@@ -75,8 +76,6 @@ class ComponentsDOMHandler : public WebUIMessageHandler {
   void HandleCheckUpdate(const base::ListValue* args);
 
  private:
-  void LoadComponents();
-
   content::NotificationRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(ComponentsDOMHandler);
@@ -86,18 +85,23 @@ ComponentsDOMHandler::ComponentsDOMHandler() {
 }
 
 void ComponentsDOMHandler::RegisterMessages() {
-  web_ui()->RegisterMessageCallback("requestComponentsData",
+  web_ui()->RegisterMessageCallback(
+      "requestComponentsData",
       base::Bind(&ComponentsDOMHandler::HandleRequestComponentsData,
                  base::Unretained(this)));
 
-  web_ui()->RegisterMessageCallback("checkUpdate",
+  web_ui()->RegisterMessageCallback(
+      "checkUpdate",
       base::Bind(&ComponentsDOMHandler::HandleCheckUpdate,
                  base::Unretained(this)));
 }
 
 void ComponentsDOMHandler::HandleRequestComponentsData(
     const base::ListValue* args) {
-  LoadComponents();
+  base::ListValue* list = ComponentsUI::LoadComponents();

[Sorin Jianu, 2014/05/08 22:07:24]

Will the memory for |list| be correctly deallocated in this case.

[Shrikant Kelkar, 2014/05/15 23:12:43]

Originally I also was concerned about this, but it appears that once you attach
this thing to DictionaryValue object, it deletes all that's attached and their
contents during time of destruction.

+  base::DictionaryValue result;
+  result.Set("components", list);
+  web_ui()->CallJavascriptFunction("returnComponentsData", result);
 }
 
 // This function is called when user presses button from html UI.
@@ -119,30 +123,6 @@ void ComponentsDOMHandler::HandleCheckUpdate(const base::ListValue* args) {
   ComponentsUI::OnDemandUpdate(component_id);
 }
 
-void ComponentsDOMHandler::LoadComponents() {
-  component_updater::ComponentUpdateService* cus =
-      g_browser_process->component_updater();
-  std::vector<component_updater::CrxComponentInfo> components;
-  cus->GetComponents(&components);
-
-  // Construct DictionaryValues to return to UI.
-  base::ListValue* component_list = new base::ListValue();
-  for (size_t j = 0; j < components.size(); ++j) {
-    const component_updater::CrxComponentInfo& component = components[j];
-
-    base::DictionaryValue* component_entry = new base::DictionaryValue();
-    component_entry->SetString("id", component.id);
-    component_entry->SetString("name", component.name);
-    component_entry->SetString("version", component.version);
-
-    component_list->Append(component_entry);
-  }
-
-  base::DictionaryValue results;
-  results.Set("components", component_list);
-  web_ui()->CallJavascriptFunction("returnComponentsData", results);
-}
-
 }  // namespace
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -157,6 +137,16 @@ ComponentsUI::ComponentsUI(content::WebUI* web_ui) : WebUIController(web_ui) {
   // Set up the chrome://components/ source.
   Profile* profile = Profile::FromWebUI(web_ui);
   content::WebUIDataSource::Add(profile, CreateComponentsUIHTMLSource(profile));
+  component_updater::ComponentUpdateService* cus =
+      g_browser_process->component_updater();
+  cus->AddObserver(this);
+}
+
+ComponentsUI::~ComponentsUI() {
+  component_updater::ComponentUpdateService* cus =
+      g_browser_process->component_updater();
+  if (cus)
+    cus->RemoveObserver(this);
 }
 
 // static
@@ -167,8 +157,103 @@ void ComponentsUI::OnDemandUpdate(const std::string& component_id) {
 }
 
 // static
+base::ListValue* ComponentsUI::LoadComponents() {
+  component_updater::ComponentUpdateService* cus =
+      g_browser_process->component_updater();
+  std::vector<std::string> component_ids;
+  component_ids = cus->GetComponentIDs();
+
+  // Construct DictionaryValues to return to UI.
+  base::ListValue* component_list = new base::ListValue();

[Sorin Jianu, 2014/05/08 22:07:24]

Who owns this memory?

[Shrikant Kelkar, 2014/05/15 23:12:43]

This instance is returned to caller, who attaches it to DictionaryValue. For how
DictionaryValue destroys contents please please check it @
https://code.google.com/p/chromium/codesearch#chromium/src/base/values.cc&rcl...

+  for (size_t j = 0; j < component_ids.size(); ++j) {
+    component_updater::CrxUpdateItem* item = cus->GetComponentDetails(

[Sorin Jianu, 2014/05/08 22:07:24]

This |item| could be const. Also, the result of calling cus->GetComponentDetails
can be NULL if the component is not found (most likely because of a bug at this
moment). One idea (it depends on personal taste) is to code this sitution
defensively and not crash.

[Shrikant Kelkar, 2014/05/15 23:12:43]

Done.

+                                                component_ids[j]);
+
+    base::DictionaryValue* component_entry = new base::DictionaryValue();

[Sorin Jianu, 2014/05/08 22:07:24]

Who owns this memory allocation?

[Shrikant Kelkar, 2014/05/15 23:12:43]

component_list is owner here.

+    component_entry->SetString("id", component_ids[j]);
+    component_entry->SetString("name", item->component.name);
+    component_entry->SetString("version", item->component.version.GetString());
+
+    component_entry->SetString("status",
+                               ServiceStatusToString(item->status).c_str());
+
+    component_list->Append(component_entry);
+  }
+
+  return component_list;
+}
+
+// static
 base::RefCountedMemory* ComponentsUI::GetFaviconResourceBytes(
       ui::ScaleFactor scale_factor) {
   return ResourceBundle::GetSharedInstance().
       LoadDataResourceBytesForScale(IDR_PLUGINS_FAVICON, scale_factor);
 }
+
+void ComponentsUI::ComponentEventToString(Events event,
+                                          std::string* converted_event) {

[Sorin Jianu, 2014/05/08 22:07:24]

Consider renaming the parameter to |event_string| or something similar.

[Shrikant Kelkar, 2014/05/15 23:12:43]

Done.

+  DCHECK(converted_event != NULL);
+  switch (event) {
+    case COMPONENT_UPDATER_STARTED:
+      *converted_event = "UpdaterStarted";
+      return;
+    case COMPONENT_UPDATER_SLEEPING:
+      *converted_event = "UpdaterSleeping";
+      return;
+    case COMPONENT_UPDATE_FOUND:
+      *converted_event = "UpdateFound";
+      return;
+    case COMPONENT_UPDATE_READY:
+      *converted_event = "UpdateReady";
+      return;
+    case COMPONENT_UPDATED:
+      *converted_event = "ComponentUpdated";
+      return;
+    case COMPONENT_NOT_UPDATED:
+      *converted_event = "ComponentNotUpdated";
+      return;
+    default:
+      *converted_event = "Unknown";
+      return;
+  }
+}
+
+std::string ComponentsUI::ServiceStatusToString(
+    component_updater::CrxUpdateItem::Status status) {
+  switch (status) {
+    case component_updater::CrxUpdateItem::kNew:
+      return "new";
+    case component_updater::CrxUpdateItem::kChecking:
+      return "checking";
+    case component_updater::CrxUpdateItem::kCanUpdate:
+      return "update";
+    case component_updater::CrxUpdateItem::kDownloadingDiff:
+      return "downloading_diff";
+    case component_updater::CrxUpdateItem::kDownloading:
+      return "downloading";
+    case component_updater::CrxUpdateItem::kUpdatingDiff:
+      return "updating_diff";
+    case component_updater::CrxUpdateItem::kUpdating:
+      return "updating";
+    case component_updater::CrxUpdateItem::kUpdated:
+      return "updated";
+    case component_updater::CrxUpdateItem::kUpToDate:
+      return "uptodate";
+    case component_updater::CrxUpdateItem::kNoUpdate:
+      return "no_update";
+    case component_updater::CrxUpdateItem::kLastStatus:
+    default:
+      return "Unknown";
+  }
+  return "Unknown";
+}
+
+void ComponentsUI::OnEvent(Events event, const std::string& id) {
+  std::string converted_event;
+  ComponentEventToString(event, &converted_event);
+  base::DictionaryValue parameters;
+  parameters.SetString("event", converted_event.c_str());
+  if (!id.empty())
+    parameters.SetString("id", id.c_str());
+  web_ui()->CallJavascriptFunction("onComponentEvent", parameters);
+}