Update Physical Web WebUI to show new results automatically

components/physical_web/webui/physical_web_base_message_handler.cc

Index: components/physical_web/webui/physical_web_base_message_handler.cc
diff --git a/components/physical_web/webui/physical_web_base_message_handler.cc b/components/physical_web/webui/physical_web_base_message_handler.cc
index 68aeca7791e14c274f27283fd78eb9726cfa9b22..4beed60557e9638ee601e4490da54b646db69f8d 100644
--- a/components/physical_web/webui/physical_web_base_message_handler.cc
+++ b/components/physical_web/webui/physical_web_base_message_handler.cc
@@ -13,20 +13,52 @@
 
 namespace physical_web_ui {
 
-PhysicalWebBaseMessageHandler::PhysicalWebBaseMessageHandler() {}
+PhysicalWebBaseMessageHandler::PhysicalWebBaseMessageHandler()
+    : data_source_(nullptr) {}
 
-PhysicalWebBaseMessageHandler::~PhysicalWebBaseMessageHandler() = default;
+PhysicalWebBaseMessageHandler::~PhysicalWebBaseMessageHandler() {
+  if (data_source_)
+    data_source_->UnregisterListener(this);
+}
+
+void PhysicalWebBaseMessageHandler::OnFound(const GURL& url) {
+  HandleRequestNearbyURLs(nullptr);
+}
+
+void PhysicalWebBaseMessageHandler::OnLost(const GURL& url) {
+  // do nothing
+}
+
+void PhysicalWebBaseMessageHandler::OnDistanceChanged(
+    const GURL& url,
+    double distance_estimate) {
+  // do nothing
+}
 
 void PhysicalWebBaseMessageHandler::RegisterMessages() {
+  base::Callback<void(const base::ListValue* args)> RequestNearbyUrlsCb =
+      base::Bind(

[mmocny, 2017/03/22 16:03:50]

Looks like you switched to using base::Bind, which states is deprecated.  Did
you mean to use BindRepeating or BindOnce?

[Ran, 2017/03/22 19:10:47]

Done.

+          [](PhysicalWebBaseMessageHandler* current,
+             const base::ListValue* args) {
+            current->HandleRequestNearbyURLs(nullptr);
+            UMA_HISTOGRAM_EXACT_LINEAR("PhysicalWeb.TotalUrls.OnInitialDisplay",
+                                       (int)current->GetPhysicalWebDataSource()
+                                           ->GetMetadataList()
+                                           ->size(),
+                                       50);
+          },
+          this);

[mmocny, 2017/03/22 16:03:50]

It looks like you switched to use raw `this`, instead of
`base::Unretained(this)` -- was this intended?

[Ran, 2017/03/22 19:10:47]

It is intended. Can't bind base::Unretained(this) to a lambda function. (even
can't capture this, Bind can only convert a captureless lambda to callback)

+  RegisterMessageCallback(kRequestNearbyUrls, RequestNearbyUrlsCb);

[mmocny, 2017/03/22 16:03:50]

Could you explain what this is doing?

It looks like you wrapped the HandleRequestNearbyURLs function with a lambda
which calls the UMA.

It still looks like OnInitialDisplay will be called every time this message is
handled, unless I misunderstand, so I don't see how this is resolved?

[Ran, 2017/03/22 19:10:47]

I separate the UMA from the HandleRequestNearbyURLs function. The UMA will be
called once the page is loaded, other time it will call HandleRequestNearbyURLs,
which does not include the UMA.

Renamed it to PhysicalWebPageLoaded to avoid confusing.

+
   RegisterMessageCallback(
-      kRequestNearbyUrls,
-      base::BindRepeating(
-          &PhysicalWebBaseMessageHandler::HandleRequestNearbyURLs,
-          base::Unretained(this)));
-  RegisterMessageCallback(kPhysicalWebItemClicked,
+      kPhysicalWebItemClicked,
       base::BindRepeating(
           &PhysicalWebBaseMessageHandler::HandlePhysicalWebItemClicked,
           base::Unretained(this)));
+
+  data_source_ = GetPhysicalWebDataSource();
+  if (data_source_)
+    data_source_->RegisterListener(this);
 }
 
 void PhysicalWebBaseMessageHandler::HandleRequestNearbyURLs(
@@ -36,9 +68,19 @@ void PhysicalWebBaseMessageHandler::HandleRequestNearbyURLs(
   std::unique_ptr<physical_web::MetadataList> metadata_list =
       GetPhysicalWebDataSource()->GetMetadataList();
 
+  // Append new metadata at the end of the list.
+  for (const auto& metadata_list_item : *metadata_list) {
+    const std::string& group_id = metadata_list_item.group_id;
+    if (metadata_map_.find(group_id) == metadata_map_.end()) {
+      ordered_group_ids_.push_back(group_id);
+      metadata_map_.emplace(group_id, metadata_list_item);
+    }
+  }
+
   auto metadata = base::MakeUnique<base::ListValue>();
   int index = 0;
-  for (const auto& metadata_list_item : *metadata_list) {
+  for (const auto& group_id : ordered_group_ids_) {

[mmocny, 2017/03/22 16:03:50]

Ran, as you can see, this is attempting to produce values one-per-groupid.

I wonder, is the group-id just not being set correctly?  Perhaps we should just
fix this code if we are seeing duplicates.

[Ran, 2017/03/22 19:10:47]

Done.

+    auto metadata_list_item = metadata_map_[group_id];
     auto metadata_item = base::MakeUnique<base::DictionaryValue>();
     metadata_item->SetString(physical_web_ui::kResolvedUrl,
                              metadata_list_item.resolved_url.spec());
@@ -57,9 +99,6 @@ void PhysicalWebBaseMessageHandler::HandleRequestNearbyURLs(
 
   results.Set(physical_web_ui::kMetadata, metadata.release());
 
-  UMA_HISTOGRAM_EXACT_LINEAR("PhysicalWeb.TotalUrls.OnInitialDisplay",
-                             (int)metadata_list->size(), 50);
-
   // Pass the list of Physical Web URL metadata to the WebUI. A jstemplate will
   // create a list view with an item for each URL.
   CallJavaScriptFunction(physical_web_ui::kReturnNearbyUrls, results);
@@ -82,4 +121,4 @@ void PhysicalWebBaseMessageHandler::HandlePhysicalWebItemClicked(
       base::UserMetricsAction("PhysicalWeb.WebUI.ListViewUrlSelected"));
 }
 
-} //  namespace physical_web_ui
+}  //  namespace physical_web_ui