Make ScriptBadgeController record when extensions run scripts on pages yet to

chrome/browser/extensions/script_badge_controller.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_EXTENSIONS_SCRIPT_BADGE_CONTROLLER_H_
 #define CHROME_BROWSER_EXTENSIONS_SCRIPT_BADGE_CONTROLLER_H_
 
 #include <map>
 #include <set>
 #include <string>
@@ skipping 46 matching lines @@
   virtual ~ScriptBadgeController();
 
   // LocationBarController implementation.
   virtual std::vector<ExtensionAction*> GetCurrentActions() const OVERRIDE;
   virtual void GetAttentionFor(const std::string& extension_id) OVERRIDE;
   virtual Action OnClicked(const std::string& extension_id,
                            int mouse_button) OVERRIDE;
   virtual void NotifyChange() OVERRIDE;
 
   // ScriptExecutor::Observer implementation.
+  // Public to be called from tests.
   virtual void OnExecuteScriptFinished(
       const std::string& extension_id, bool success, int32 page_id,
       const std::string& error, const base::ListValue& script_result) OVERRIDE;
 
+  // IPC::Message handlers.
+  // Public to be called from tests.

[Jeffrey Yasskin, 2012/07/25 22:22:39]

<obligatory-c++-trivia>
Because these implement public methods from public base classes, you can
actually call them from tests using
"static_cast<content::WebContentsObserver*>(script_badge_controller)->OnContentScriptsExecuting(...)".
That defeats the general point of making them private, although it might serve
enough documentation purpose to be worth privatizing the methods anyway.
</obligatory-c++-trivia>

[not at google - send to devlin, 2012/07/26 08:52:46]

I was going to say "I like it" and started changing it.

Then I was like, damn it past-Ben, you forgot to make that method virtual, lazy.

Then I realised it's not an override, just one of those methods that you
delegate IPC handling to. So, public it is :(

(either that or faking out IPCs, but tooo much effort).

+  void OnContentScriptsExecuting(const std::set<std::string>& extension_ids,
+                                 int32 page_id);
+
  private:
   // Gets the ExtensionService for |tab_contents_|.
   ExtensionService* GetExtensionService();
 
-  // Gets the current page ID.
+  // Gets the current page ID, or -1 if there isn't one yet.
   int32 GetPageID();
 
   // content::WebContentsObserver implementation.
   virtual void DidNavigateMainFrame(
       const content::LoadCommittedDetails& details,
       const content::FrameNavigateParams& params) OVERRIDE;
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
 
   // content::NotificationObserver implementation.
   virtual void Observe(int type,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
-  // IPC::Message handlers.
-  void OnContentScriptsExecuting(const std::set<std::string>& extension_ids,
-                                 int32 page_id);
-
   // Adds the extension's icon to the list of script badges.  Returns
   // the script badge ExtensionAction that was added, or NULL if
   // extension_id isn't valid.
   ExtensionAction* AddExtensionToCurrentActions(
       const std::string& extension_id);
 
   // Called when an extension is running script on the current tab,
   // and tries to insert an extension into the relevant collections.
   // Returns true if any change was made.
   bool MarkExtensionExecuting(const std::string& extension_id);
 
   // Tries to erase an extension from the relevant collections, and returns
   // whether any change was made.
   bool EraseExtension(const Extension* extension);
 
   // Our parent TabContents.
   TabContents* tab_contents_;
 
   // The current extension actions in the order they appeared.  These come from
   // calls to ExecuteScript or getAttention on the current frame.
   std::vector<ExtensionAction*> current_actions_;
 
   // The extensions that have actions in current_actions_.
   std::set<std::string> extensions_in_current_actions_;
 
+  // Extensions that have actions in a page ID that hasn't been reached yet.

[Jeffrey Yasskin, 2012/07/25 22:22:39]

This feels like something that will bite us later if we don't figure out what's
really causing it. How does the OnExecuting message get received before the
navigation entry is set?

[not at google - send to devlin, 2012/07/26 08:52:46]

Yeah, so the crash was from OnContentScriptExecuting.

My initial assumption, and the assumption of this patch, is that it runs early
(like on document creation) then send up the IPC, somehow before the navigation
entry is set.

The documentation in NavigationController just says "it can be NULL" but not
why. I assumed it was before there were any navigation entries? And if these
IPCs can arrive before the navigation entry is created, then presumably they can
arrive when they're stale too?

I briefly also wondered if it was because the navigation stuff had been torn
down, but then why would the IPC arrive? Saying that now, I realise it's just as
feasible as it not being around when it's been created, so eh.

I guess I should really go and look through the content code to verify all this.
It can't be a black box forever.

+  typedef std::map<int, std::vector<std::string> > FutureActionsMap;
+  FutureActionsMap future_actions_;
+
   // Listen to extension unloaded notifications.
   content::NotificationRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(ScriptBadgeController);
 };
 
 }  // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_SCRIPT_BADGE_CONTROLLER_H_