[Reading List iOS] Display distilled URL instead of entry URL in omnibox

ios/chrome/browser/reading_list/offline_url_utils.h

Index: ios/chrome/browser/reading_list/offline_url_utils.h
diff --git a/ios/chrome/browser/reading_list/offline_url_utils.h b/ios/chrome/browser/reading_list/offline_url_utils.h
index c2c0dcc20924ca870fb1d8fe87071a1306828f68..178f0bc3e39a55074faa970b027c5fd022890e0e 100644
--- a/ios/chrome/browser/reading_list/offline_url_utils.h
+++ b/ios/chrome/browser/reading_list/offline_url_utils.h
@@ -14,11 +14,19 @@
 namespace reading_list {
 
 // The distilled URL chrome://offline/... that will load the file at |path|.
-GURL DistilledURLForPath(const base::FilePath& path, const GURL& virtual_url);
-
-// If |distilled_url| has a query "virtualURL" query params that is a URL,
-// returns it. If not, return |distilled_url|
-GURL VirtualURLForDistilledURL(const GURL& distilled_url);
+// |entry_url| is the URL of the ReadingListEntry.
+// |virtual_url| is the URL to display in the omnibox.
+GURL OfflineURLForPath(const base::FilePath& distilled_path,

[gambard, 2017/01/20 14:49:34]

Can you change the function name? You obscure the |entry_url| and |virtual_url|
part.

[jif, 2017/01/20 15:01:25]

Consider explaining why |entry_url| can be different from |virtual_url|.

[Olivier, 2017/01/20 16:09:14]

Done.

[Olivier, 2017/01/20 16:09:14]

C++ function names don't always include parameters and distilled_path is the
only mandatory one.
I think the name reflects what the function do, give a chrome://offline URL that
allows to open |distilled_path|.

+                       const GURL& entry_url,
+                       const GURL& virtual_url);
+
+// If |offline_url| has a "virtualURL" query params that is a URL, returns it.
+// If not, return |EntryURLForOfflineURL(|offline_url|)|
+GURL VirtualURLForOfflineURL(const GURL& offline_url);
+
+// If |offline_url| has a "entryURL" query params that is a URL, returns it.
+// If not, return |offline_url|
+GURL EntryURLForOfflineURL(const GURL& offline_url);

[gambard, 2017/01/20 14:49:34]

Can you please set the same function order in .h and .cc?

[Olivier, 2017/01/20 16:09:14]

Done.

 
 // The file URL pointing to the local file to load to display |distilled_url|.
 // If |resources_root_url| is not nullptr, it is set to a file URL to the