[OfflinePages] RequestCoordinatorFactory return null for incognito

chrome/browser/net/net_error_tab_helper.cc

Index: chrome/browser/net/net_error_tab_helper.cc
diff --git a/chrome/browser/net/net_error_tab_helper.cc b/chrome/browser/net/net_error_tab_helper.cc
index 823d96fd9a59b8d287529321bd40321bb0be47ea..27f15f1f3fac13fae35d797aa5ceccd57bcf9334 100644
--- a/chrome/browser/net/net_error_tab_helper.cc
+++ b/chrome/browser/net/net_error_tab_helper.cc
@@ -281,6 +281,7 @@ void NetErrorTabHelper::DownloadPageLaterHelper(const GURL& page_url) {
   offline_pages::RequestCoordinator* request_coordinator =
       offline_pages::RequestCoordinatorFactory::GetForBrowserContext(
           web_contents()->GetBrowserContext());
+  CHECK(request_coordinator);

[fgorski, 2016/11/22 21:45:14]

check is guaranteeing a crash in incognito and I am not sure that is what you
mean.

I would let Jian Li handle this part properly.

[dougarnett, 2016/11/22 22:01:16]

Yes, that was my original plan. Was trying to respond to Pete comment.
Happy to drop or keep. 

Crash means we find issue earlier and fix rather than someone clicking on button
and seeing the Downloading snackbar but nothing happening (which would really
need different snackbar message). Not presenting the button in the first place
is the clean fix so perhaps an assertion makes more sense here than error UX -
if we want to check here. It could be DCHECK (and expect to crash later or for
no progress ever downstream for non-debug build).

[jianli, 2016/11/22 22:06:36]

The fix for not showing download page later button under incognito has been
landed. I don't see adding DCHECK or CHECK here adds some extra value since it
will crash at line 287 if for some reason request_coordinator is null.

   offline_pages::ClientId client_id(
       offline_pages::kAsyncNamespace, base::GenerateGUID());
   request_coordinator->SavePageLater(