Disallow showing offline previews on a reload.

Disallow showing offline previews on a reload.

This change prevents reloads being shown an offline preview. This does
not prevent other previews from being shown. A reload on an offline
preview is still considered to be non-opt out for the blacklist unless the user hit "show original".

BUG=672237
Committed: https://crrev.com/bc5da354fd37ccda92436a438f3f41ff0d1b940e
Cr-Commit-Position: refs/heads/master@{#438549}

[RyanSturm, 2016-12-07 22:18:31]

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

[commit-bot: I haz the power, 2016-12-07 22:19:04]

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

[RyanSturm, 2016-12-07 22:38:56]

Description was changed from

==========
Disallow showing offline previews on a reload.

This change prevents reloads being shown an offline preview. This does
not prevent other previews from being shown. A reload on an offline
preview is still considered to be non-opt out for the blacklist.

BUG=672237
==========

to

==========
Disallow showing offline previews on a reload.

This change prevents reloads being shown an offline preview. This does
not prevent other previews from being shown. A reload on an offline
preview is still considered to be non-opt out for the blacklist unless the user
hit "show original".

BUG=672237
==========

[RyanSturm, 2016-12-07 22:58:39]

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

[commit-bot: I haz the power, 2016-12-07 22:58:57]

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

[RyanSturm, 2016-12-07 23:05:07]

ryansturm@chromium.org changed reviewers:
+ tbansal@chromium.org

[RyanSturm, 2016-12-07 23:05:07]

tbansal: PTAL

[commit-bot: I haz the power, 2016-12-08 00:19:00]

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

[commit-bot: I haz the power, 2016-12-08 00:19:01]

Dry run: This issue passed the CQ dry run.

[RyanSturm, 2016-12-12 19:47:38]

tbansal: ping

[tbansal1, 2016-12-12 19:52:21]

https://codereview.chromium.org/2563493002/diff/20001/components/previews/cor...
File components/previews/core/previews_black_list.h (right):

https://codereview.chromium.org/2563493002/diff/20001/components/previews/cor...
components/previews/core/previews_black_list.h:52: // If the page was reloaded,
the user should not be shown an offline preview.
Should the user be shown the offline page if they are still offline after the
reload?

https://codereview.chromium.org/2563493002/diff/20001/components/previews/cor...
File components/previews/core/previews_io_data.cc (right):

https://codereview.chromium.org/2563493002/diff/20001/components/previews/cor...
components/previews/core/previews_io_data.cc:121: // LOAD_VALIDATE_CACHE and
LOAD_BYPASS_CACHE mean the user reloaded the page.
s/and/or/

Also, I am not sure if the statement is correct. There might be some other uses
of these 2 flags besides reload (e.g., forward-backward or tab restore). May be
not today, but in future? Might be better to name the enum more generically.

[RyanSturm, 2016-12-12 22:40:24]

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

[RyanSturm, 2016-12-12 22:42:15]

https://codereview.chromium.org/2563493002/diff/20001/components/previews/cor...
File components/previews/core/previews_black_list.h (right):

https://codereview.chromium.org/2563493002/diff/20001/components/previews/cor...
components/previews/core/previews_black_list.h:52: // If the page was reloaded,
the user should not be shown an offline preview.
On 2016/12/12 19:52:21, tbansal1 wrote:
> Should the user be shown the offline page if they are still offline after the
> reload?

This isn't the offline pages so much as offline previews. If the user reloads,
if they are offline, they will still be shown an offline page due to the normal
offline page logic.

https://codereview.chromium.org/2563493002/diff/20001/components/previews/cor...
File components/previews/core/previews_io_data.cc (right):

https://codereview.chromium.org/2563493002/diff/20001/components/previews/cor...
components/previews/core/previews_io_data.cc:121: // LOAD_VALIDATE_CACHE and
LOAD_BYPASS_CACHE mean the user reloaded the page.
On 2016/12/12 19:52:21, tbansal1 wrote:
> s/and/or/
> 
Done.

> Also, I am not sure if the statement is correct. There might be some other
uses
> of these 2 flags besides reload (e.g., forward-backward or tab restore). May
be
> not today, but in future? Might be better to name the enum more generically.

SKIP_CACHE_VALIDATION is forward/back. I think the enum is appropriately named,
as the intention is to prevent reload-esque main frame requests.

[commit-bot: I haz the power, 2016-12-12 22:42:35]

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

[tbansal1, 2016-12-12 23:12:37]

lgtm

[commit-bot: I haz the power, 2016-12-13 02:45:45]

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

[commit-bot: I haz the power, 2016-12-13 02:45:46]

Dry run: This issue passed the CQ dry run.

[RyanSturm, 2016-12-13 22:32:18]

ryansturm@chromium.org changed reviewers:
+ asvitkine@chromium.org

[RyanSturm, 2016-12-13 22:32:19]

asvitkine: PTAL

[Alexei Svitkine (slow), 2016-12-14 16:32:41]

lgtm

[RyanSturm, 2016-12-14 16:34:38]

The CQ bit was checked by ryansturm@chromium.org

[commit-bot: I haz the power, 2016-12-14 16:35: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, 2016-12-14 18:43:07]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481733278566180,
"parent_rev": "b6196514ea20f2e53005c21b8437ad31184b3280", "commit_rev":
"486faec093f2f5424cf89c4f2e5a54fa8204da7e"}

[commit-bot: I haz the power, 2016-12-14 18:43:39]

Description was changed from

==========
Disallow showing offline previews on a reload.

This change prevents reloads being shown an offline preview. This does
not prevent other previews from being shown. A reload on an offline
preview is still considered to be non-opt out for the blacklist unless the user
hit "show original".

BUG=672237
==========

to

==========
Disallow showing offline previews on a reload.

This change prevents reloads being shown an offline preview. This does
not prevent other previews from being shown. A reload on an offline
preview is still considered to be non-opt out for the blacklist unless the user
hit "show original".

BUG=672237

Review-Url: https://codereview.chromium.org/2563493002
==========

[commit-bot: I haz the power, 2016-12-14 18:43:40]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-14 18:46:05]

Description was changed from

==========
Disallow showing offline previews on a reload.

This change prevents reloads being shown an offline preview. This does
not prevent other previews from being shown. A reload on an offline
preview is still considered to be non-opt out for the blacklist unless the user
hit "show original".

BUG=672237

Review-Url: https://codereview.chromium.org/2563493002
==========

to

==========
Disallow showing offline previews on a reload.

This change prevents reloads being shown an offline preview. This does
not prevent other previews from being shown. A reload on an offline
preview is still considered to be non-opt out for the blacklist unless the user
hit "show original".

BUG=672237

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

[commit-bot: I haz the power, 2016-12-14 18:46:06]

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