app_list: OnExtensionLoaded calls UpdateResults asynchronously rather than directly.

chrome/browser/ui/app_list/search/app_search_provider.cc

Index: chrome/browser/ui/app_list/search/app_search_provider.cc
diff --git a/chrome/browser/ui/app_list/search/app_search_provider.cc b/chrome/browser/ui/app_list/search/app_search_provider.cc
index 431493332ea662ac2d5cb1ed0a90081d5a4317eb..a45e4d9c19a4b14ee0e7cad18674d9cfb1e6fdbf 100644
--- a/chrome/browser/ui/app_list/search/app_search_provider.cc
+++ b/chrome/browser/ui/app_list/search/app_search_provider.cc
@@ -6,6 +6,8 @@
 
 #include <string>
 
+#include "base/bind.h"
+#include "base/message_loop/message_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/clock.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -52,7 +54,8 @@ AppSearchProvider::AppSearchProvider(Profile* profile,
     : profile_(profile),
       list_controller_(list_controller),
       extension_registry_observer_(this),
-      clock_(clock.Pass()) {
+      clock_(clock.Pass()),
+      update_results_factory_(this) {
   extension_registry_observer_.Add(ExtensionRegistry::Get(profile_));
   RefreshApps();
 }
@@ -101,6 +104,8 @@ void AppSearchProvider::UpdateResults() {
     }
     Add(result.Pass());
   }
+
+  update_results_factory_.InvalidateWeakPtrs();

[tapted, 2015/01/07 04:12:30]

TBH I haven't used WeakPtrs in Chrome all that much. But I think this step is
here to handle an UpdateResults call that doesn't come asynchronously via the
closure (otherwise it would only invalidate the weak pointer in the closure,
which is about to be cleaned up anyway).

However, in that case, it would be possible for things to be ordered like

OnExtensionLoaded [queue UpdateResults]
explicit UpdateResults (via Start()?) [invalidate queue]
OnExtensionLoaded [skip queuing UpdateResult since HasWeakPtrs]
async UpdateResults also skipped because it got invalidated
* still missing UpdateResults to handle the second OnExtensionLoaded


I *think* this line can just be removed to fix that case. The downside could be
that we do two UpdateResults() in a row without a refresh, but that's fine.

[xiyuan, 2015/01/07 04:51:57]

WeakPtrFactory::InvalidateWeakPtrs would mark the WeakPtr flag invalid and
release its reference to the flag. This way, all previous WeakPtrs returned from
its GetWeakPtr() call become invalid and unusable. And HasWeakPtrs would return
false because the flag is gone and the factory does not hold any WeakPtr.

So the case you worry about actually works. The explicit UpdateResults call
invalidates previous WeakPtr and effectively cancels the previous queued
UpdateResults call. The next OnExtensionLoaded would queue a new call because
HasWeakPtrs returns false now.

 }
 
 void AppSearchProvider::AddApps(const extensions::ExtensionSet& extensions) {
@@ -132,7 +137,12 @@ void AppSearchProvider::OnExtensionLoaded(
     content::BrowserContext* browser_context,
     const extensions::Extension* extension) {
   RefreshApps();
-  UpdateResults();
+  if (!update_results_factory_.HasWeakPtrs()) {
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&AppSearchProvider::UpdateResults,
+                   update_results_factory_.GetWeakPtr()));
+  }
 }
 
 void AppSearchProvider::OnExtensionUninstalled(
@@ -140,7 +150,12 @@ void AppSearchProvider::OnExtensionUninstalled(
     const extensions::Extension* extension,
     extensions::UninstallReason reason) {
   RefreshApps();
-  UpdateResults();
+  if (!update_results_factory_.HasWeakPtrs()) {
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&AppSearchProvider::UpdateResults,
+                   update_results_factory_.GetWeakPtr()));
+  }
 }
 
 }  // namespace app_list