Show offline interstitial page when offline and reload when reconnected to network.

Show offline interstitial page when offline and reload when reconnected to network.
* Added OfflineResourceHandler that intercept the request and show interstitial page.
* Added OfflineLoadPage that is shown when offline. 
   This gets deleted when
    - User pressed "Load now" btton to proceed or
    - User pressed "Cancel" button to cancel loading
    - Network become available.
   The page first appears as blank page (a little darker than white for now. I'll update when mock is ready), 
   and then options become available after 3 seconds maximum.
* Added unit test for OfflineLoadPage class.
* OfflineLoadService class to control when/if a load request
 should proceed regardless of network status.
 The current implementation is tentative and will proceed
 if if loading was requested in a given tab.
 I'll revisit this class to improve the logic in separate CL
 later.

Known Issue:
 - thumbnail is not working yet. I'll working on this in separate Cl.
 - a tab shows URL instead of title string. I'll fix this in separate CL.
 - InitNavigationParams in offline_load_page_unittest is copied from safe_browsing_blocking_page_unittest. I'll move this to common place in separate CL. (hopefully before checking this in)



BUG=chromium-os:3605
TEST=unit test: offline_load_page_unittest
     manual: disable wifi and ethernet, then login.
             chrome will show the offline page when tab is activated. Enable network (wifi or ethernet) and all tab will start loading again.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=52433

[oshima, 2010-07-12 23:40:13]

Sorry for big CL. I tried to make this small but wasn't successful.

[DaveMoore, 2010-07-13 16:36:46]

http://codereview.chromium.org/2931005/diff/11001/3013
File chrome/browser/chromeos/network_state_notifier.cc (right):

http://codereview.chromium.org/2931005/diff/11001/3013#newcode28
chrome/browser/chromeos/network_state_notifier.cc:28: }
You might want to make this an instance method so it's more easily mockable.

http://codereview.chromium.org/2931005/diff/11001/3013#newcode54
chrome/browser/chromeos/network_state_notifier.cc:54: offline_start_time_ =
Time::Now();
two line conditionals followed by a statement should use braces to make them
more clear.

http://codereview.chromium.org/2931005/diff/11001/3013#newcode68
chrome/browser/chromeos/network_state_notifier.cc:68: return
NetworkStateDetails::DISCONNECTED;
The comment doesn't match the code...which is the default?

http://codereview.chromium.org/2931005/diff/11001/3014
File chrome/browser/chromeos/network_state_notifier.h (right):

http://codereview.chromium.org/2931005/diff/11001/3014#newcode78
chrome/browser/chromeos/network_state_notifier.h:78:
ScopedRunnableMethodFactory<NetworkStateNotifier> task_factory_;
Nit: comment.

http://codereview.chromium.org/2931005/diff/11001/3016
File chrome/browser/chromeos/offline/offline_load_page.cc (right):

http://codereview.chromium.org/2931005/diff/11001/3016#newcode95
chrome/browser/chromeos/offline/offline_load_page.cc:95: if (command.length() >
1 && command[0] == '"') {
You should check if the length is > 2 not 1, as you subtract 2 from it later.

http://codereview.chromium.org/2931005/diff/11001/3023
File chrome/browser/renderer_host/resource_dispatcher_host.cc (right):

http://codereview.chromium.org/2931005/diff/11001/3023#newcode459
chrome/browser/renderer_host/resource_dispatcher_host.cc:459: #if
defined(OS_CHROMEOS)
How does this interact w/ safe browsing?

http://codereview.chromium.org/2931005/diff/11001/3025
File chrome/browser/resources/offline_load.html (right):

http://codereview.chromium.org/2931005/diff/11001/3025#newcode81
chrome/browser/resources/offline_load.html:81: }
Can we make the buttons fade in?

[oshima, 2010-07-14 20:39:31]

http://codereview.chromium.org/2931005/diff/11001/3013
File chrome/browser/chromeos/network_state_notifier.cc (right):

http://codereview.chromium.org/2931005/diff/11001/3013#newcode28
chrome/browser/chromeos/network_state_notifier.cc:28: }
On 2010/07/13 16:36:46, davemoore wrote:
> You might want to make this an instance method so it's more easily mockable.

Good point. May I do it in separate CL for ui_test?

http://codereview.chromium.org/2931005/diff/11001/3013#newcode54
chrome/browser/chromeos/network_state_notifier.cc:54: offline_start_time_ =
Time::Now();
On 2010/07/13 16:36:46, davemoore wrote:
> two line conditionals followed by a statement should use braces to make them
> more clear.
done. i've seen both cases and seems to be arbitrary tho.

http://codereview.chromium.org/2931005/diff/11001/3013#newcode68
chrome/browser/chromeos/network_state_notifier.cc:68: return
NetworkStateDetails::DISCONNECTED;
On 2010/07/13 16:36:46, davemoore wrote:
> The comment doesn't match the code...which is the default?

oops, forgot to revert test code. fixed.

http://codereview.chromium.org/2931005/diff/11001/3014
File chrome/browser/chromeos/network_state_notifier.h (right):

http://codereview.chromium.org/2931005/diff/11001/3014#newcode78
chrome/browser/chromeos/network_state_notifier.h:78:
ScopedRunnableMethodFactory<NetworkStateNotifier> task_factory_;
On 2010/07/13 16:36:46, davemoore wrote:
> Nit: comment.

Done.

http://codereview.chromium.org/2931005/diff/11001/3016
File chrome/browser/chromeos/offline/offline_load_page.cc (right):

http://codereview.chromium.org/2931005/diff/11001/3016#newcode95
chrome/browser/chromeos/offline/offline_load_page.cc:95: if (command.length() >
1 && command[0] == '"') {
On 2010/07/13 16:36:46, davemoore wrote:
> You should check if the length is > 2 not 1, as you subtract 2 from it later.
nice catch, thanks. fixed the original as well. (safe_browsing_blocking_page.cc)

http://codereview.chromium.org/2931005/diff/11001/3023
File chrome/browser/renderer_host/resource_dispatcher_host.cc (right):

http://codereview.chromium.org/2931005/diff/11001/3023#newcode459
chrome/browser/renderer_host/resource_dispatcher_host.cc:459: #if
defined(OS_CHROMEOS)
On 2010/07/13 16:36:46, davemoore wrote:
> How does this interact w/ safe browsing?

Added comment in the code. I'll working on ui_tests and add this scenario.

[DaveMoore, 2010-07-14 20:48:27]

My prior comment about > 1 being > 2 was wrong....> 1 is the right thing here.

Other than that, LGTM

[oshima, 2010-07-14 21:06:44]

On 2010/07/14 20:48:27, davemoore wrote:
> My prior comment about > 1 being > 2 was wrong....> 1 is the right thing here.
> 
> Other than that, LGTM

Doh, may be i need some break. thanks.