Renamed NavigationHandle::IsSamePage to NavigationHandle::IsSameDocument

Renamed NavigationHandle::IsSamePage to NavigationHandle::IsSameDocument

Old name was presumable used to match was_within_same_page IPC, but
IsSameDocument name better reflects the the type of the navigation
(navigation did not change the document object).

This change will make naming more consistent with WebKit (which has
SameDocumentNavigationSource class) and Chrome for iOS, which uses
SameDocument term.

BUG=695189
TBR=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2716493004
Cr-Commit-Position: refs/heads/master@{#455106}
Committed: https://chromium.googlesource.com/chromium/src/+/a11672fb4c998040224d7dc97e5bdc05d46a4145

[Eugene But (OOO till 7-30), 2017-02-23 19:03:05]

Description was changed from

==========
Renamed NavigationHandle::IsSamePage to NavigationHandle::IsSameDocument

Old name was presumable used to match was_within_same_page IPC, but
IsSameDocument name better reflects the the type of the navigation
(navigation did not change the document object).

This change will make naming more consistent with WebKit (which has
SameDocumentNavigationSource class) and Chrome for iOS, which uses
SameDocument term.

BUG=None
==========

to

==========
Renamed NavigationHandle::IsSamePage to NavigationHandle::IsSameDocument

Old name was presumable used to match was_within_same_page IPC, but
IsSameDocument name better reflects the the type of the navigation
(navigation did not change the document object).

This change will make naming more consistent with WebKit (which has
SameDocumentNavigationSource class) and Chrome for iOS, which uses
SameDocument term.

BUG=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Eugene But (OOO till 7-30), 2017-02-23 19:03:44]

The CQ bit was checked by eugenebut@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 19:04:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Eugene But (OOO till 7-30), 2017-02-23 19:57:24]

The CQ bit was checked by eugenebut@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 19:58:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Eugene But (OOO till 7-30), 2017-02-23 21:24:27]

eugenebut@chromium.org changed reviewers:
+ clamy@chromium.org

[Eugene But (OOO till 7-30), 2017-02-23 21:24:28]

clamy@ I will add more people as reviewers if this looks good to you.

[commit-bot: I haz the power, 2017-02-23 22:20:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 22:20:33]

Dry run: This issue passed the CQ dry run.

[Eugene But (OOO till 7-30), 2017-02-24 16:36:44]

Description was changed from

==========
Renamed NavigationHandle::IsSamePage to NavigationHandle::IsSameDocument

Old name was presumable used to match was_within_same_page IPC, but
IsSameDocument name better reflects the the type of the navigation
(navigation did not change the document object).

This change will make naming more consistent with WebKit (which has
SameDocumentNavigationSource class) and Chrome for iOS, which uses
SameDocument term.

BUG=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Renamed NavigationHandle::IsSamePage to NavigationHandle::IsSameDocument

Old name was presumable used to match was_within_same_page IPC, but
IsSameDocument name better reflects the the type of the navigation
(navigation did not change the document object).

This change will make naming more consistent with WebKit (which has
SameDocumentNavigationSource class) and Chrome for iOS, which uses
SameDocument term.

BUG=695189
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[clamy, 2017-02-27 15:19:28]

Thanks for the cleanup! content/ lgtm.

[Eugene But (OOO till 7-30), 2017-02-27 16:21:12]

eugenebut@chromium.org changed reviewers:
+ jam@chromium.org

[Eugene But (OOO till 7-30), 2017-02-27 16:21:13]

+jam@ for top level owners

[jam, 2017-02-27 20:24:54]

what's more consistent in content and embedders of it? searching for samepage
shows lots of hit, so it seems like we should aim for consistency there
otherwise it's confusing? and if we want to go with same document, then we
should update all the content/chrome/component usage to reflect that.

[Eugene But (OOO till 7-30), 2017-02-27 22:59:43]

On 2017/02/27 20:24:54, jam wrote:
> what's more consistent in content and embedders of it? searching for samepage
> shows lots of hit, so it seems like we should aim for consistency there
> otherwise it's confusing? and if we want to go with same document, then we
> should update all the content/chrome/component usage to reflect that.
SamePage seems like more common term in content, components and chrome code.
SameDocument is something that is used in Blink code. If you ok with renaming
this, then I can send more CLs and use SameDocument where necessary.

Thanks!

[jam, 2017-02-28 16:22:23]

On 2017/02/27 22:59:43, Eugene But wrote:
> On 2017/02/27 20:24:54, jam wrote:
> > what's more consistent in content and embedders of it? searching for
samepage
> > shows lots of hit, so it seems like we should aim for consistency there
> > otherwise it's confusing? and if we want to go with same document, then we
> > should update all the content/chrome/component usage to reflect that.
> SamePage seems like more common term in content, components and chrome code.
> SameDocument is something that is used in Blink code. If you ok with renaming
> this, then I can send more CLs and use SameDocument where necessary.
> 
> Thanks!

I find 37 hits in the codebase for samedocument/same_document, and 233 for
same_page/samepage. Given how long the samepage term has been used, I'm not sure
changing the vast majority of callsites would be an improvement.

[Eugene But (OOO till 7-30), 2017-02-28 16:49:29]

On 2017/02/28 16:22:23, jam wrote:
> On 2017/02/27 22:59:43, Eugene But wrote:
> > On 2017/02/27 20:24:54, jam wrote:
> > > what's more consistent in content and embedders of it? searching for
> samepage
> > > shows lots of hit, so it seems like we should aim for consistency there
> > > otherwise it's confusing? and if we want to go with same document, then we
> > > should update all the content/chrome/component usage to reflect that.
> > SamePage seems like more common term in content, components and chrome code.
> > SameDocument is something that is used in Blink code. If you ok with
renaming
> > this, then I can send more CLs and use SameDocument where necessary.
> > 
> > Thanks!
> 
> I find 37 hits in the codebase for samedocument/same_document, and 233 for
> same_page/samepage. Given how long the samepage term has been used, I'm not
sure
> changing the vast majority of callsites would be an improvement.
Thanks, John.

Do you think people got used to "SamePage" so much that renaming will bring more
confusion than clarity?
Or this because you don't think that "SameDocument" is an improvement over
"SamePage"?

If we both agree that SameDocument is a better name then renaming will be a
positive thing in the long term run.

[Charlie Reis, 2017-02-28 17:27:43]

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

[Charlie Reis, 2017-02-28 17:32:29]

On 2017/02/28 16:49:29, Eugene But wrote:
> On 2017/02/28 16:22:23, jam wrote:
> > On 2017/02/27 22:59:43, Eugene But wrote:
> > > On 2017/02/27 20:24:54, jam wrote:
> > > > what's more consistent in content and embedders of it? searching for
> > samepage
> > > > shows lots of hit, so it seems like we should aim for consistency there
> > > > otherwise it's confusing? and if we want to go with same document, then
we
> > > > should update all the content/chrome/component usage to reflect that.
> > > SamePage seems like more common term in content, components and chrome
code.
> > > SameDocument is something that is used in Blink code. If you ok with
> renaming
> > > this, then I can send more CLs and use SameDocument where necessary.
> > > 
> > > Thanks!
> > 
> > I find 37 hits in the codebase for samedocument/same_document, and 233 for
> > same_page/samepage. Given how long the samepage term has been used, I'm not
> sure
> > changing the vast majority of callsites would be an improvement.
> Thanks, John.
> 
> Do you think people got used to "SamePage" so much that renaming will bring
more
> confusion than clarity?
> Or this because you don't think that "SameDocument" is an improvement over
> "SamePage"?
> 
> If we both agree that SameDocument is a better name then renaming will be a
> positive thing in the long term run.

I'll put in an argument that renaming everything to SameDocument seems more
precise, and might be a worthwhile thing to converge on (despite the current
heavy use of SamePage to the contrary).  In general, "page" refers to the whole
frame tree and "document" refers to what's in a single frame, and we're
discussing what happens for a navigation in a single frame here.

I'll also note that in-page/InPage is a common term inside NavigationController,
but it means the same thing.  We should probably update that usage as well if we
go down this path.

[jam, 2017-03-01 15:33:45]

On 2017/02/28 17:32:29, Charlie Reis (slow) wrote:
> On 2017/02/28 16:49:29, Eugene But wrote:
> > On 2017/02/28 16:22:23, jam wrote:
> > > On 2017/02/27 22:59:43, Eugene But wrote:
> > > > On 2017/02/27 20:24:54, jam wrote:
> > > > > what's more consistent in content and embedders of it? searching for
> > > samepage
> > > > > shows lots of hit, so it seems like we should aim for consistency
there
> > > > > otherwise it's confusing? and if we want to go with same document,
then
> we
> > > > > should update all the content/chrome/component usage to reflect that.
> > > > SamePage seems like more common term in content, components and chrome
> code.
> > > > SameDocument is something that is used in Blink code. If you ok with
> > renaming
> > > > this, then I can send more CLs and use SameDocument where necessary.
> > > > 
> > > > Thanks!
> > > 
> > > I find 37 hits in the codebase for samedocument/same_document, and 233 for
> > > same_page/samepage. Given how long the samepage term has been used, I'm
not
> > sure
> > > changing the vast majority of callsites would be an improvement.
> > Thanks, John.
> > 
> > Do you think people got used to "SamePage" so much that renaming will bring
> more
> > confusion than clarity?
> > Or this because you don't think that "SameDocument" is an improvement over
> > "SamePage"?
> > 
> > If we both agree that SameDocument is a better name then renaming will be a
> > positive thing in the long term run.
> 
> I'll put in an argument that renaming everything to SameDocument seems more
> precise, and might be a worthwhile thing to converge on (despite the current
> heavy use of SamePage to the contrary).  In general, "page" refers to the
whole
> frame tree and "document" refers to what's in a single frame, and we're
> discussing what happens for a navigation in a single frame here.

Does that argue for calling it InFrame?

One thing that I worry about is that we're introducing another term (document);
do we really need to? 
> 
> I'll also note that in-page/InPage is a common term inside
NavigationController,
> but it means the same thing.  We should probably update that usage as well if
we
> go down this path.

[Eugene But (OOO till 7-30), 2017-03-01 16:36:49]

"document" is already an existing term described in HTML or whatever web spec.
"SameDocument" is already used in Blink, so I believe introducing it in content
and chrome is fine. I feel that InFrame is also different from SameDocument
(former is a navigation inside an iframe, and latter is a navigation which does
not change "document" object).

[clamy, 2017-03-01 16:41:50]

FWIW: I think InFrame is even more confusing. Technically, all main-frame
navigations are in frame, as the frame itself doesn't change.

We already have several WCO methods with Document in their name, so I don't
think it's weird to have SameDocument here.

I also suggested they go along with the change, as I have seen several people
misunderstand what IsSamePage means. They thought it meant a same-process
navigation as opposed to what it really is, a same-document navigation.

[nasko, 2017-03-01 17:47:12]

On 2017/03/01 16:41:50, clamy wrote:
> FWIW: I think InFrame is even more confusing. Technically, all main-frame
> navigations are in frame, as the frame itself doesn't change.
> 
> We already have several WCO methods with Document in their name, so I don't
> think it's weird to have SameDocument here.
> 
> I also suggested they go along with the change, as I have seen several people
> misunderstand what IsSamePage means. They thought it meant a same-process
> navigation as opposed to what it really is, a same-document navigation.

IMHO, "document" is the most correct concept to use. I don't think we are
introducing a new one, as it is fundamental to the web. I find "page" to be
very overloaded term and it is often misused.

I'm definitely in favor for changing code to be more explicit and use 
"document", so we can be more precise.

[Eugene But (OOO till 7-30), 2017-03-02 22:56:46]

On 2017/03/01 17:47:12, nasko (slow) wrote:
> On 2017/03/01 16:41:50, clamy wrote:
> > FWIW: I think InFrame is even more confusing. Technically, all main-frame
> > navigations are in frame, as the frame itself doesn't change.
> > 
> > We already have several WCO methods with Document in their name, so I don't
> > think it's weird to have SameDocument here.
> > 
> > I also suggested they go along with the change, as I have seen several
people
> > misunderstand what IsSamePage means. They thought it meant a same-process
> > navigation as opposed to what it really is, a same-document navigation.
> 
> IMHO, "document" is the most correct concept to use. I don't think we are
> introducing a new one, as it is fundamental to the web. I find "page" to be
> very overloaded term and it is often misused.
> 
> I'm definitely in favor for changing code to be more explicit and use 
> "document", so we can be more precise.
John, it looks like other folks (clamy@, creis@, nasko@) support this change.
Does it change your opinion?

[jam, 2017-03-03 16:45:05]

On 2017/03/02 22:56:46, Eugene But wrote:
> On 2017/03/01 17:47:12, nasko (slow) wrote:
> > On 2017/03/01 16:41:50, clamy wrote:
> > > FWIW: I think InFrame is even more confusing. Technically, all main-frame
> > > navigations are in frame, as the frame itself doesn't change.
> > > 
> > > We already have several WCO methods with Document in their name, so I
don't
> > > think it's weird to have SameDocument here.
> > > 
> > > I also suggested they go along with the change, as I have seen several
> people
> > > misunderstand what IsSamePage means. They thought it meant a same-process
> > > navigation as opposed to what it really is, a same-document navigation.
> > 
> > IMHO, "document" is the most correct concept to use. I don't think we are
> > introducing a new one, as it is fundamental to the web. I find "page" to be
> > very overloaded term and it is often misused.
> > 
> > I'm definitely in favor for changing code to be more explicit and use 
> > "document", so we can be more precise.
> John, it looks like other folks (clamy@, creis@, nasko@) support this change.
> Does it change your opinion?

(sorry for delay)

yes, that is very helpful, thank for the input everyone. lgtm

[Eugene But (OOO till 7-30), 2017-03-03 19:58:05]

Thank you! I will send more CLs to rename remaining APIs.

[Eugene But (OOO till 7-30), 2017-03-03 19:58:11]

The CQ bit was checked by eugenebut@chromium.org

[Eugene But (OOO till 7-30), 2017-03-03 19:58:12]

The patchset sent to the CQ was uploaded after l-g-t-m from clamy@chromium.org,
jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2716493004/#ps40001
(title: "Rebased")

[commit-bot: I haz the power, 2017-03-03 19:58:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-03 20:07:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-03 20:07:28]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[gone, 2017-03-03 20:08:48]

dfalcantara@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[gone, 2017-03-03 20:08:50]

drive by OWNERs lgtm for infobars/

[Eugene But (OOO till 7-30), 2017-03-03 21:25:23]

The CQ bit was checked by eugenebut@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-03 21:25:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-03 21:52:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-03 21:52:53]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Eugene But (OOO till 7-30), 2017-03-04 00:32:55]

eugenebut@chromium.org changed reviewers:
+ dbeam@chromium.org

[Eugene But (OOO till 7-30), 2017-03-04 00:32:56]

+ dbeam@ for chrome/browser/ui/webui/options

[Eugene But (OOO till 7-30), 2017-03-04 00:33:04]

The CQ bit was checked by eugenebut@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-04 00:33:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-04 01:52:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-04 01:52:35]

Dry run: This issue passed the CQ dry run.

[Eugene But (OOO till 7-30), 2017-03-07 03:25:19]

The CQ bit was checked by eugenebut@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-07 03:26:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Eugene But (OOO till 7-30), 2017-03-07 04:23:29]

Description was changed from

==========
Renamed NavigationHandle::IsSamePage to NavigationHandle::IsSameDocument

Old name was presumable used to match was_within_same_page IPC, but
IsSameDocument name better reflects the the type of the navigation
(navigation did not change the document object).

This change will make naming more consistent with WebKit (which has
SameDocumentNavigationSource class) and Chrome for iOS, which uses
SameDocument term.

BUG=695189
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Renamed NavigationHandle::IsSamePage to NavigationHandle::IsSameDocument

Old name was presumable used to match was_within_same_page IPC, but
IsSameDocument name better reflects the the type of the navigation
(navigation did not change the document object).

This change will make naming more consistent with WebKit (which has
SameDocumentNavigationSource class) and Chrome for iOS, which uses
SameDocument term.

BUG=695189
TBR=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Eugene But (OOO till 7-30), 2017-03-07 04:23:40]

The CQ bit was unchecked by eugenebut@chromium.org

[Eugene But (OOO till 7-30), 2017-03-07 04:23:47]

The CQ bit was checked by eugenebut@chromium.org

[Eugene But (OOO till 7-30), 2017-03-07 04:23:47]

The patchset sent to the CQ was uploaded after l-g-t-m from clamy@chromium.org,
jam@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2716493004/#ps100001
(title: "Rebased")

[commit-bot: I haz the power, 2017-03-07 04:24:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-07 06:30:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-07 06:30:12]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Eugene But (OOO till 7-30), 2017-03-07 15:23:39]

The CQ bit was checked by eugenebut@chromium.org

[commit-bot: I haz the power, 2017-03-07 15:24:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-07 17:13:58]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1488900219660530,
"parent_rev": "a20ad732333dc9db2b545886acd16a11abdc5842", "commit_rev":
"a11672fb4c998040224d7dc97e5bdc05d46a4145"}

[commit-bot: I haz the power, 2017-03-07 17:14:42]

Description was changed from

==========
Renamed NavigationHandle::IsSamePage to NavigationHandle::IsSameDocument

Old name was presumable used to match was_within_same_page IPC, but
IsSameDocument name better reflects the the type of the navigation
(navigation did not change the document object).

This change will make naming more consistent with WebKit (which has
SameDocumentNavigationSource class) and Chrome for iOS, which uses
SameDocument term.

BUG=695189
TBR=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Renamed NavigationHandle::IsSamePage to NavigationHandle::IsSameDocument

Old name was presumable used to match was_within_same_page IPC, but
IsSameDocument name better reflects the the type of the navigation
(navigation did not change the document object).

This change will make naming more consistent with WebKit (which has
SameDocumentNavigationSource class) and Chrome for iOS, which uses
SameDocument term.

BUG=695189
TBR=dbeam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2716493004
Cr-Commit-Position: refs/heads/master@{#455106}
Committed:
https://chromium.googlesource.com/chromium/src/+/a11672fb4c998040224d7dc97e5b...
==========

[commit-bot: I haz the power, 2017-03-07 17:14:45]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/a11672fb4c998040224d7dc97e5b...