[Offline pages] Add null pointer checks in offline internals code

chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc

Index: chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc
diff --git a/chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc b/chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc
index 9a8adbb9d7b39ef011ccf34d79cba9c9d8e6b35f..201d8caff1d119a798bb9d2b328cfc83020ccb21 100644
--- a/chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc
+++ b/chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc
@@ -173,7 +173,8 @@ void OfflineInternalsUIMessageHandler::HandleSetRecordPageModel(
     const base::ListValue* args) {
   bool should_record;
   CHECK(args->GetBoolean(0, &should_record));
-  offline_page_model_->GetLogger()->SetIsLogging(should_record);
+  if (offline_page_model_)

[Dan Beam, 2016/09/01 01:05:58]

can we just do

  if (!offline_page_model_)
    return;

or something in here and HandleSetRecordRequestQueue?

should these message handlers be hit (i.e. is there some way to do this from the
UI while incognito?).  if not: maybe just leave the code as is

[chili, 2016/09/01 01:29:15]

I think the only way to do this from UI would be if UI first sends an
"isIncognito?" message.  If true, then doesn't do anything and if false, then
calls everything else

On the other hand, I think we should have this too as an additional safeguard. 
While it's true that we don't expect non-incognito profiles to not have a page
model, there may be other features in play that causes this to error out (i.e.
some required flag is unset, etc).

+    offline_page_model_->GetLogger()->SetIsLogging(should_record);
 }
 
 void OfflineInternalsUIMessageHandler::HandleGetNetworkStatus(
@@ -191,7 +192,8 @@ void OfflineInternalsUIMessageHandler::HandleSetRecordRequestQueue(
     const base::ListValue* args) {
   bool should_record;
   CHECK(args->GetBoolean(0, &should_record));
-  request_coordinator_->GetLogger()->SetIsLogging(should_record);
+  if (request_coordinator_)
+    request_coordinator_->GetLogger()->SetIsLogging(should_record);
 }
 
 void OfflineInternalsUIMessageHandler::HandleGetLoggingState(
@@ -202,9 +204,13 @@ void OfflineInternalsUIMessageHandler::HandleGetLoggingState(
 
   base::DictionaryValue result;
   result.SetBoolean("modelIsLogging",
-                    offline_page_model_->GetLogger()->GetIsLogging());
+                    offline_page_model_
+                        ? offline_page_model_->GetLogger()->GetIsLogging()
+                        : false);
   result.SetBoolean("queueIsLogging",
-                    request_coordinator_->GetLogger()->GetIsLogging());
+                    request_coordinator_
+                        ? request_coordinator_->GetLogger()->GetIsLogging()
+                        : false);
   ResolveJavascriptCallback(*callback_id, result);
 }
 
@@ -215,8 +221,10 @@ void OfflineInternalsUIMessageHandler::HandleGetEventLogs(
   CHECK(args->Get(0, &callback_id));
 
   std::vector<std::string> logs;
-  offline_page_model_->GetLogger()->GetLogs(&logs);
-  request_coordinator_->GetLogger()->GetLogs(&logs);
+  if (offline_page_model_)
+    offline_page_model_->GetLogger()->GetLogs(&logs);
+  if (request_coordinator_)
+    request_coordinator_->GetLogger()->GetLogs(&logs);
   std::sort(logs.begin(), logs.end());
 
   base::ListValue result;
@@ -238,12 +246,16 @@ void OfflineInternalsUIMessageHandler::HandleAddToRequestQueue(
   std::ostringstream id_stream;
   id_stream << base::GenerateGUID();
 
-  ResolveJavascriptCallback(
-      *callback_id,
-      base::FundamentalValue(request_coordinator_->SavePageLater(
-          GURL(url), offline_pages::ClientId(offline_pages::kAsyncNamespace,
-                                             id_stream.str()),
-          true)));
+  if (request_coordinator_) {
+    ResolveJavascriptCallback(
+        *callback_id,
+        base::FundamentalValue(request_coordinator_->SavePageLater(
+            GURL(url), offline_pages::ClientId(offline_pages::kAsyncNamespace,
+                                               id_stream.str()),
+            true)));
+  } else {
+    ResolveJavascriptCallback(*callback_id, base::FundamentalValue(false));

[Dan Beam, 2016/09/01 01:05:58]

can we do this higher?  why bother getting |url| or generating a GUID if it
doesn't matter to the response?

[chili, 2016/09/01 01:29:15]

Done.

+  }
 }
 
 void OfflineInternalsUIMessageHandler::RegisterMessages() {