Clarify that WebContents::GetLastCommittedURL returns the virtual URL.

Clarify that WebContents::GetLastCommittedURL returns the virtual URL.

NavigationHandle::GetURL is not necessarily consistent with
WebContents::GetLastCommittedURL, as the URL being navigated to may be
changed for display in the URL bar. For example, when viewing the page
source, NavigationHandle::GetURL returns the page's URL, but
WebContents::GetLastCommittedURL returns the URL prefixed with
"view-source:". This CL notes the difference in the header comments for
both methods.

BUG=618553
Committed: https://crrev.com/319d693f722a8bc60ca52b6735b8efae7875d5d0
Cr-Commit-Position: refs/heads/master@{#400857}

[dominickn, 2016-06-17 08:53:03]

dominickn@chromium.org changed reviewers:
+ creis@chromium.org

[dominickn, 2016-06-17 08:53:04]

Hi Charlie, PTAL thanks! This is a minor header comment update.

[Charlie Reis, 2016-06-20 17:33:06]

Thanks for getting to this, and sorry for not having a chance to take a look
while we were at BlinkOn!  Some suggestions below, but otherwise LGTM.

https://codereview.chromium.org/2076063002/diff/1/content/public/browser/navi...
File content/public/browser/navigation_handle.h (right):

https://codereview.chromium.org/2076063002/diff/1/content/public/browser/navi...
content/public/browser/navigation_handle.h:44: // virtual URL is prefixed with
"view-source".
nit: Add colon to "view-source:"

https://codereview.chromium.org/2076063002/diff/1/content/public/browser/web_...
File content/public/browser/web_contents.h (right):

https://codereview.chromium.org/2076063002/diff/1/content/public/browser/web_...
content/public/browser/web_contents.h:213: // Gets the last committed URL. This
is the URL that would be displayed in the
We probably want to be more explicit about the difference between "visible,"
"virtual," and "displayed," relative to GetVisibleURL.

Instead, let's say:
Gets the virtual URL of the last committed page in this WebContents.  Virtual
URLs are meant to be displayed to the user (e.g., they include the
"view-source:" prefix for view source URLs, unlike NavigationEntry::GetURL). 
The last committed page is the current security context and the content that is
actually displayed within the tab.  See also GetVisibleURL above, which may
differ from this URL.

And let's update the beginning of the GetVisibleURL comment to say:
Gets the virtual URL currently being displayed...

[dominickn, 2016-06-20 23:56:06]

Thanks!

https://codereview.chromium.org/2076063002/diff/1/content/public/browser/navi...
File content/public/browser/navigation_handle.h (right):

https://codereview.chromium.org/2076063002/diff/1/content/public/browser/navi...
content/public/browser/navigation_handle.h:44: // virtual URL is prefixed with
"view-source".
On 2016/06/20 17:33:06, Charlie Reis wrote:
> nit: Add colon to "view-source:"

Done.

https://codereview.chromium.org/2076063002/diff/1/content/public/browser/web_...
File content/public/browser/web_contents.h (right):

https://codereview.chromium.org/2076063002/diff/1/content/public/browser/web_...
content/public/browser/web_contents.h:213: // Gets the last committed URL. This
is the URL that would be displayed in the
On 2016/06/20 17:33:06, Charlie Reis wrote:
> We probably want to be more explicit about the difference between "visible,"
> "virtual," and "displayed," relative to GetVisibleURL.
> 
> Instead, let's say:
> Gets the virtual URL of the last committed page in this WebContents.  Virtual
> URLs are meant to be displayed to the user (e.g., they include the
> "view-source:" prefix for view source URLs, unlike NavigationEntry::GetURL). 
> The last committed page is the current security context and the content that
is
> actually displayed within the tab.  See also GetVisibleURL above, which may
> differ from this URL.
> 
> And let's update the beginning of the GetVisibleURL comment to say:
> Gets the virtual URL currently being displayed...

Done.

[dominickn, 2016-06-20 23:56:11]

The CQ bit was checked by dominickn@chromium.org

[dominickn, 2016-06-20 23:56:11]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org
Link to the patchset: https://codereview.chromium.org/2076063002/#ps20001
(title: "Address comments")

[commit-bot: I haz the power, 2016-06-20 23:56:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2076063002/20001

[commit-bot: I haz the power, 2016-06-21 00:59:43]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-21 01:02:25]

Description was changed from

==========
Clarify that WebContents::GetLastCommittedURL returns the virtual URL.

NavigationHandle::GetURL is not necessarily consistent with
WebContents::GetLastCommittedURL, as the URL being navigated to may be
changed for display in the URL bar. For example, when viewing the page
source, NavigationHandle::GetURL returns the page's URL, but
WebContents::GetLastCommittedURL returns the URL prefixed with
"view-source:". This CL notes the difference in the header comments for
both methods.

BUG=618553
==========

to

==========
Clarify that WebContents::GetLastCommittedURL returns the virtual URL.

NavigationHandle::GetURL is not necessarily consistent with
WebContents::GetLastCommittedURL, as the URL being navigated to may be
changed for display in the URL bar. For example, when viewing the page
source, NavigationHandle::GetURL returns the page's URL, but
WebContents::GetLastCommittedURL returns the URL prefixed with
"view-source:". This CL notes the difference in the header comments for
both methods.

BUG=618553

Committed: https://crrev.com/319d693f722a8bc60ca52b6735b8efae7875d5d0
Cr-Commit-Position: refs/heads/master@{#400857}
==========

[commit-bot: I haz the power, 2016-06-21 01:02:26]

Patchset 2 (id:??) landed as
https://crrev.com/319d693f722a8bc60ca52b6735b8efae7875d5d0
Cr-Commit-Position: refs/heads/master@{#400857}