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..1c116904913f92858615af632226e87aa2627007 100644
--- a/components/physical_web/webui/physical_web_base_message_handler.cc
+++ b/components/physical_web/webui/physical_web_base_message_handler.cc
@@ -13,32 +13,74 @@
 
 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) {
+  ReturnNearbyURLs(nullptr);
+}
+
+void PhysicalWebBaseMessageHandler::OnLost(const GURL& url) {
+  // do nothing
+}
+
+void PhysicalWebBaseMessageHandler::OnDistanceChanged(
+    const GURL& url,
+    double distance_estimate) {
+  // do nothing
+}
 
 void PhysicalWebBaseMessageHandler::RegisterMessages() {
-  RegisterMessageCallback(
-      kRequestNearbyUrls,
+  base::Callback<void(const base::ListValue* args)> PhysicalWebPageLoadedCb =
       base::BindRepeating(
-          &PhysicalWebBaseMessageHandler::HandleRequestNearbyURLs,
-          base::Unretained(this)));
-  RegisterMessageCallback(kPhysicalWebItemClicked,
+          [](PhysicalWebBaseMessageHandler* current,
+             const base::ListValue* args) {
+            current->ReturnNearbyURLs(nullptr);
+            UMA_HISTOGRAM_EXACT_LINEAR("PhysicalWeb.TotalUrls.OnInitialDisplay",
+                                       (int)current->GetPhysicalWebDataSource()
+                                           ->GetMetadataList()
+                                           ->size(),
+                                       50);
+          },
+          this);
+  RegisterMessageCallback(kPhysicalWebPageLoaded, PhysicalWebPageLoadedCb);

[mmocny, 2017/03/24 17:15:12]

I think you should create a member function called HandlePhysicalWebPageLoaded
since I think you will now want to track Load and Reload differently, plus the
lambda is getting ugly.

Also, I think you should not call GetMetadataList()->size() here, you should use
the actual number of results that are shown, same as your other CL
https://codereview.chromium.org/2775653002

[Ran, 2017/03/24 18:45:19]

Done.

+
+  RegisterMessageCallback(
+      kPhysicalWebItemClicked,
       base::BindRepeating(
           &PhysicalWebBaseMessageHandler::HandlePhysicalWebItemClicked,
           base::Unretained(this)));
+
+  data_source_ = GetPhysicalWebDataSource();
+  if (data_source_)
+    data_source_->RegisterListener(this);

[mmocny, 2017/03/24 17:15:12]

Should we perhaps do this in the constructor instead?  This isn't related to
registering messages.

 I know we cannot push results to webview until after OnPageLoaded, so
alternatively we can also move this registration there.  However I think we may
already have to do some more work given that on page reloads we don't unregister
this listener, and probably already need a way to track when page is
loaded/unloaded to prevent OnFound causing an issue if timing is bad.

Also, if we subscribed in the constructor we could start a foreground scan + PWS
resolve in parallel while the webui page loads.  I don't expect this is a big
deal, but it may be a good idea for code-cleanliness anyway.

[Ran, 2017/03/24 18:45:19]

Can't call it in constructor as GetPhysicalWebDataSource is a pure virtual
function..

[mmocny, 2017/03/24 18:54:14]

Okay fair enough.  We could create a "start" method, but at that point I think
thats what RegisterMessages is.

Just to review: who named this "RegisterMessages"?  Is this standard naming
scheme or should we rename it?

[mattreynolds, 2017/03/27 21:46:59]

The name comes from the web_ui message handler which calls it RegisterMethods in
both the iOS and non-iOS implementations:

content/public/browser/web_ui_message_handler.h
ios/web/public/webui/web_ui_ios_message_handler.h

Both implementations have the same documentation suggesting it's fine to put
other initialization tasks in RegisterMethods:

  // This is where subclasses specify which messages they'd like to handle and
  // perform any additional initialization. At this point web_ui() will return
  // the associated WebUI[IOS] object.

 }
 
-void PhysicalWebBaseMessageHandler::HandleRequestNearbyURLs(
+void PhysicalWebBaseMessageHandler::ReturnNearbyURLs(
     const base::ListValue* args) {
   base::DictionaryValue results;
 
   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_) {
+    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