[OfflinePages] Restarts immediate processing if stopped due to no net

components/offline_pages/background/request_coordinator.h

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_OFFLINE_PAGES_BACKGROUND_REQUEST_COORDINATOR_H_
 #define COMPONENTS_OFFLINE_PAGES_BACKGROUND_REQUEST_COORDINATOR_H_
 
 #include <memory>
 #include <set>
 #include <string>
 #include <vector>
 
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/supports_user_data.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "components/keyed_service/core/keyed_service.h"
+#include "components/offline_pages/background/connection_notifier.h"
 #include "components/offline_pages/background/device_conditions.h"
 #include "components/offline_pages/background/offliner.h"
 #include "components/offline_pages/background/request_coordinator_event_logger.h"
 #include "components/offline_pages/background/request_notifier.h"
 #include "components/offline_pages/background/request_queue.h"
 #include "components/offline_pages/background/scheduler.h"
 #include "net/nqe/network_quality_estimator.h"
 #include "url/gurl.h"
 
 namespace offline_pages {
@@ skipping 217 matching lines @@
   bool StartProcessingInternal(const ProcessingWindowState processing_state,
                                const DeviceConditions& device_conditions,
                                const base::Callback<void(bool)>& callback);
 
   // Start processing now if connected (but with conservative assumption
   // as to other device conditions).
   void StartImmediatelyIfConnected();
 
   OfflinerImmediateStartStatus TryImmediateStart();
 
+  // Requests a callback upon the next network connection to start processing.
+  void RequestConnectedEventForStarting();
+
+  // Clears the request for connected event if it was set.
+  void ClearConnectedEventRequest();
+
+  // Handles receiving a connection event. Will start immediate processing.
+  void HandleConnectedEventForStarting();
+
   // Check the request queue, and schedule a task corresponding
   // to the least restrictive type of request in the queue.
   void ScheduleAsNeeded();
 
   // Callback from the request picker when it has chosen our next request.
   void RequestPicked(const SavePageRequest& request);
 
   // Callback from the request picker when no more requests are in the queue.
   // The parameter is a signal for what (if any) conditions to schedule future
   // processing for.
@@ skipping 67 matching lines @@
   void GetOffliner();
 
   // Method to wrap calls to getting the connection type so it can be
   // changed for tests.
   net::NetworkChangeNotifier::ConnectionType GetConnectionType();
 
   void SetNetworkConditionsForTest(
       net::NetworkChangeNotifier::ConnectionType connection) {
     use_test_connection_type_ = true;
     test_connection_type_ = connection;
+    if (connection_notifier_)

[Pete Williamson, 2016/12/02 01:42:57]

Why add this?  It will fire an event, right?  In theory, I would expect that we
shouldn't need to fire an event when setting up network conditions because we
have already set the network to be up or down.

[dougarnett, 2016/12/02 02:24:54]

Yes, I want to be able to fire an event if there is an active notifier which
won't happen from our local use_test_connection_type_ and test_conneciton_type
members here that only work with GetConnectionType.

I could create another method to just do the event but I would like the test
state to also be consistent. I suppose I can call both then (if there are 2
methods) but this seemed attractive initially for changing the network state in
a test as a unit (firing an event if appropriate). Perhaps this is more of an
UpdateNetworkConditionsForTest()

Thoughts on how to proceed?

[dougarnett, 2016/12/02 19:08:46]

Added separate method in test class

+      connection_notifier_->OnConnectionTypeChanged(connection);
   }
 
   void SetDeviceConditionsForTest(const DeviceConditions& current_conditions) {
     current_conditions_.reset(new DeviceConditions(current_conditions));
   }
 
   // KeyedService implementation:
   void Shutdown() override;
 
   friend class RequestCoordinatorTest;
@@ skipping 42 matching lines @@
   // done with them.
   std::set<int64_t> disabled_requests_;
   // Calling this returns to the scheduler across the JNI bridge.
   base::Callback<void(bool)> scheduler_callback_;
   // Logger to record events.
   RequestCoordinatorEventLogger event_logger_;
   // Timer to watch for pre-render attempts running too long.
   base::OneShotTimer watchdog_timer_;
   // Callback invoked when an immediate request is done (default empty).
   base::Callback<void(bool)> immediate_schedule_callback_;
+  // Used for potential immediate processing when we get network connection.
+  std::unique_ptr<ConnectionNotifier> connection_notifier_;
   // Allows us to pass a weak pointer to callbacks.
   base::WeakPtrFactory<RequestCoordinator> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RequestCoordinator);
 };
 
 }  // namespace offline_pages
 
 #endif  // COMPONENTS_OFFLINE_PAGES_BACKGROUND_REQUEST_COORDINATOR_H_