Remove page id from the renderer side of extension's GetApplicationInfo.

BUG=371151
TEST=everything still works
NOTRY=true

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@281088 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 65 insertions, 79 deletions

diff --git a/chrome/browser/extensions/tab_helper.cc b/chrome/browser/extensions/tab_helper.cc
index cf9090a..2759429 100644
--- a/chrome/browser/extensions/tab_helper.cc
+++ b/chrome/browser/extensions/tab_helper.cc
@@ -101,10 +101,13 @@ TabHelper::TabHelper(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
       extension_app_(NULL),
       extension_function_dispatcher_(
-          Profile::FromBrowserContext(web_contents->GetBrowserContext()), this),
+          Profile::FromBrowserContext(web_contents->GetBrowserContext()),
+          this),
       pending_web_app_action_(NONE),
-      script_executor_(new ScriptExecutor(web_contents,
-                                          &script_execution_observers_)),
+      last_committed_page_id_(-1),
+      update_shortcut_on_load_complete_(false),
+      script_executor_(
+          new ScriptExecutor(web_contents, &script_execution_observers_)),
       location_bar_controller_(new LocationBarController(web_contents)),
       image_loader_ptr_factory_(this),
       webstore_inline_installer_factory_(new WebstoreInlineInstallerFactory()) {
@@ -136,30 +139,16 @@ TabHelper::~TabHelper() {
 
 void TabHelper::CreateApplicationShortcuts() {
   DCHECK(CanCreateApplicationShortcuts());
-  NavigationEntry* entry =
-      web_contents()->GetController().GetLastCommittedEntry();
-  if (!entry)
-    return;
-
-  pending_web_app_action_ = CREATE_SHORTCUT;
-
   // Start fetching web app info for CreateApplicationShortcut dialog and show
   // the dialog when the data is available in OnDidGetApplicationInfo.
-  GetApplicationInfo(entry->GetPageID());
+  GetApplicationInfo(CREATE_SHORTCUT);
 }
 
 void TabHelper::CreateHostedAppFromWebContents() {
   DCHECK(CanCreateBookmarkApp());
-  NavigationEntry* entry =
-      web_contents()->GetController().GetLastCommittedEntry();
-  if (!entry)
-    return;
-
-  pending_web_app_action_ = CREATE_HOSTED_APP;
-
   // Start fetching web app info for CreateApplicationShortcut dialog and show
   // the dialog when the data is available in OnDidGetApplicationInfo.
-  GetApplicationInfo(entry->GetPageID());
+  GetApplicationInfo(CREATE_HOSTED_APP);
 }
 
 bool TabHelper::CanCreateApplicationShortcuts() const {
@@ -327,15 +316,15 @@ void TabHelper::DidCloneToNewWebContents(WebContents* old_web_contents,
   new_helper->extension_app_icon_ = extension_app_icon_;
 }
 
-void TabHelper::OnDidGetApplicationInfo(int32 page_id,
-                                        const WebApplicationInfo& info) {
+void TabHelper::OnDidGetApplicationInfo(const WebApplicationInfo& info) {
 #if !defined(OS_MACOSX)
   web_app_info_ = info;
 
   NavigationEntry* entry =
       web_contents()->GetController().GetLastCommittedEntry();
-  if (!entry || (entry->GetPageID() != page_id))
+  if (!entry || last_committed_page_id_ != entry->GetPageID())
     return;
+  last_committed_page_id_ = -1;
 
   switch (pending_web_app_action_) {
     case CREATE_SHORTCUT: {
@@ -536,32 +525,34 @@ WebContents* TabHelper::GetAssociatedWebContents() const {
   return web_contents();
 }
 
-void TabHelper::GetApplicationInfo(int32 page_id) {
-  Send(new ChromeExtensionMsg_GetApplicationInfo(routing_id(), page_id));
+void TabHelper::GetApplicationInfo(WebAppAction action) {
+  NavigationEntry* entry =
+      web_contents()->GetController().GetLastCommittedEntry();
+  if (!entry)
+    return;
+
+  pending_web_app_action_ = action;
+  last_committed_page_id_ = entry->GetPageID();
+
+  Send(new ChromeExtensionMsg_GetApplicationInfo(routing_id()));
 }
 
 void TabHelper::Observe(int type,
                         const content::NotificationSource& source,
                         const content::NotificationDetails& details) {
-  switch (type) {
-    case content::NOTIFICATION_LOAD_STOP: {
-      const NavigationController& controller =
-          *content::Source<NavigationController>(source).ptr();
-      DCHECK_EQ(controller.GetWebContents(), web_contents());
-
-      if (pending_web_app_action_ == UPDATE_SHORTCUT) {
-        // Schedule a shortcut update when web application info is available if
-        // last committed entry is not NULL. Last committed entry could be NULL
-        // when an interstitial page is injected (e.g. bad https certificate,
-        // malware site etc). When this happens, we abort the shortcut update.
-        NavigationEntry* entry = controller.GetLastCommittedEntry();
-        if (entry)
-          GetApplicationInfo(entry->GetPageID());
-        else
-          pending_web_app_action_ = NONE;
-      }
-      break;
-    }
+  DCHECK_EQ(content::NOTIFICATION_LOAD_STOP, type);
+  const NavigationController& controller =
+      *content::Source<NavigationController>(source).ptr();
+  DCHECK_EQ(controller.GetWebContents(), web_contents());
+
+  if (update_shortcut_on_load_complete_) {
+    update_shortcut_on_load_complete_ = false;
+    // Schedule a shortcut update when web application info is available if
+    // last committed entry is not NULL. Last committed entry could be NULL
+    // when an interstitial page is injected (e.g. bad https certificate,
+    // malware site etc). When this happens, we abort the shortcut update.
+    if (controller.GetLastCommittedEntry())
+      GetApplicationInfo(UPDATE_SHORTCUT);
   }
 }
 
diff --git a/chrome/browser/extensions/tab_helper.h b/chrome/browser/extensions/tab_helper.h
index 6947021..9725f21 100644
--- a/chrome/browser/extensions/tab_helper.h
+++ b/chrome/browser/extensions/tab_helper.h
@@ -49,15 +49,6 @@ class TabHelper : public content::WebContentsObserver,
                   public content::NotificationObserver,
                   public content::WebContentsUserData<TabHelper> {
  public:
-  // Different types of action when web app info is available.
-  // OnDidGetApplicationInfo uses this to dispatch calls.
-  enum WebAppAction {
-    NONE,              // No action at all.
-    CREATE_SHORTCUT,   // Bring up create application shortcut dialog.
-    CREATE_HOSTED_APP, // Create and install a hosted app.
-    UPDATE_SHORTCUT    // Update icon for app shortcut.
-  };
-
   // Observer base class for classes that need to be notified when content
   // scripts and/or tabs.executeScript calls run on a page.
   class ScriptExecutionObserver {
@@ -102,8 +93,8 @@ class TabHelper : public content::WebContentsObserver,
   bool CanCreateApplicationShortcuts() const;
   bool CanCreateBookmarkApp() const;
 
-  void set_pending_web_app_action(WebAppAction action) {
-    pending_web_app_action_ = action;
+  void UpdateShortcutOnLoadComplete() {
+    update_shortcut_on_load_complete_ = true;
   }
 
   // App extensions ------------------------------------------------------------
@@ -164,6 +155,15 @@ class TabHelper : public content::WebContentsObserver,
       WebstoreInlineInstallerFactory* factory);
 
  private:
+  // Different types of action when web app info is available.
+  // OnDidGetApplicationInfo uses this to dispatch calls.
+  enum WebAppAction {
+    NONE,               // No action at all.
+    CREATE_SHORTCUT,    // Bring up create application shortcut dialog.
+    CREATE_HOSTED_APP,  // Create and install a hosted app.
+    UPDATE_SHORTCUT     // Update icon for app shortcut.
+  };
+
   explicit TabHelper(content::WebContents* web_contents);
   friend class content::WebContentsUserData<TabHelper>;
 
@@ -191,7 +191,7 @@ class TabHelper : public content::WebContentsObserver,
   virtual content::WebContents* GetAssociatedWebContents() const OVERRIDE;
 
   // Message handlers.
-  void OnDidGetApplicationInfo(int32 page_id, const WebApplicationInfo& info);
+  void OnDidGetApplicationInfo(const WebApplicationInfo& info);
   void OnInlineWebstoreInstall(int install_id,
                                int return_route_id,
                                const std::string& webstore_item_id,
@@ -234,7 +234,7 @@ class TabHelper : public content::WebContentsObserver,
   // Requests application info for the specified page. This is an asynchronous
   // request. The delegate is notified by way of OnDidGetApplicationInfo when
   // the data is available.
-  void GetApplicationInfo(int32 page_id);
+  void GetApplicationInfo(WebAppAction action);
 
   // Sends our tab ID to |render_view_host|.
   void SetTabId(content::RenderViewHost* render_view_host);
@@ -263,6 +263,13 @@ class TabHelper : public content::WebContentsObserver,
   // from a WebContents.
   WebAppAction pending_web_app_action_;
 
+  // Which page id was active when the GetApplicationInfo request was sent, for
+  // verification when the reply returns.
+  int32 last_committed_page_id_;
+
+  // Whether to trigger an update when the page load completes.
+  bool update_shortcut_on_load_complete_;
+
   content::NotificationRegistrar registrar_;
 
   scoped_ptr<ScriptExecutor> script_executor_;
diff --git a/chrome/browser/ui/extensions/application_launch.cc b/chrome/browser/ui/extensions/application_launch.cc
index a5e9cc17..e6470bb 100644
--- a/chrome/browser/ui/extensions/application_launch.cc
+++ b/chrome/browser/ui/extensions/application_launch.cc
@@ -457,14 +457,7 @@ WebContents* OpenAppShortcutWindow(Profile* profile,
   if (!tab)
     return NULL;
 
-  // Set UPDATE_SHORTCUT as the pending web app action. This action is picked
-  // up in LoadingStateChanged to schedule a GetApplicationInfo. And when
-  // the web app info is available, extensions::TabHelper notifies Browser via
-  // OnDidGetApplicationInfo, which calls
-  // web_app::UpdateShortcutForTabContents when it sees UPDATE_SHORTCUT as
-  // pending web app action.
-  extensions::TabHelper::FromWebContents(tab)->set_pending_web_app_action(
-      extensions::TabHelper::UPDATE_SHORTCUT);
+  extensions::TabHelper::FromWebContents(tab)->UpdateShortcutOnLoadComplete();
 
   return tab;
 }
diff --git a/chrome/browser/web_applications/web_app_unittest.cc b/chrome/browser/web_applications/web_app_unittest.cc
index b75640e..5bc608d 100644
--- a/chrome/browser/web_applications/web_app_unittest.cc
+++ b/chrome/browser/web_applications/web_app_unittest.cc
@@ -42,8 +42,7 @@ TEST_F(WebApplicationTest, GetShortcutInfoForTab) {
   web_app_info.app_url = url;
 
   RenderViewHostTester::TestOnMessageReceived(
-      rvh(),
-      ChromeExtensionHostMsg_DidGetApplicationInfo(0, 0, web_app_info));
+      rvh(), ChromeExtensionHostMsg_DidGetApplicationInfo(0, web_app_info));
   web_app::ShortcutInfo info;
   web_app::GetShortcutInfoForTab(web_contents(), &info);
 
diff --git a/chrome/common/extensions/chrome_extension_messages.h b/chrome/common/extensions/chrome_extension_messages.h
index 2372412..8b2fb7a 100644
--- a/chrome/common/extensions/chrome_extension_messages.h
+++ b/chrome/common/extensions/chrome_extension_messages.h
@@ -35,8 +35,7 @@ IPC_STRUCT_TRAITS_END()
 
 // Requests application info for the page. The renderer responds back with
 // ExtensionHostMsg_DidGetApplicationInfo.
-IPC_MESSAGE_ROUTED1(ChromeExtensionMsg_GetApplicationInfo,
-                    int32 /* page_id */)
+IPC_MESSAGE_ROUTED0(ChromeExtensionMsg_GetApplicationInfo)
 
 // Sent by the renderer to implement chrome.webstore.install().
 IPC_MESSAGE_ROUTED5(ExtensionHostMsg_InlineWebstoreInstall,
@@ -65,6 +64,5 @@ IPC_MESSAGE_ROUTED3(ExtensionMsg_InlineWebstoreInstallResponse,
 
 // Messages sent from the renderer to the browser.
 
-IPC_MESSAGE_ROUTED2(ChromeExtensionHostMsg_DidGetApplicationInfo,
-                    int32 /* page_id */,
+IPC_MESSAGE_ROUTED1(ChromeExtensionHostMsg_DidGetApplicationInfo,
                     WebApplicationInfo)
diff --git a/chrome/renderer/extensions/chrome_extension_helper.cc b/chrome/renderer/extensions/chrome_extension_helper.cc
index df7c4b2..479d1d0 100644
--- a/chrome/renderer/extensions/chrome_extension_helper.cc
+++ b/chrome/renderer/extensions/chrome_extension_helper.cc
@@ -34,13 +34,11 @@ bool ChromeExtensionHelper::OnMessageReceived(
   return handled;
 }
 
-void ChromeExtensionHelper::OnGetApplicationInfo(int page_id) {
+void ChromeExtensionHelper::OnGetApplicationInfo() {
   WebApplicationInfo app_info;
-  if (page_id == render_view()->GetPageId()) {
-    base::string16 error;
-    web_apps::ParseWebAppFromWebDocument(
-        render_view()->GetWebView()->mainFrame(), &app_info, &error);
-  }
+  base::string16 error;
+  web_apps::ParseWebAppFromWebDocument(
+      render_view()->GetWebView()->mainFrame(), &app_info, &error);
 
   // Prune out any data URLs in the set of icons.  The browser process expects
   // any icon with a data URL to have originated from a favicon.  We don't want
@@ -53,8 +51,8 @@ void ChromeExtensionHelper::OnGetApplicationInfo(int page_id) {
     }
   }
 
-  Send(new ChromeExtensionHostMsg_DidGetApplicationInfo(
-      routing_id(), page_id, app_info));
+  Send(
+      new ChromeExtensionHostMsg_DidGetApplicationInfo(routing_id(), app_info));
 }
 
 }  // namespace extensions
diff --git a/chrome/renderer/extensions/chrome_extension_helper.h b/chrome/renderer/extensions/chrome_extension_helper.h
index e26ccda..2ec4478 100644
--- a/chrome/renderer/extensions/chrome_extension_helper.h
+++ b/chrome/renderer/extensions/chrome_extension_helper.h
@@ -28,7 +28,7 @@ class ChromeExtensionHelper
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
 
   // IPC message handlers.
-  void OnGetApplicationInfo(int page_id);
+  void OnGetApplicationInfo();
 
   // The app info that we are processing. This is used when installing an app
   // via application definition. The in-progress web app is stored here while