OOPIFs: Transitioning Get/Send...SavableResourceLinks away from RenderViewHost.

content/browser/download/save_package.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 CONTENT_BROWSER_DOWNLOAD_SAVE_PACKAGE_H_
 #define CONTENT_BROWSER_DOWNLOAD_SAVE_PACKAGE_H_
 
 #include <queue>
 #include <set>
 #include <string>
@@ skipping 129 matching lines @@
   // Notes from Init() above applies here as well.
   void InternalInit();
 
   void Stop();
   void CheckFinish();
   void SaveNextFile(bool process_all_remainder_items);
   void DoSavingProcess();
 
   // WebContentsObserver implementation.
   bool OnMessageReceived(const IPC::Message& message) override;
+  bool OnMessageReceived(const IPC::Message& message,
+                         RenderFrameHost* render_frame_host) override;
 
   // DownloadItem::Observer implementation.
   void OnDownloadDestroyed(DownloadItem* download) override;
 
   // Update the download history of this item upon completion.
   void FinalizeDownloadEntry();
 
   // Detach from DownloadManager.
   void StopObservation();
 
@@ skipping 10 matching lines @@
       base::FilePath::StringType* pure_file_name);
 
   // Create a file name based on the response from the server.
   bool GenerateFileName(const std::string& disposition,
                         const GURL& url,
                         bool need_html_ext,
                         base::FilePath::StringType* generated_name);
 
   // Get all savable resource links from current web page, include main
   // frame and sub-frame.
-  void GetAllSavableResourceLinksForCurrentPage();
+  void GetSavableResourceLinksForCurrentPage();
+  void GetSavableResourceLinksForFrame(RenderFrameHost*);

[nasko, 2015/09/02 23:45:55]

Parameters need names.

[Łukasz Anforowicz, 2015/09/03 16:59:57]

Initially I wanted to push back on this feedback (language allows no parameter
names in declarations + I felt that the parameter name [thinking about
"render_frame_host"] would be redundant + I don't see anything in the style
guide that would talk about this).  OTOH, I guess there is a better name than
"render_frame_host" [i.e. "target" conveys some info beyond the type] and I can
go forward with this.  So - done here (but then "broken" in declaration of
OnMessageReceived).

[nasko, 2015/09/03 18:17:02]

From the Google C++ style guide:

"All parameters should be named, with identical names in the declaration and
implementation."

[Łukasz Anforowicz, 2015/09/03 19:41:24]

Ok...

+  void OnReceivedNonSavableFrameIndication(RenderFrameHost* render_frame_host);

[nasko, 2015/09/02 23:45:55]

The usual pattern for naming IPC message handlers is On(IPC message name,
without the prefix). In this case it will be OnNonSavableResponse.

Also, ordering should be the same between header and .cc file.

[Łukasz Anforowicz, 2015/09/03 16:59:57]

Done.

+  void OnReceivedSavableResourceLinksForFrame(
+      RenderFrameHost* render_frame_host,
+      const std::vector<GURL>& resources_list,
+      const std::vector<Referrer>& referrers_list);
+  void CompleteSavableResourceLinksResponseFromFrame();
+  std::set<GURL> unique_urls_to_save_;  // Used to de-dupe urls.

[nasko, 2015/09/02 23:45:55]

Member variables go down in the class, after all methods.

[Łukasz Anforowicz, 2015/09/03 16:59:57]

Done.

+  int number_of_frames_with_pending_get_savable_resource_links_;
+
   // Get html data by serializing all frames of current page with lists
   // which contain all resource links that have local copy.
   void GetSerializedHtmlDataForCurrentPageWithLocalLinks();
 
   // Look up SaveItem by save id from in progress map.
   SaveItem* LookupItemInProcessBySaveId(int32 save_id);
 
   // Remove SaveItem from in progress map and put it to saved map.
   void PutInProgressItemToSavedMap(SaveItem* save_item);
 
   // Retrieves the URL to be saved from the WebContents.
   GURL GetUrlToBeSaved();
 
   void CreateDirectoryOnFileThread(const base::FilePath& website_save_dir,
                                    const base::FilePath& download_save_dir,
                                    bool skip_dir_check,
                                    const std::string& mime_type,
                                    const std::string& accept_langs);
   void ContinueGetSaveInfo(const base::FilePath& suggested_path,
                            bool can_save_as_complete);
   void OnPathPicked(
       const base::FilePath& final_name,
       SavePageType type,
       const SavePackageDownloadCreatedCallback& cb);
-  void OnReceivedSavableResourceLinksForCurrentPage(
-      const std::vector<GURL>& resources_list,
-      const std::vector<Referrer>& referrers_list,
-      const std::vector<GURL>& frames_list);
 
   void OnReceivedSerializedHtmlData(const GURL& frame_url,
                                     const std::string& data,
                                     int32 status);
 
   typedef base::hash_map<std::string, SaveItem*> SaveUrlItemMap;
   // in_progress_items_ is map of all saving job in in-progress state.
   SaveUrlItemMap in_progress_items_;
   // saved_failed_items_ is map of all saving job which are failed.
   SaveUrlItemMap saved_failed_items_;
@@ skipping 112 matching lines @@
   friend class SavePackageTest;
   FRIEND_TEST_ALL_PREFIXES(SavePackageTest, TestSuggestedSaveNames);
   FRIEND_TEST_ALL_PREFIXES(SavePackageTest, TestLongSafePureFilename);
 
   DISALLOW_COPY_AND_ASSIGN(SavePackage);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_DOWNLOAD_SAVE_PACKAGE_H_