If you reload an offline page while online, load the online version.

If you reload an offline page while online, load the online version.

If you have an offline page showing, but connectivity has returned, when
you pull down on the page to refresh, we should reload the online version.

This change catches the onPageLoadStarted event to check for that case.
The onLoadUrl event does not get called on a reload, and the onUpdateUrl
event is also not called on a reload, so onPageLoadStarted is likely
our best candidate.

BUG=565529
Committed: https://crrev.com/dc263a448b303e30854fb04812a912aafb70f47e
Cr-Commit-Position: refs/heads/master@{#371572}

[Pete Williamson, 2016-01-13 22:26:57]

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

[commit-bot: I haz the power, 2016-01-13 22:28:00]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1586783003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1586783003/20001

[commit-bot: I haz the power, 2016-01-14 00:38:17]

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

[commit-bot: I haz the power, 2016-01-14 00:38:18]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_TIMED_OUT, no
build URL)
  android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_TIMED_OUT,
no build URL)
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_TIMED_OUT,
no build URL)
  android_clang_dbg_recipe on tryserver.chromium.android (JOB_TIMED_OUT, no
build URL)
  android_compile_dbg on tryserver.chromium.android (JOB_TIMED_OUT, no build
URL)
  cast_shell_android on tryserver.chromium.android (JOB_TIMED_OUT, no build URL)
  linux_android_rel_ng on tryserver.chromium.android (JOB_TIMED_OUT, no build
URL)
  cast_shell_linux on tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_TIMED_OUT, no build
URL)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build
URL)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build URL)
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build
URL)

[Pete Williamson, 2016-01-14 18:28:24]

petewil@chromium.org changed reviewers:
+ fgorski@chromium.org

[fgorski, 2016-01-14 19:17:24]

https://codereview.chromium.org/1586783003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageTabObserver.java
(right):

https://codereview.chromium.org/1586783003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageTabObserver.java:73:
tab.loadUrl(new LoadUrlParams(tab.getOfflinePageOriginalUrl(),
PageTransition.RELOAD));
So a reload will cause another reload? Is there an impact on history?
Is this a common pattern to use in such case? *It might be. I don't know.

Also I have a feeling that there might be a few things we will be measuring here
with UMA.
Can you place the reload code in OfflinePageUtils?

[Pete Williamson, 2016-01-26 18:04:16]

On 2016/01/14 19:17:24, fgorski wrote:
>
https://codereview.chromium.org/1586783003/diff/20001/chrome/android/java/src...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageTabObserver.java
> (right):
> 
>
https://codereview.chromium.org/1586783003/diff/20001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageTabObserver.java:73:
> tab.loadUrl(new LoadUrlParams(tab.getOfflinePageOriginalUrl(),
> PageTransition.RELOAD));
> So a reload will cause another reload? Is there an impact on history?
> Is this a common pattern to use in such case? *It might be. I don't know.
> 
> Also I have a feeling that there might be a few things we will be measuring
here
> with UMA.
> Can you place the reload code in OfflinePageUtils?

Done.

History:  When I use the feature, the history looks like this (most recent at
top)

Online version of the page   // This is the one we picked from the reload
Offline version of the page  // We Loaded an offline version of the page to get
here (note that there are not two, reload doesn't add one)
Online version of the page   // This is from when we started and got an offline
copy.

In other words, we only have one of the offline version of the page in the
history stack, not two (one is correct for this scenario).

UMA: Any specific suggestions for things you'd like to see measured?  Just one
for when we override the reload, or is there something else you think we might
want?

[fgorski, 2016-01-26 18:47:32]

lgtm

For the UMA I would consider tracking either a user action (online reload) or
the fact that the user is online/offline when reloading as a UMA (which gives
you the reload action as a sum). Probably latter is better.

[Pete Williamson, 2016-01-26 19:17:19]

The CQ bit was checked by petewil@chromium.org

[commit-bot: I haz the power, 2016-01-26 19:17:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1586783003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1586783003/40001

[commit-bot: I haz the power, 2016-01-26 20:05:19]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-26 20:07:12]

Description was changed from

==========
If you reload an offline page while online, load the online version.

If you have an offline page showing, but connectivity has returned, when
you pull down on the page to refresh, we should reload the online version.

This change catches the onPageLoadStarted event to check for that case.
The onLoadUrl event does not get called on a reload, and the onUpdateUrl
event is also not called on a reload, so onPageLoadStarted is likely
our best candidate.

BUG=565529
==========

to

==========
If you reload an offline page while online, load the online version.

If you have an offline page showing, but connectivity has returned, when
you pull down on the page to refresh, we should reload the online version.

This change catches the onPageLoadStarted event to check for that case.
The onLoadUrl event does not get called on a reload, and the onUpdateUrl
event is also not called on a reload, so onPageLoadStarted is likely
our best candidate.

BUG=565529

Committed: https://crrev.com/dc263a448b303e30854fb04812a912aafb70f47e
Cr-Commit-Position: refs/heads/master@{#371572}
==========

[commit-bot: I haz the power, 2016-01-26 20:07:12]

Patchset 3 (id:??) landed as
https://crrev.com/dc263a448b303e30854fb04812a912aafb70f47e
Cr-Commit-Position: refs/heads/master@{#371572}