[Reading List] Show page info for offline pages

[Reading List] Show page info for offline pages

Offline pages pages show informations about offline state in the page
info model.
Clean-up after https://codereview.chromium.org/2723683002

BUG=696628
Review-Url: https://codereview.chromium.org/2721403002
Cr-Commit-Position: refs/heads/master@{#454030}
Committed: https://chromium.googlesource.com/chromium/src/+/013ba67833cc40fbc072a1f59ffd67ec22bb6a44

[Olivier, 2017-03-01 18:11:42]

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

[Eugene But (OOO till 7-30), 2017-03-01 18:37:04]

lgtm

[kkhorimoto, 2017-03-01 18:43:53]

https://codereview.chromium.org/2721403002/diff/1/ios/chrome/browser/ui/brows...
File ios/chrome/browser/ui/browser_view_controller.mm (right):

https://codereview.chromium.org/2721403002/diff/1/ios/chrome/browser/ui/brows...
ios/chrome/browser/ui/browser_view_controller.mm:3346: _browserState,
navItem->GetURL(), navItem->GetSSL(), bridge);
Are we sure we want to be able to show the PageInfo popover for reading list
NavigationItems?  It looks like the popover will be using the URL instead of the
virtual URL, which may be a jarring experience since the omnibox will be showing
a different URL than the info popover.  Additionally, it looks like this popover
is related to SSL status, which is not really relevant to reading list pages
since they're native pages loaded from files.

[kkhorimoto, 2017-03-01 18:46:42]

https://codereview.chromium.org/2721403002/diff/1/ios/chrome/browser/ui/brows...
File ios/chrome/browser/ui/browser_view_controller.mm (right):

https://codereview.chromium.org/2721403002/diff/1/ios/chrome/browser/ui/brows...
ios/chrome/browser/ui/browser_view_controller.mm:3346: _browserState,
navItem->GetURL(), navItem->GetSSL(), bridge);
On 2017/03/01 18:43:53, kkhorimoto_ wrote:
> Are we sure we want to be able to show the PageInfo popover for reading list
> NavigationItems?  It looks like the popover will be using the URL instead of
the
> virtual URL, which may be a jarring experience since the omnibox will be
showing
> a different URL than the info popover.  Additionally, it looks like this
popover
> is related to SSL status, which is not really relevant to reading list pages
> since they're native pages loaded from files.

Actually Eugene showed me that there's already a separate info popover for
reading list that behaves correctly. lgtm!

[Olivier, 2017-03-01 20:56:32]

The CQ bit was checked by olivierrobin@chromium.org

[commit-bot: I haz the power, 2017-03-01 20:57:02]

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-01 21:16:28]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1488401792209540, "parent_rev":
"843b41a25f71a9980dbfe2f5a70407efd0ef2ea2", "commit_rev":
"013ba67833cc40fbc072a1f59ffd67ec22bb6a44"}

[commit-bot: I haz the power, 2017-03-01 21:17:08]

Description was changed from

==========
[Reading List] Show page info for offline pages

Offline pages pages show informations about offline state in the page
info model.
Clean-up after https://codereview.chromium.org/2723683002

BUG=696628
==========

to

==========
[Reading List] Show page info for offline pages

Offline pages pages show informations about offline state in the page
info model.
Clean-up after https://codereview.chromium.org/2723683002

BUG=696628

Review-Url: https://codereview.chromium.org/2721403002
Cr-Commit-Position: refs/heads/master@{#454030}
Committed:
https://chromium.googlesource.com/chromium/src/+/013ba67833cc40fbc072a1f59ffd...
==========

[commit-bot: I haz the power, 2017-03-01 21:17:09]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/013ba67833cc40fbc072a1f59ffd...