Add callback in UserScriptLoader to notify users when scripts are loaded.

BUG=461052, 482446

Review URL: https://codereview.chromium.org/1110143002

Cr-Commit-Position: refs/heads/master@{#328385}

13 files changed, 169 insertions, 18 deletions

diff --git a/base/scoped_observer.h b/base/scoped_observer.h
index 5b0d533..422701b 100644
--- a/base/scoped_observer.h
+++ b/base/scoped_observer.h
@@ -48,6 +48,8 @@ class ScopedObserver {
         sources_.end();
   }
 
+  bool IsObservingSources() const { return !sources_.empty(); }
+
  private:
   Observer* observer_;
 
diff --git a/components/guest_view/browser/guest_view_base.cc b/components/guest_view/browser/guest_view_base.cc
index 1b1b5ed..201e081 100644
--- a/components/guest_view/browser/guest_view_base.cc
+++ b/components/guest_view/browser/guest_view_base.cc
@@ -478,7 +478,8 @@ void GuestViewBase::SetGuestHost(content::GuestHost* guest_host) {
 
 void GuestViewBase::WillAttach(content::WebContents* embedder_web_contents,
                                int element_instance_id,
-                               bool is_full_page_plugin) {
+                               bool is_full_page_plugin,
+                               const base::Closure& callback) {
   if (owner_web_contents_ != embedder_web_contents) {
     DCHECK_EQ(owner_contents_observer_->web_contents(), owner_web_contents_);
     // Stop tracking the old embedder's zoom level.
@@ -497,6 +498,16 @@ void GuestViewBase::WillAttach(content::WebContents* embedder_web_contents,
   is_full_page_plugin_ = is_full_page_plugin;
 
   WillAttachToEmbedder();
+
+  // Completing attachment will resume suspended resource loads and then send
+  // queued events.
+  SignalWhenReady(callback);
+}
+
+void GuestViewBase::SignalWhenReady(const base::Closure& callback) {
+  // The default behavior is to call the |callback| immediately. Derived classes
+  // can implement an alternative signal for readiness.
+  callback.Run();
 }
 
 int GuestViewBase::LogicalPixelsToPhysicalPixels(double logical_pixels) const {
diff --git a/components/guest_view/browser/guest_view_base.h b/components/guest_view/browser/guest_view_base.h
index 74fb59c..04d3f41 100644
--- a/components/guest_view/browser/guest_view_base.h
+++ b/components/guest_view/browser/guest_view_base.h
@@ -281,7 +281,8 @@ class GuestViewBase : public content::BrowserPluginGuestDelegate,
   void SetGuestHost(content::GuestHost* guest_host) final;
   void WillAttach(content::WebContents* embedder_web_contents,
                   int browser_plugin_instance_id,
-                  bool is_full_page_plugin) final;
+                  bool is_full_page_plugin,
+                  const base::Closure& callback) final;
 
   // ui_zoom::ZoomObserver implementation.
   void OnZoomChanged(
@@ -348,6 +349,11 @@ class GuestViewBase : public content::BrowserPluginGuestDelegate,
 
   void SetGuestZoomLevelToMatchEmbedder();
 
+  // Signals that the guest view is ready.  The default implementation signals
+  // immediately, but derived class can override this if they need to do
+  // asynchronous setup.
+  virtual void SignalWhenReady(const base::Closure& callback);
+
  private:
   class OwnerContentsObserver;
 
diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc
index 79e0dff..c43717b 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/browser_plugin_guest.cc
@@ -631,9 +631,21 @@ void BrowserPluginGuest::Attach(
         ack);
     last_pending_frame_.reset();
   }
+
+  // The guest is owned by the embedder. Attach is queued up so we cannot
+  // change embedders before attach completes. If the embedder goes away,
+  // so does the guest and so we will never call WillAttachComplete because
+  // we have a weak ptr.
   delegate_->WillAttach(embedder_web_contents, browser_plugin_instance_id,
-                        params.is_full_page_plugin);
+                        params.is_full_page_plugin,
+                        base::Bind(&BrowserPluginGuest::OnWillAttachComplete,
+                                   weak_ptr_factory_.GetWeakPtr(),
+                                   embedder_web_contents, params));
+}
 
+void BrowserPluginGuest::OnWillAttachComplete(
+    WebContentsImpl* embedder_web_contents,
+    const BrowserPluginHostMsg_Attach_Params& params) {
   // If a RenderView has already been created for this new window, then we need
   // to initialize the browser-side state now so that the RenderFrameHostManager
   // does not create a new RenderView on navigation.
diff --git a/content/browser/browser_plugin/browser_plugin_guest.h b/content/browser/browser_plugin/browser_plugin_guest.h
index 946d8d9..86278b9 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.h
+++ b/content/browser/browser_plugin/browser_plugin_guest.h
@@ -339,6 +339,10 @@ class CONTENT_EXPORT BrowserPluginGuest : public GuestHost,
                          bool is_top_level,
                          const std::string& name);
 
+  // Called when WillAttach is complete.
+  void OnWillAttachComplete(WebContentsImpl* embedder_web_contents,
+                            const BrowserPluginHostMsg_Attach_Params& params);
+
   // Forwards all messages from the |pending_messages_| queue to the embedder.
   void SendQueuedMessages();
 
diff --git a/content/public/browser/browser_plugin_guest_delegate.h b/content/public/browser/browser_plugin_guest_delegate.h
index fdc3037..000e3b6 100644
--- a/content/public/browser/browser_plugin_guest_delegate.h
+++ b/content/public/browser/browser_plugin_guest_delegate.h
@@ -31,9 +31,12 @@ class CONTENT_EXPORT BrowserPluginGuestDelegate {
   // Notification that the embedder will begin attachment. This is called
   // prior to resuming resource loads. |element_instance_id| uniquely identifies
   // the element that will serve as a container for the guest.
+  // Once the content embedder has completed setting up state for attachment, it
+  // must call the |completion_callback| to complete attachment.
   virtual void WillAttach(content::WebContents* embedder_web_contents,
                           int element_instance_id,
-                          bool is_full_page_plugin) {}
+                          bool is_full_page_plugin,
+                          const base::Closure& completion_callback) {}
 
   virtual WebContents* CreateNewGuestWindow(
       const WebContents::CreateParams& create_params);
diff --git a/extensions/browser/declarative_user_script_master.h b/extensions/browser/declarative_user_script_master.h
index 55c94f5..eb27b0c 100644
--- a/extensions/browser/declarative_user_script_master.h
+++ b/extensions/browser/declarative_user_script_master.h
@@ -56,6 +56,8 @@ class DeclarativeUserScriptMaster {
 
   const HostID& host_id() const { return host_id_; }
 
+  UserScriptLoader* loader() { return loader_.get(); }
+
  private:
   // ID of host that owns scripts that this component manages.
   HostID host_id_;
diff --git a/extensions/browser/guest_view/web_view/web_view_content_script_manager.cc b/extensions/browser/guest_view/web_view/web_view_content_script_manager.cc
index ca503dd..b03eab7 100644
--- a/extensions/browser/guest_view/web_view/web_view_content_script_manager.cc
+++ b/extensions/browser/guest_view/web_view/web_view_content_script_manager.cc
@@ -87,7 +87,7 @@ class WebViewContentScriptManager::OwnerWebContentsObserver
 
 WebViewContentScriptManager::WebViewContentScriptManager(
     content::BrowserContext* browser_context)
-    : browser_context_(browser_context) {
+    : user_script_loader_observer_(this), browser_context_(browser_context) {
 }
 
 WebViewContentScriptManager::~WebViewContentScriptManager() {
@@ -141,22 +141,34 @@ void WebViewContentScriptManager::AddContentScripts(
 
   // Step 2: updates the guest_content_script_map_.
   ContentScriptMap& map = iter->second;
+  std::set<UserScript> scripts_to_delete;
   for (const UserScript& script : scripts) {
     auto map_iter = map.find(script.name());
     // If a content script has the same name as the new one, remove the old
     // script first, and insert the new one.
     if (map_iter != map.end()) {
-      master->RemoveScript(map_iter->second);
+      scripts_to_delete.insert(map_iter->second);
       map.erase(map_iter);
     }
     map.insert(std::pair<std::string, UserScript>(script.name(), script));
     ids_to_add.insert(script.id());
   }
 
-  // Step 3: adds new scripts to the master.
+  if (!scripts_to_delete.empty()) {
+    master->RemoveScripts(scripts_to_delete);
+  }
+
+  // Step 3: makes WebViewContentScriptManager become an observer of the
+  // |loader| for scripts loaded event.
+  UserScriptLoader* loader = master->loader();
+  DCHECK(loader);
+  if (!user_script_loader_observer_.IsObserving(loader))
+    user_script_loader_observer_.Add(loader);
+
+  // Step 4: adds new scripts to the master.
   master->AddScripts(scripts, embedder_process_id, embedder_routing_id);
 
-  // Step 4: creates owner web contents observer for the given
+  // Step 5: creates owner web contents observer for the given
   // |embedder_web_contents| if it doesn't exist.
   auto observer_iter =
       owner_web_contents_observer_map_.find(embedder_web_contents);
@@ -169,7 +181,7 @@ void WebViewContentScriptManager::AddContentScripts(
     observer_iter->second->add_view_instance_id(view_instance_id);
   }
 
-  // Step 5: updates WebViewRenderState in the IO thread.
+  // Step 6: updates WebViewRenderState in the IO thread.
   // It is safe to use base::Unretained(WebViewRendererState::GetInstance())
   // since WebViewRendererState::GetInstance() always returns a Singleton of
   // WebViewRendererState.
@@ -208,7 +220,7 @@ void WebViewContentScriptManager::RemoveContentScripts(
   std::set<UserScript> scripts_to_delete;
 
   // Step 1: removes content scripts from |master| and updates
-  // |guest_content_script_map_|
+  // |guest_content_script_map_|.
   std::map<std::string, UserScript>& map = script_map_iter->second;
   // If the |script_name_list| is empty, all the content scripts added by the
   // guest will be removed; otherwise, removes the scripts in the
@@ -232,10 +244,17 @@ void WebViewContentScriptManager::RemoveContentScripts(
     }
   }
 
-  // Step 2: removes content scripts from master.
+  // Step 2: makes WebViewContentScriptManager become an observer of the
+  // |loader| for scripts loaded event.
+  UserScriptLoader* loader = master->loader();
+  DCHECK(loader);
+  if (!user_script_loader_observer_.IsObserving(loader))
+    user_script_loader_observer_.Add(loader);
+
+  // Step 3: removes content scripts from master.
   master->RemoveScripts(scripts_to_delete);
 
-  // Step 3: updates WebViewRenderState in the IO thread.
+  // Step 4: updates WebViewRenderState in the IO thread.
   if (!ids_to_delete.empty()) {
     content::BrowserThread::PostTask(
         content::BrowserThread::IO, FROM_HERE,
@@ -267,4 +286,32 @@ std::set<int> WebViewContentScriptManager::GetContentScriptIDSet(
   return ids;
 }
 
+void WebViewContentScriptManager::SignalOnScriptsLoaded(
+    const base::Closure& callback) {
+  if (!user_script_loader_observer_.IsObservingSources()) {
+    callback.Run();
+    return;
+  }
+  pending_scripts_loading_callbacks_.push_back(callback);
+}
+
+void WebViewContentScriptManager::OnScriptsLoaded(UserScriptLoader* loader) {
+  user_script_loader_observer_.Remove(loader);
+  RunCallbacksIfReady();
+}
+
+void WebViewContentScriptManager::OnUserScriptLoaderDestroyed(
+    UserScriptLoader* loader) {
+  user_script_loader_observer_.Remove(loader);
+  RunCallbacksIfReady();
+}
+
+void WebViewContentScriptManager::RunCallbacksIfReady() {
+  if (user_script_loader_observer_.IsObservingSources())
+    return;
+  for (auto& callback : pending_scripts_loading_callbacks_)
+    callback.Run();
+  pending_scripts_loading_callbacks_.clear();
+}
+
 }  // namespace extensions
diff --git a/extensions/browser/guest_view/web_view/web_view_content_script_manager.h b/extensions/browser/guest_view/web_view/web_view_content_script_manager.h
index 77c72a9..f6a2521 100644
--- a/extensions/browser/guest_view/web_view/web_view_content_script_manager.h
+++ b/extensions/browser/guest_view/web_view/web_view_content_script_manager.h
@@ -10,7 +10,9 @@
 #include <string>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/supports_user_data.h"
+#include "extensions/browser/user_script_loader.h"
 
 struct HostID;
 
@@ -26,7 +28,8 @@ class UserScript;
 // guest adds and removes programmatically.
 // TODO(hanxi): crbug.com/476938. Introduce a new class to manage the lifetime
 // of <webview> and clean up WebViewContentScriptManager.
-class WebViewContentScriptManager : public base::SupportsUserData::Data {
+class WebViewContentScriptManager : public base::SupportsUserData::Data,
+                                    public UserScriptLoader::Observer {
  public:
   explicit WebViewContentScriptManager(
       content::BrowserContext* browser_context);
@@ -57,6 +60,12 @@ class WebViewContentScriptManager : public base::SupportsUserData::Data {
   std::set<int> GetContentScriptIDSet(int embedder_process_id,
                                       int view_instance_id);
 
+  // Checks if there is any pending content scripts to load.
+  // If no, run |callback| immediately; otherwise caches the |callback|, and
+  // the |callback| will be called after all the pending content scripts are
+  // loaded.
+  void SignalOnScriptsLoaded(const base::Closure& callback);
+
  private:
   class OwnerWebContentsObserver;
 
@@ -67,15 +76,31 @@ class WebViewContentScriptManager : public base::SupportsUserData::Data {
   using OwnerWebContentsObserverMap =
       std::map<content::WebContents*, linked_ptr<OwnerWebContentsObserver>>;
 
+  // UserScriptLoader::Observer implementation:
+  void OnScriptsLoaded(UserScriptLoader* loader) override;
+  void OnUserScriptLoaderDestroyed(UserScriptLoader* loader) override;
+
   // Called by OwnerWebContentsObserver when the observer sees a main frame
   // navigation or sees the process has went away or the |embedder_web_contents|
   // is being destroyed.
   void RemoveObserver(content::WebContents* embedder_web_contents);
 
+  // If |user_script_loader_observer_| doesn't observe any source, we will run
+  // all the remaining callbacks in |pending_scripts_loading_callbacks_|.
+  void RunCallbacksIfReady();
+
   OwnerWebContentsObserverMap owner_web_contents_observer_map_;
 
   GuestContentScriptMap guest_content_script_map_;
 
+  // WebViewContentScriptManager observes UserScriptLoader to wait for scripts
+  // loaded event.
+  ScopedObserver<UserScriptLoader, UserScriptLoader::Observer>
+      user_script_loader_observer_;
+
+  // Caches callbacks and resumes them when all the scripts are loaded.
+  std::vector<base::Closure> pending_scripts_loading_callbacks_;
+
   content::BrowserContext* browser_context_;
 
   DISALLOW_COPY_AND_ASSIGN(WebViewContentScriptManager);
diff --git a/extensions/browser/guest_view/web_view/web_view_guest.cc b/extensions/browser/guest_view/web_view/web_view_guest.cc
index 15d1ca9..11aad86 100644
--- a/extensions/browser/guest_view/web_view/web_view_guest.cc
+++ b/extensions/browser/guest_view/web_view/web_view_guest.cc
@@ -962,6 +962,11 @@ void WebViewGuest::RequestPointerLockPermission(
       callback);
 }
 
+void WebViewGuest::SignalWhenReady(const base::Closure& callback) {
+  auto manager = WebViewContentScriptManager::Get(browser_context());
+  manager->SignalOnScriptsLoaded(callback);
+}
+
 void WebViewGuest::WillAttachToEmbedder() {
   rules_registry_id_ = GetOrGenerateRulesRegistryID(
       owner_web_contents()->GetRenderProcessHost()->GetID(),
@@ -985,8 +990,17 @@ void WebViewGuest::NavigateGuest(const std::string& src,
 
   GURL url = ResolveURL(src);
 
-  LoadURLWithParams(url, content::Referrer(),
-                    ui::PAGE_TRANSITION_AUTO_TOPLEVEL,
+  // We wait for all the content scripts to load and then navigate the guest
+  // if the navigation is embedder-initiated. For browser-initiated navigations,
+  // content scripts will be ready.
+  if (force_navigation) {
+    SignalWhenReady(
+        base::Bind(&WebViewGuest::LoadURLWithParams,
+                   weak_ptr_factory_.GetWeakPtr(), url, content::Referrer(),
+                   ui::PAGE_TRANSITION_AUTO_TOPLEVEL, force_navigation));
+    return;
+  }
+  LoadURLWithParams(url, content::Referrer(), ui::PAGE_TRANSITION_AUTO_TOPLEVEL,
                     force_navigation);
 }
 
@@ -1090,8 +1104,8 @@ void WebViewGuest::ApplyAttributes(const base::DictionaryValue& params) {
   // Only read the src attribute if this is not a New Window API flow.
   if (!is_pending_new_window) {
     std::string src;
-    params.GetString(webview::kAttributeSrc, &src);
-    NavigateGuest(src, false /* force_navigation */);
+    if (params.GetString(webview::kAttributeSrc, &src))
+      NavigateGuest(src, true /* force_navigation */);
   }
 }
 
@@ -1319,7 +1333,7 @@ void WebViewGuest::LoadURLWithParams(const GURL& url,
   if (scheme_is_blocked || !url.is_valid()) {
     LoadAbort(true /* is_top_level */, url, net::ERR_ABORTED,
               net::ErrorToShortString(net::ERR_ABORTED));
-    NavigateGuest(url::kAboutBlankURL, true /* force_navigation */);
+    NavigateGuest(url::kAboutBlankURL, false /* force_navigation */);
     return;
   }
 
diff --git a/extensions/browser/guest_view/web_view/web_view_guest.h b/extensions/browser/guest_view/web_view/web_view_guest.h
index 6ca0105..dcfd378 100644
--- a/extensions/browser/guest_view/web_view/web_view_guest.h
+++ b/extensions/browser/guest_view/web_view/web_view_guest.h
@@ -120,6 +120,7 @@ class WebViewGuest : public guest_view::GuestView<WebViewGuest>,
   void GuestZoomChanged(double old_zoom_level, double new_zoom_level) override;
   bool IsAutoSizeSupported() const override;
   bool IsDragAndDropEnabled() const override;
+  void SignalWhenReady(const base::Closure& callback) override;
   void WillAttachToEmbedder() override;
   void WillDestroy() override;
 
diff --git a/extensions/browser/user_script_loader.cc b/extensions/browser/user_script_loader.cc
index 94b2c07..7daf95d 100644
--- a/extensions/browser/user_script_loader.cc
+++ b/extensions/browser/user_script_loader.cc
@@ -154,6 +154,7 @@ UserScriptLoader::UserScriptLoader(BrowserContext* browser_context,
 }
 
 UserScriptLoader::~UserScriptLoader() {
+  FOR_EACH_OBSERVER(Observer, observers_, OnUserScriptLoaderDestroyed(this));
 }
 
 void UserScriptLoader::AddScripts(const std::set<UserScript>& scripts) {
@@ -317,6 +318,14 @@ scoped_ptr<base::SharedMemory> UserScriptLoader::Serialize(
                                                 /*read_only=*/true));
 }
 
+void UserScriptLoader::AddObserver(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void UserScriptLoader::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 void UserScriptLoader::SetReady(bool ready) {
   bool was_ready = ready_;
   ready_ = ready;
@@ -358,10 +367,12 @@ void UserScriptLoader::OnScriptsLoaded(
   }
   changed_hosts_.clear();
 
+  // TODO(hanxi): Remove the NOTIFICATION_USER_SCRIPTS_UPDATED.
   content::NotificationService::current()->Notify(
       extensions::NOTIFICATION_USER_SCRIPTS_UPDATED,
       content::Source<BrowserContext>(browser_context_),
       content::Details<base::SharedMemory>(shared_memory_.get()));
+  FOR_EACH_OBSERVER(Observer, observers_, OnScriptsLoaded(this));
 }
 
 void UserScriptLoader::SendUpdate(content::RenderProcessHost* process,
diff --git a/extensions/browser/user_script_loader.h b/extensions/browser/user_script_loader.h
index 1eeea31..c1f10ac 100644
--- a/extensions/browser/user_script_loader.h
+++ b/extensions/browser/user_script_loader.h
@@ -12,6 +12,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/shared_memory.h"
 #include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
 #include "base/scoped_observer.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
@@ -41,6 +42,11 @@ class UserScriptLoader : public content::NotificationObserver {
   using LoadScriptsCallback =
       base::Callback<void(scoped_ptr<UserScriptList>,
                           scoped_ptr<base::SharedMemory>)>;
+  class Observer {
+   public:
+    virtual void OnScriptsLoaded(UserScriptLoader* loader) = 0;
+    virtual void OnUserScriptLoaderDestroyed(UserScriptLoader* loader) = 0;
+  };
 
   // Parses the includes out of |script| and returns them in |includes|.
   static bool ParseMetadataHeader(const base::StringPiece& script_text,
@@ -79,6 +85,10 @@ class UserScriptLoader : public content::NotificationObserver {
   static scoped_ptr<base::SharedMemory> Serialize(
       const extensions::UserScriptList& scripts);
 
+  // Adds or removes observers.
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
  protected:
   // Allows the derived classes have different ways to load user scripts.
   virtual void LoadScripts(scoped_ptr<UserScriptList> user_scripts,
@@ -164,6 +174,9 @@ class UserScriptLoader : public content::NotificationObserver {
   // non-empty value for declarative user script shared memory regions.
   HostID host_id_;
 
+  // The associated observers.
+  ObserverList<Observer> observers_;
+
   base::WeakPtrFactory<UserScriptLoader> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(UserScriptLoader);