Revert 148074 - Pass the render process id to the FrameNavigationState.

It's used in addition to frame ids to identify frames correctly.

Also expose the processId via webNavigation.getFrame and getAllFrames

BUG=136090
TEST=ExtensionApiTest.WebNavigationGetFrame

Review URL: https://chromiumcodereview.appspot.com/10805066

TBR=jochen@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10808097

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

chrome/browser/extensions/api/web_navigation/frame_navigation_state.cc

@@ -24,51 +24,16 @@ const char* kValidSchemes[] = {
 
 }  // namespace
 
-FrameNavigationState::FrameID::FrameID()
-    : frame_num(-1),
-      render_process_id(-1) {
-}
-
-FrameNavigationState::FrameID::FrameID(int64 frame_num,
-                                       int render_process_id)
-    : frame_num(frame_num),
-      render_process_id(render_process_id) {
-}
-
-FrameNavigationState::FrameID::~FrameID() {}
-
-bool FrameNavigationState::FrameID::operator<(
-    const FrameNavigationState::FrameID& other) const {
-  return frame_num < other.frame_num ||
-      (frame_num == other.frame_num &&
-       render_process_id < other.render_process_id);
-}
-
-bool FrameNavigationState::FrameID::operator==(
-    const FrameNavigationState::FrameID& other) const {
-  return frame_num == other.frame_num &&
-      render_process_id == other.render_process_id;
-}
-
-bool FrameNavigationState::FrameID::operator!=(
-    const FrameNavigationState::FrameID& other) const {
-  return !(*this == other);
-}
-
-// static
-FrameNavigationState::FrameID FrameNavigationState::kInvalidFrameID =
-    FrameNavigationState::FrameID(-1, -1);
-
 // static
 bool FrameNavigationState::allow_extension_scheme_ = false;
 
 FrameNavigationState::FrameNavigationState()
-    : main_frame_id_(kInvalidFrameID) {
+    : main_frame_id_(-1) {
 }
 
 FrameNavigationState::~FrameNavigationState() {}
 
-bool FrameNavigationState::CanSendEvents(FrameID frame_id) const {
+bool FrameNavigationState::CanSendEvents(int64 frame_id) const {
   FrameIdToStateMap::const_iterator frame_state =
       frame_state_map_.find(frame_id);
   if (frame_state == frame_state_map_.end() ||
@@ -91,7 +56,7 @@ bool FrameNavigationState::IsValidUrl(const GURL& url) const {
   return false;
 }
 
-void FrameNavigationState::TrackFrame(FrameID frame_id,
+void FrameNavigationState::TrackFrame(int64 frame_id,
                                       const GURL& url,
                                       bool is_main_frame,
                                       bool is_error_page) {
@@ -112,7 +77,7 @@ void FrameNavigationState::TrackFrame(FrameID frame_id,
   frame_ids_.insert(frame_id);
 }
 
-void FrameNavigationState::UpdateFrame(FrameID frame_id, const GURL& url) {
+void FrameNavigationState::UpdateFrame(int64 frame_id, const GURL& url) {
   FrameIdToStateMap::iterator frame_state = frame_state_map_.find(frame_id);
   if (frame_state == frame_state_map_.end()) {
     NOTREACHED();
@@ -121,13 +86,13 @@ void FrameNavigationState::UpdateFrame(FrameID frame_id, const GURL& url) {
   frame_state->second.url = url;
 }
 
-bool FrameNavigationState::IsValidFrame(FrameID frame_id) const {
+bool FrameNavigationState::IsValidFrame(int64 frame_id) const {
   FrameIdToStateMap::const_iterator frame_state =
       frame_state_map_.find(frame_id);
   return (frame_state != frame_state_map_.end());
 }
 
-GURL FrameNavigationState::GetUrl(FrameID frame_id) const {
+GURL FrameNavigationState::GetUrl(int64 frame_id) const {
   FrameIdToStateMap::const_iterator frame_state =
       frame_state_map_.find(frame_id);
   if (frame_state == frame_state_map_.end()) {
@@ -137,56 +102,56 @@ GURL FrameNavigationState::GetUrl(FrameID frame_id) const {
   return frame_state->second.url;
 }
 
-bool FrameNavigationState::IsMainFrame(FrameID frame_id) const {
-  return main_frame_id_ != kInvalidFrameID && main_frame_id_ == frame_id;
+bool FrameNavigationState::IsMainFrame(int64 frame_id) const {
+  return main_frame_id_ != -1 && main_frame_id_ == frame_id;
 }
 
-FrameNavigationState::FrameID FrameNavigationState::GetMainFrameID() const {
+int64 FrameNavigationState::GetMainFrameID() const {
   return main_frame_id_;
 }
 
-void FrameNavigationState::SetErrorOccurredInFrame(FrameID frame_id) {
+void FrameNavigationState::SetErrorOccurredInFrame(int64 frame_id) {
   DCHECK(frame_state_map_.find(frame_id) != frame_state_map_.end());
   frame_state_map_[frame_id].error_occurred = true;
 }
 
-bool FrameNavigationState::GetErrorOccurredInFrame(FrameID frame_id) const {
+bool FrameNavigationState::GetErrorOccurredInFrame(int64 frame_id) const {
   FrameIdToStateMap::const_iterator frame_state =
       frame_state_map_.find(frame_id);
   return (frame_state == frame_state_map_.end() ||
           frame_state->second.error_occurred);
 }
 
-void FrameNavigationState::SetNavigationCompleted(FrameID frame_id) {
+void FrameNavigationState::SetNavigationCompleted(int64 frame_id) {
   DCHECK(frame_state_map_.find(frame_id) != frame_state_map_.end());
   frame_state_map_[frame_id].is_navigating = false;
 }
 
-bool FrameNavigationState::GetNavigationCompleted(FrameID frame_id) const {
+bool FrameNavigationState::GetNavigationCompleted(int64 frame_id) const {
   FrameIdToStateMap::const_iterator frame_state =
       frame_state_map_.find(frame_id);
   return (frame_state == frame_state_map_.end() ||
           !frame_state->second.is_navigating);
 }
 
-void FrameNavigationState::SetNavigationCommitted(FrameID frame_id) {
+void FrameNavigationState::SetNavigationCommitted(int64 frame_id) {
   DCHECK(frame_state_map_.find(frame_id) != frame_state_map_.end());
   frame_state_map_[frame_id].is_committed = true;
 }
 
-bool FrameNavigationState::GetNavigationCommitted(FrameID frame_id) const {
+bool FrameNavigationState::GetNavigationCommitted(int64 frame_id) const {
   FrameIdToStateMap::const_iterator frame_state =
       frame_state_map_.find(frame_id);
   return (frame_state != frame_state_map_.end() &&
           frame_state->second.is_committed);
 }
 
-void FrameNavigationState::SetIsServerRedirected(FrameID frame_id) {
+void FrameNavigationState::SetIsServerRedirected(int64 frame_id) {
   DCHECK(frame_state_map_.find(frame_id) != frame_state_map_.end());
   frame_state_map_[frame_id].is_server_redirected = true;
 }
 
-bool FrameNavigationState::GetIsServerRedirected(FrameID frame_id) const {
+bool FrameNavigationState::GetIsServerRedirected(int64 frame_id) const {
   FrameIdToStateMap::const_iterator frame_state =
       frame_state_map_.find(frame_id);
   return (frame_state != frame_state_map_.end() &&

chrome/browser/extensions/api/web_navigation/frame_navigation_state.h

@@ -18,24 +18,7 @@ namespace extensions {
 // occurred so no further events for this frame are being sent.
 class FrameNavigationState {
  public:
-  // A frame is uniquely identified by its frame ID and the render process ID.
-  // We use the render process ID instead of e.g. a pointer to the RVH so we
-  // don't depend on the lifetime of the RVH.
-  struct FrameID {
-    FrameID();
-    FrameID(int64 frame_num, int render_process_id);
-    ~FrameID();
-
-    bool operator<(const FrameID& other) const;
-    bool operator==(const FrameID& other) const;
-    bool operator!=(const FrameID& other) const;
-
-    int64 frame_num;
-    int render_process_id;
-  };
-  typedef std::set<FrameID>::const_iterator const_iterator;
-
-  static FrameID kInvalidFrameID;
+  typedef std::set<int64>::const_iterator const_iterator;
 
   FrameNavigationState();
   ~FrameNavigationState();
@@ -45,59 +28,59 @@ class FrameNavigationState {
   const_iterator end() const { return frame_ids_.end(); }
 
   // True if navigation events for the given frame can be sent.
-  bool CanSendEvents(FrameID frame_id) const;
+  bool CanSendEvents(int64 frame_id) const;
 
   // True if in general webNavigation events may be sent for the given URL.
   bool IsValidUrl(const GURL& url) const;
 
   // Starts to track a frame identified by its |frame_id| showing the URL |url|.
-  void TrackFrame(FrameID frame_id,
+  void TrackFrame(int64 frame_id,
                   const GURL& url,
                   bool is_main_frame,
                   bool is_error_page);
 
   // Update the URL associated with a given frame.
-  void UpdateFrame(FrameID frame_id, const GURL& url);
+  void UpdateFrame(int64 frame_id, const GURL& url);
 
   // Returns true if |frame_id| is a known frame.
-  bool IsValidFrame(FrameID frame_id) const;
+  bool IsValidFrame(int64 frame_id) const;
 
   // Returns the URL corresponding to a tracked frame given by its |frame_id|.
-  GURL GetUrl(FrameID frame_id) const;
+  GURL GetUrl(int64 frame_id) const;
 
   // True if the frame given by its |frame_id| is the main frame of its tab.
-  bool IsMainFrame(FrameID frame_id) const;
+  bool IsMainFrame(int64 frame_id) const;
 
   // Returns the frame ID of the main frame, or -1 if the frame ID is not
   // known.
-  FrameID GetMainFrameID() const;
+  int64 GetMainFrameID() const;
 
   // Marks a frame as in an error state, i.e. the onErrorOccurred event was
   // fired for this frame, and no further events should be sent for it.
-  void SetErrorOccurredInFrame(FrameID frame_id);
+  void SetErrorOccurredInFrame(int64 frame_id);
 
   // True if the frame is marked as being in an error state.
-  bool GetErrorOccurredInFrame(FrameID frame_id) const;
+  bool GetErrorOccurredInFrame(int64 frame_id) const;
 
   // Marks a frame as having finished its last navigation, i.e. the onCompleted
   // event was fired for this frame.
-  void SetNavigationCompleted(FrameID frame_id);
+  void SetNavigationCompleted(int64 frame_id);
 
   // True if the frame is currently not navigating.
-  bool GetNavigationCompleted(FrameID frame_id) const;
+  bool GetNavigationCompleted(int64 frame_id) const;
 
   // Marks a frame as having committed its navigation, i.e. the onCommitted
   // event was fired for this frame.
-  void SetNavigationCommitted(FrameID frame_id);
+  void SetNavigationCommitted(int64 frame_id);
 
   // True if the frame has committed its navigation.
-  bool GetNavigationCommitted(FrameID frame_id) const;
+  bool GetNavigationCommitted(int64 frame_id) const;
 
   // Marks a frame as redirected by the server.
-  void SetIsServerRedirected(FrameID frame_id);
+  void SetIsServerRedirected(int64 frame_id);
 
   // True if the frame was redirected by the server.
-  bool GetIsServerRedirected(FrameID frame_id) const;
+  bool GetIsServerRedirected(int64 frame_id) const;
 
 #ifdef UNIT_TEST
   static void set_allow_extension_scheme(bool allow_extension_scheme) {
@@ -114,16 +97,16 @@ class FrameNavigationState {
     bool is_server_redirected;  // True if a server redirect happened.
     GURL url;  // URL of this frame.
   };
-  typedef std::map<FrameID, FrameState> FrameIdToStateMap;
+  typedef std::map<int64, FrameState> FrameIdToStateMap;
 
   // Tracks the state of known frames.
   FrameIdToStateMap frame_state_map_;
 
   // Set of all known frames.
-  std::set<FrameID> frame_ids_;
+  std::set<int64> frame_ids_;
 
   // The current main frame.
-  FrameID main_frame_id_;
+  int64 main_frame_id_;
 
   // If true, also allow events from chrome-extension:// URLs.
   static bool allow_extension_scheme_;

chrome/browser/extensions/api/web_navigation/frame_navigation_state_unittest.cc

@@ -17,8 +17,8 @@ class FrameNavigationStateTest : public ChromeRenderViewHostTestHarness {
 // goes away.
 TEST_F(FrameNavigationStateTest, TrackFrame) {
   FrameNavigationState navigation_state;
-  const FrameNavigationState::FrameID frame_id1(23, 1);
-  const FrameNavigationState::FrameID frame_id2(42, 1);
+  const int64 frame_id1 = 23;
+  const int64 frame_id2 = 42;
   const GURL url1("http://www.google.com/");
   const GURL url2("http://mail.google.com/");
 
@@ -48,7 +48,7 @@ TEST_F(FrameNavigationStateTest, TrackFrame) {
 // before a new navigation happened in this frame.
 TEST_F(FrameNavigationStateTest, ErrorState) {
   FrameNavigationState navigation_state;
-  const FrameNavigationState::FrameID frame_id(42, 1);
+  const int64 frame_id = 42;
   const GURL url("http://www.google.com/");
 
   navigation_state.TrackFrame(frame_id, url, true, false);
@@ -75,8 +75,8 @@ TEST_F(FrameNavigationStateTest, ErrorState) {
 // before a new navigation happened in this frame.
 TEST_F(FrameNavigationStateTest, ErrorStateFrame) {
   FrameNavigationState navigation_state;
-  const FrameNavigationState::FrameID frame_id1(23, 1);
-  const FrameNavigationState::FrameID frame_id2(42, 1);
+  const int64 frame_id1 = 23;
+  const int64 frame_id2 = 42;
   const GURL url("http://www.google.com/");
 
   navigation_state.TrackFrame(frame_id1, url, true, false);
@@ -103,7 +103,7 @@ TEST_F(FrameNavigationStateTest, ErrorStateFrame) {
 // Tests that no events are send for a not web-safe scheme.
 TEST_F(FrameNavigationStateTest, WebSafeScheme) {
   FrameNavigationState navigation_state;
-  const FrameNavigationState::FrameID frame_id(23, 1);
+  const int64 frame_id = 23;
   const GURL url("unsafe://www.google.com/");
 
   navigation_state.TrackFrame(frame_id, url, true, false);

chrome/browser/extensions/api/web_navigation/web_navigation_api.h

@@ -50,34 +50,34 @@ class WebNavigationTabObserver : public content::NotificationObserver,
   virtual void AboutToNavigateRenderView(
       content::RenderViewHost* render_view_host) OVERRIDE;
   virtual void DidStartProvisionalLoadForFrame(
-      int64 frame_num,
+      int64 frame_id,
       bool is_main_frame,
       const GURL& validated_url,
       bool is_error_page,
       content::RenderViewHost* render_view_host) OVERRIDE;
   virtual void DidCommitProvisionalLoadForFrame(
-      int64 frame_num,
+      int64 frame_id,
       bool is_main_frame,
       const GURL& url,
       content::PageTransition transition_type,
       content::RenderViewHost* render_view_host) OVERRIDE;
   virtual void DidFailProvisionalLoad(
-      int64 frame_num,
+      int64 frame_id,
       bool is_main_frame,
       const GURL& validated_url,
       int error_code,
       const string16& error_description,
       content::RenderViewHost* render_view_host) OVERRIDE;
   virtual void DocumentLoadedInFrame(
-      int64 frame_num,
+      int64 frame_id,
       content::RenderViewHost* render_view_host) OVERRIDE;
   virtual void DidFinishLoad(
-      int64 frame_num,
+      int64 frame_id,
       const GURL& validated_url,
       bool is_main_frame,
       content::RenderViewHost* render_view_host) OVERRIDE;
   virtual void DidFailLoad(
-      int64 frame_num,
+      int64 frame_id,
       const GURL& validated_url,
       bool is_main_frame,
       int error_code,
@@ -88,14 +88,13 @@ class WebNavigationTabObserver : public content::NotificationObserver,
                                    const content::Referrer& referrer,
                                    WindowOpenDisposition disposition,
                                    content::PageTransition transition,
-                                   int64 source_frame_num) OVERRIDE;
+                                   int64 source_frame_id) OVERRIDE;
   virtual void WebContentsDestroyed(content::WebContents* tab) OVERRIDE;
 
  private:
   // True if the transition and target url correspond to a reference fragment
   // navigation.
-  bool IsReferenceFragmentNavigation(FrameNavigationState::FrameID frame_id,
-                                     const GURL& url);
+  bool IsReferenceFragmentNavigation(int64 frame_id, const GURL& url);
 
   // Tracks the state of the frames we are sending events for.
   FrameNavigationState navigation_state_;

chrome/browser/extensions/api/web_navigation/web_navigation_apitest.cc

@@ -189,7 +189,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, WebNavigationUserAction) {
   params.media_type = WebKit::WebContextMenuData::MediaTypeNone;
   params.page_url = url;
   params.frame_id = WebNavigationTabObserver::Get(tab)->
-      frame_navigation_state().GetMainFrameID().frame_num;
+      frame_navigation_state().GetMainFrameID();
   params.link_url = extension->GetResourceURL("userAction/b.html");
 
   TestRenderViewContextMenu menu(tab, params);

chrome/common/extensions/api/web_navigation.json

@@ -18,7 +18,6 @@
             "description": "Information about the frame to retrieve information about.",
             "properties": {
               "tabId": { "type": "integer", "minimum": 0, "description": "The ID of the tab in which the frame is." },
-              "processId": {"type": "integer", "description": "The ID of the process runs the renderer for this tab."},
               "frameId": { "type": "integer", "minimum": 0, "description": "The ID of the frame in the given tab." }
             }
           },
@@ -71,10 +70,6 @@
                       "type": "boolean",
                       "description": "True if the last navigation in this frame was interrupted by an error, i.e. the onErrorOccurred event fired."
                     },
-                    "processId": {
-                      "type": "integer",
-                      "description": "The ID of the process runs the renderer for this tab."
-                    },
                     "frameId": {
                       "type": "integer",
                       "description": "The ID of the frame. 0 indicates that this is the main frame; a positive value indicates the ID of a subframe."

chrome/common/extensions/docs/extensions/webNavigation.html

@@ -592,29 +592,6 @@ The following transition qualifiers exist:
           <!-- OBJECT METHODS -->
           <!-- OBJECT EVENT FIELDS -->
           <!-- FUNCTION PARAMETERS -->
-        </div>
-              </div><div>
-                <div>
-          <dt>
-            <var>processId</var>
-              <em>
-                <!-- TYPE -->
-                <div style="display:inline">
-                  (
-                    <span id="typeTemplate">
-                      <span>
-                        <span>integer</span>
-                      </span>
-                    </span>
-                  )
-                </div>
-              </em>
-          </dt>
-          <dd>The ID of the process runs the renderer for this tab.</dd>
-          <!-- OBJECT PROPERTIES -->
-          <!-- OBJECT METHODS -->
-          <!-- OBJECT EVENT FIELDS -->
-          <!-- FUNCTION PARAMETERS -->
         </div>
               </div><div>
                 <div>
@@ -731,29 +708,6 @@ The following transition qualifiers exist:
           <!-- OBJECT METHODS -->
           <!-- OBJECT EVENT FIELDS -->
           <!-- FUNCTION PARAMETERS -->
-        </div>
-              </div><div>
-                <div>
-          <dt>
-            <var>processId</var>
-              <em>
-                <!-- TYPE -->
-                <div style="display:inline">
-                  (
-                    <span id="typeTemplate">
-                      <span>
-                        <span>integer</span>
-                      </span>
-                    </span>
-                  )
-                </div>
-              </em>
-          </dt>
-          <dd>The ID of the process runs the renderer for this tab.</dd>
-          <!-- OBJECT PROPERTIES -->
-          <!-- OBJECT METHODS -->
-          <!-- OBJECT EVENT FIELDS -->
-          <!-- FUNCTION PARAMETERS -->
         </div>
               </div><div>
                 <div>

chrome/test/data/extensions/api_test/webnavigation/test_getFrame.html

 <script>
 var URL = chrome.extension.getURL("getFrame/a.html");
 var tabId = -1;
-var processId = -1;
 
 chrome.test.runTests([
   function testGetFrame() {
@@ -14,9 +13,8 @@ chrome.test.runTests([
             return;
           if (details.url != URL)
             return;
-          processId = details.processId;
           chrome.webNavigation.getFrame(
-              {tabId: tabId, frameId: 0, processId: processId},
+              {tabId: tabId, frameId: 0},
               function(details) {
             chrome.test.assertEq({errorOccurred: false, url: URL},
                                  details);
@@ -29,7 +27,7 @@ chrome.test.runTests([
 
   function testGetInvalidFrame() {
     chrome.webNavigation.getFrame(
-        {tabId: tabId, frameId: 1, processId: processId},
+        {tabId: tabId, frameId: 1},
       function (details) {
         chrome.test.assertEq(null, details);
         chrome.test.succeed();
@@ -40,12 +38,8 @@ chrome.test.runTests([
     chrome.webNavigation.getAllFrames(
         {tabId: tabId},
       function (details) {
-        chrome.test.assertEq(
-            [{errorOccurred: false,
-              frameId: 0,
-              processId: processId,
-              url: URL}],
-             details);
+        chrome.test.assertEq([{errorOccurred: false, frameId: 0, url: URL}],
+                             details);
         chrome.test.succeed();
     });
   }