Reland Tab History and BrowserViewController CRWSessionEntry removal.

Reland Tab History and BrowserViewController CRWSessionEntry removal.

Original CLs:
https://codereview.chromium.org/2693013005
https://codereview.chromium.org/2690943006

Notable Changes:
- Created web::GetDisplayTitleForUrl() to be used in tests.
- Correctly use "Tab History" as a11y identifier instead of label.
- Update GetTitleForDisplay() to use virtual URL as fallback.

BUG=454984, 546348, 546355, 695413
Review-Url: https://codereview.chromium.org/2722983003
Cr-Commit-Position: refs/heads/master@{#454749}
Committed: https://chromium.googlesource.com/chromium/src/+/7aed9e267aedf0f2c98ed3c802613ddd080546d1

[kkhorimoto, 2017-03-01 22:35:13]

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

[kkhorimoto, 2017-03-01 22:35:13]

PTAL: first patch has original changes, second patch has fixes

[Eugene But (OOO till 7-30), 2017-03-01 22:45:44]

https://codereview.chromium.org/2722983003/diff/20001/ios/chrome/browser/ui/h...
File ios/chrome/browser/ui/history/tab_history_view_controller.mm (right):

https://codereview.chromium.org/2722983003/diff/20001/ios/chrome/browser/ui/h...
ios/chrome/browser/ui/history/tab_history_view_controller.mm:279:
collectionView.accessibilityIdentifier = kCollectionViewIdentifier;
Why? Accessibility labels are more useful, because they are also used in voice
over.

https://codereview.chromium.org/2722983003/diff/20001/ios/web/navigation/navi...
File ios/web/navigation/navigation_item_impl.mm (right):

https://codereview.chromium.org/2722983003/diff/20001/ios/web/navigation/navi...
ios/web/navigation/navigation_item_impl.mm:300: DCHECK(!url.is_empty());
Is this DCHECK valid?

https://codereview.chromium.org/2722983003/diff/20001/ios/web/navigation/navi...
ios/web/navigation/navigation_item_impl.mm:304: if (url.SchemeIsFile()) {
Old code used url, new code uses virtualURL. Is that ok?

https://codereview.chromium.org/2722983003/diff/20001/ios/web/test/url_test_u...
File ios/web/test/url_test_util.mm (right):

https://codereview.chromium.org/2722983003/diff/20001/ios/web/test/url_test_u...
ios/web/test/url_test_util.mm:5: #import "ios/web/public/test/url_test_util.h"
s/import/include 

Same comment for other includes of this file

[kkhorimoto, 2017-03-01 23:23:47]

https://codereview.chromium.org/2722983003/diff/20001/ios/chrome/browser/ui/h...
File ios/chrome/browser/ui/history/tab_history_view_controller.mm (right):

https://codereview.chromium.org/2722983003/diff/20001/ios/chrome/browser/ui/h...
ios/chrome/browser/ui/history/tab_history_view_controller.mm:279:
collectionView.accessibilityIdentifier = kCollectionViewIdentifier;
On 2017/03/01 22:45:44, Eugene But wrote:
> Why? Accessibility labels are more useful, because they are also used in voice
> over.

This just preserves old behavior.  I've added a TODO to decide on better a11y
behavior for this feature.

https://codereview.chromium.org/2722983003/diff/20001/ios/web/navigation/navi...
File ios/web/navigation/navigation_item_impl.mm (right):

https://codereview.chromium.org/2722983003/diff/20001/ios/web/navigation/navi...
ios/web/navigation/navigation_item_impl.mm:300: DCHECK(!url.is_empty());
On 2017/03/01 22:45:44, Eugene But wrote:
> Is this DCHECK valid?

I've updated to an early return for code robustness.

https://codereview.chromium.org/2722983003/diff/20001/ios/web/navigation/navi...
ios/web/navigation/navigation_item_impl.mm:304: if (url.SchemeIsFile()) {
On 2017/03/01 22:45:44, Eugene But wrote:
> Old code used url, new code uses virtualURL. Is that ok?

Yes, since the virtual URL is what's displayed to the user, it also makes sense
to use it to decide the displayed title.  I've updated the CL comment with this
as well.

https://codereview.chromium.org/2722983003/diff/20001/ios/web/test/url_test_u...
File ios/web/test/url_test_util.mm (right):

https://codereview.chromium.org/2722983003/diff/20001/ios/web/test/url_test_u...
ios/web/test/url_test_util.mm:5: #import "ios/web/public/test/url_test_util.h"
On 2017/03/01 22:45:44, Eugene But wrote:
> s/import/include 
> 
> Same comment for other includes of this file

Done.

[kkhorimoto, 2017-03-01 23:24:06]

Description was changed from

==========
Reland Tab History and BrowserViewController CRWSessionEntry removal.

Original CLs:
https://codereview.chromium.org/2693013005
https://codereview.chromium.org/2690943006

Notable Changes:
- Created web::GetDisplayTitleForUrl() to be used in tests.
- Correctly use "Tab History" as a11y identifier instead of label.

BUG=454984, 546348, 546355, 695413
==========

to

==========
Reland Tab History and BrowserViewController CRWSessionEntry removal.

Original CLs:
https://codereview.chromium.org/2693013005
https://codereview.chromium.org/2690943006

Notable Changes:
- Created web::GetDisplayTitleForUrl() to be used in tests.
- Correctly use "Tab History" as a11y identifier instead of label.
- Update GetTitleForDisplay() to use virtual URL as fallback.

BUG=454984, 546348, 546355, 695413
==========

[kkhorimoto, 2017-03-01 23:24:18]

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

[commit-bot: I haz the power, 2017-03-01 23:24:43]

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-01 23:32:14]

lgtm

[commit-bot: I haz the power, 2017-03-02 00:29:26]

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

[commit-bot: I haz the power, 2017-03-02 00:29:27]

Dry run: This issue passed the CQ dry run.

[kkhorimoto, 2017-03-04 01:32:04]

The CQ bit was checked by kkhorimoto@chromium.org

[commit-bot: I haz the power, 2017-03-04 01:32:25]

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 02:28:58]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488591124852400,
"parent_rev": "409ec2dcbbdab8f2eb6666930a92a569c4a14155", "commit_rev":
"7aed9e267aedf0f2c98ed3c802613ddd080546d1"}

[commit-bot: I haz the power, 2017-03-04 02:29:35]

Description was changed from

==========
Reland Tab History and BrowserViewController CRWSessionEntry removal.

Original CLs:
https://codereview.chromium.org/2693013005
https://codereview.chromium.org/2690943006

Notable Changes:
- Created web::GetDisplayTitleForUrl() to be used in tests.
- Correctly use "Tab History" as a11y identifier instead of label.
- Update GetTitleForDisplay() to use virtual URL as fallback.

BUG=454984, 546348, 546355, 695413
==========

to

==========
Reland Tab History and BrowserViewController CRWSessionEntry removal.

Original CLs:
https://codereview.chromium.org/2693013005
https://codereview.chromium.org/2690943006

Notable Changes:
- Created web::GetDisplayTitleForUrl() to be used in tests.
- Correctly use "Tab History" as a11y identifier instead of label.
- Update GetTitleForDisplay() to use virtual URL as fallback.

BUG=454984, 546348, 546355, 695413

Review-Url: https://codereview.chromium.org/2722983003
Cr-Commit-Position: refs/heads/master@{#454749}
Committed:
https://chromium.googlesource.com/chromium/src/+/7aed9e267aedf0f2c98ed3c80261...
==========

[commit-bot: I haz the power, 2017-03-04 02:29:37]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/7aed9e267aedf0f2c98ed3c80261...