Do not start timer on when reloading a Reading List offline page on iOS.

Do not start timer on when reloading a Reading List offline page on iOS.

If a user has a distilled entry in the Reading List and is on a poor network, the
distilled entry will be loaded and the user cannot access the online page.
To avoid blocking the user from the up to date page, reloading a page from the
offline page will not start the offline triggering timer.
Reloading again from the pending loading will load back the online version.

BUG=680481
Review-Url: https://codereview.chromium.org/2616093002
Cr-Commit-Position: refs/heads/master@{#443287}
Committed: https://chromium.googlesource.com/chromium/src/+/3dcbdfb6ef790697a538ef3f0ab909e44af8ee5d

[Olivier, 2017-01-12 13:57:56]

Description was changed from

==========
Do not start timer on reloading

BUG=
==========

to

==========
Do not start timer on reloading

BUG=680481
==========

[Olivier, 2017-01-12 13:58:02]

olivierrobin@chromium.org changed reviewers:
+ jif@chromium.org

[jif, 2017-01-12 16:30:22]

lgtm

https://codereview.chromium.org/2616093002/diff/20001/ios/chrome/browser/read...
File ios/chrome/browser/reading_list/reading_list_web_state_observer.mm (right):

https://codereview.chromium.org/2616093002/diff/20001/ios/chrome/browser/read...
ios/chrome/browser/reading_list/reading_list_web_state_observer.mm:180:
is_reload || ui::PageTransitionCoreTypeIs(item->GetTransitionType(),
Not a fan of |is_reload| changing value multiple times (ideally it would be a
const bool), but can't find a better way of doing things... so that will do.

https://codereview.chromium.org/2616093002/diff/20001/ios/chrome/browser/read...
ios/chrome/browser/reading_list/reading_list_web_state_observer.mm:183: // the
online page even on bad networks. Do not need to launch timer.
s/Do not need to launch timer./No need to launch the timer./

[jif, 2017-01-12 16:31:31]

Description was changed from

==========
Do not start timer on reloading

BUG=680481
==========

to

==========
Do not start timer on when reloading an offline page on iOS.

BUG=680481
==========

[jif, 2017-01-12 16:33:04]

On 2017/01/12 16:30:22, jif wrote:
> lgtm
> 
>
https://codereview.chromium.org/2616093002/diff/20001/ios/chrome/browser/read...
> File ios/chrome/browser/reading_list/reading_list_web_state_observer.mm
(right):
> 
>
https://codereview.chromium.org/2616093002/diff/20001/ios/chrome/browser/read...
> ios/chrome/browser/reading_list/reading_list_web_state_observer.mm:180:
> is_reload || ui::PageTransitionCoreTypeIs(item->GetTransitionType(),
> Not a fan of |is_reload| changing value multiple times (ideally it would be a
> const bool), but can't find a better way of doing things... so that will do.
> 
>
https://codereview.chromium.org/2616093002/diff/20001/ios/chrome/browser/read...
> ios/chrome/browser/reading_list/reading_list_web_state_observer.mm:183: // the
> online page even on bad networks. Do not need to launch timer.
> s/Do not need to launch timer./No need to launch the timer./

Can you make the description of the commit slightly more descriptive? (Note:
I've already changed it a bit, but it's still not good).

[Olivier, 2017-01-12 16:53:20]

Description was changed from

==========
Do not start timer on when reloading an offline page on iOS.

BUG=680481
==========

to

==========
Do not start timer on when reloading a Reading List offline page on iOS.

If a user has a distilled entry in the Reading List and is on a poor network,
the
distilled entry will be loaded and the user cannot access the online page.
To avoid blocking the user from the up to date page, reloading a page from the
offline page will not start the offline triggering timer.
Reloading again from the pending loading will load back the online version.

BUG=680481
==========

[Olivier, 2017-01-12 16:53:35]

Done, Thanks

https://codereview.chromium.org/2616093002/diff/20001/ios/chrome/browser/read...
File ios/chrome/browser/reading_list/reading_list_web_state_observer.mm (right):

https://codereview.chromium.org/2616093002/diff/20001/ios/chrome/browser/read...
ios/chrome/browser/reading_list/reading_list_web_state_observer.mm:183: // the
online page even on bad networks. Do not need to launch timer.
On 2017/01/12 16:30:22, jif wrote:
> s/Do not need to launch timer./No need to launch the timer./

Done.

[Olivier, 2017-01-12 17:42:08]

The CQ bit was checked by olivierrobin@chromium.org

[Olivier, 2017-01-12 17:42:08]

The patchset sent to the CQ was uploaded after l-g-t-m from jif@chromium.org
Link to the patchset: https://codereview.chromium.org/2616093002/#ps40001
(title: "feedback")

[commit-bot: I haz the power, 2017-01-12 17:43:59]

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-01-12 17:58:05]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484242928343010,
"parent_rev": "b2e0703838bc7140c4129c491085a1da55cacbbf", "commit_rev":
"3dcbdfb6ef790697a538ef3f0ab909e44af8ee5d"}

[commit-bot: I haz the power, 2017-01-12 17:58:58]

Description was changed from

==========
Do not start timer on when reloading a Reading List offline page on iOS.

If a user has a distilled entry in the Reading List and is on a poor network,
the
distilled entry will be loaded and the user cannot access the online page.
To avoid blocking the user from the up to date page, reloading a page from the
offline page will not start the offline triggering timer.
Reloading again from the pending loading will load back the online version.

BUG=680481
==========

to

==========
Do not start timer on when reloading a Reading List offline page on iOS.

If a user has a distilled entry in the Reading List and is on a poor network,
the
distilled entry will be loaded and the user cannot access the online page.
To avoid blocking the user from the up to date page, reloading a page from the
offline page will not start the offline triggering timer.
Reloading again from the pending loading will load back the online version.

BUG=680481

Review-Url: https://codereview.chromium.org/2616093002
Cr-Commit-Position: refs/heads/master@{#443287}
Committed:
https://chromium.googlesource.com/chromium/src/+/3dcbdfb6ef790697a538ef3f0ab9...
==========

[commit-bot: I haz the power, 2017-01-12 17:59:00]

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