Allow IPRO requests to use the Central Controller.

net/instaweb/rewriter/public/in_place_rewrite_context.h

@@ -114,6 +114,12 @@ class InPlaceRewriteContext : public SingleRewriteContext {
   // resource than wait for the optimization.
   virtual bool CreationLockBeforeStartFetch() const { return false; }
 
+  // The context nested inside this context can be scheduled via the
+  // CentralController. See comment in RewriteContext::ObtainLockForCreation.
+  bool ScheduleNestedContextViaCentalController() const override {
+    return true;
+  }
+
  private:
   friend class RecordingFetch;
   // Implements RewriteContext::Harvest().

net/instaweb/rewriter/public/rewrite_context.h

@@ -629,6 +629,15 @@ class RewriteContext {
   // true, allowing more intelligent prioritization.
   virtual bool ScheduleViaCentralController() { return false; }
 
+  // In general, ScheduleViaCentralController() is ignored for nested Contexts.
+  // However, in the case of (at least) IPRO we need to schedule the inner
+  // context via the Controller. This can be overridden by such contexts, which
+  // are DHCHECKed to have at most one nested context.
+  // See longer comment in ObtainLockForCreation implementation.
+  virtual bool ScheduleNestedContextViaCentalController() const {
+    return false;
+  }
+
   // Obtain a lock to create the resource. callback may not be invoked for an
   // indeterminate time.
   void ObtainLockForCreation(ServerContext* server_context, Function* callback);

net/instaweb/rewriter/rewrite_context.cc

@@ -1582,7 +1582,30 @@ void RewriteContext::OutputCacheMiss() {
 
 void RewriteContext::ObtainLockForCreation(ServerContext* server_context,
                                            Function* callback) {
-  if (ScheduleViaCentralController() && !has_parent()) {
+  // Because the CentralController can block indefinitely, it's important that
+  // any given sequence of rewrite only requests a single lock from it. For
+  // instance, if all the image rewrites within a css rewrite requested a
+  // controller lock it would be at best slow and could easily deadlock if
+  // insufficient "rewrite tokens" are available. In general we prevent this by
+  // only allowing "root" contexts to obtain a lock, ie: those without a parent.
+  // Unfortunately, in the case of IPRO the "interesting" context is nested
+  // inside an InPlaceRewriteContext. We don't want to require all IPRO requests
+  // go via the controller, since many are fast. So instead we have an
+  // escape-hatch that allows InPlaceRewriteContext to declare itself safe for
+  // nesting.
+  bool context_safe_for_controller = !has_parent();
+  if (has_parent() && !parent_->has_parent()) {
+    context_safe_for_controller =
+        parent_->ScheduleNestedContextViaCentalController();
+    if (context_safe_for_controller && parent_->num_nested() > 1) {
+      // If a context declares itself safe for nesting but actually has multiple
+      // nested contexts, it can cause the problems described above.
+      context_safe_for_controller = false;
+      LOG(DFATAL) << "Parent context declared itself safe for nesting, but it "
+                  << "has " << parent_->num_nested() << " children";
+    }
+  }
+  if (ScheduleViaCentralController() && context_safe_for_controller) {
     server_context->central_controller()->ScheduleRewrite(
         new TryLockFunction(LockName(), Driver()->rewrite_worker(), callback,
                             this));

pagespeed/apache/gzip_test.sh

@@ -50,7 +50,6 @@ start_test Efficacy of ModPagespeedFetchWithGzip
 
 # TODO(sligocki): The serf_fetch_bytes_count should be available on
 # this vhost's pagespeed_admin/statistics page. Why isn't it?
-GLOBAL_STATISTICS_URL="$PRIMARY_SERVER/pagespeed_global_admin/statistics?PageSpeed=off"
 STATS=$OUTDIR/gzip_efficacy_stats
 $WGET_DUMP $GLOBAL_STATISTICS_URL > $STATS.1
 

pagespeed/automatic/system_test_helpers.sh

@@ -139,6 +139,8 @@ EXAMPLE_ROOT=$PRIMARY_SERVER/mod_pagespeed_example
 # Currently we are, so disable that so that it doesn't spoil our stats.
 DEFAULT_STATISTICS_URL=$PRIMARY_SERVER/mod_pagespeed_statistics?PageSpeed=off
 STATISTICS_URL=${STATISTICS_URL:-$DEFAULT_STATISTICS_URL}
+DEFAULT_GLOBAL_STATISTICS_URL="$PRIMARY_SERVER/pagespeed_global_admin/statistics?PageSpeed=off"
+GLOBAL_STATISTICS_URL=${GLOBAL_STATISTICS_URL:-$DEFAULT_GLOBAL_STATISTICS_URL}
 BAD_RESOURCE_URL=$PRIMARY_SERVER/mod_pagespeed/W.bad.pagespeed.cf.hash.css
 MESSAGE_URL=$PRIMARY_SERVER/pagespeed_admin/message_history
 CONSOLE_URL=$PRIMARY_SERVER/pagespeed_admin/console

pagespeed/system/system_test.sh

@@ -373,6 +373,17 @@ echo $WGET_DUMP $TEST_ROOT/ipro/instant/deadline/purple.css
 OUT=$($WGET_DUMP $TEST_ROOT/ipro/instant/deadline/purple.css)
 check_from "$OUT" fgrep -q 'body{background:#9370db}'
 
+start_test IPRO requests are routed through the controller API
+
+STATS=$OUTDIR/controller_stats
+$WGET_DUMP $GLOBAL_STATISTICS_URL > $STATS.0
+
+OUT=$($WGET_DUMP $TEST_ROOT/ipro/instant/wait/purple.css?random=$RANDOM)
+check_from "$OUT" fgrep -q 'body{background:#9370db}'
+
+$WGET_DUMP $GLOBAL_STATISTICS_URL > $STATS.1
+check_stat $STATS.0 $STATS.1 named-lock-rewrite-scheduler-granted 1
+
 start_test json keeps its content type
 URL="$TEST_ROOT/example.json"
 OUT=$($WGET_DUMP "$URL?PageSpeed=off")