Unify the TimeoutTelemetry methods to make budget collection easier

RESOLVED FIXED in Firefox 56

Status

()

Core
DOM
RESOLVED FIXED
6 months ago
2 months ago

People

(Reporter: farre, Assigned: farre)

Tracking

unspecified
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Assignee)

Description

6 months ago
Here we should also be able to move this class to its own file.
(Assignee)

Updated

6 months ago
Assignee: nobody → afarre
Blocks: 1362322
(Assignee)

Updated

6 months ago
No longer blocks: 1362322
(Assignee)

Updated

6 months ago
Blocks: 1362322
(Assignee)

Comment 1

6 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b57ab548867e614348948c70e41e569ffbb9a95d
(Assignee)

Comment 2

6 months ago
Created attachment 8878516 [details] [diff] [review]
0001-Bug-1373536-Unify-execution-measurements.-r-bkelly.patch
Attachment #8878516 - Flags: review?(bkelly)
(Assignee)

Comment 3

6 months ago
Created attachment 8878517 [details] [diff] [review]
0002-Bug-1373536-Move-TimeoutBudgetManager-to-its-own-fil.patch
Attachment #8878517 - Flags: review?(bkelly)

Comment 4

6 months ago
Comment on attachment 8878516 [details] [diff] [review]
0001-Bug-1373536-Unify-execution-measurements.-r-bkelly.patch

Review of attachment 8878516 [details] [diff] [review]:
-----------------------------------------------------------------

Seems reasonable.  I flagged a few issues in the existing code to fix in a follow-up, though.

::: dom/base/TimeoutManager.cpp
@@ +27,3 @@
>  {
>  public:
> +  static TimeoutBudgetManager& Get();

If you are going to have a static instance, then the public API here could just be static functions instead of class methods.  That makes the call sites a bit simpler to read.

This can be a follow-on patch.

@@ +27,4 @@
>  {
>  public:
> +  static TimeoutBudgetManager& Get();
> +  TimeoutBudgetManager() : mLastCollection(TimeStamp::Now()) {}

As it stands code can accidentally create a TimeoutBudgetManager() since the constructor is public.  This should probably be made private or completely hidden by switching to static functions instead of Get()+methods.

Again, this can be a follow-on patch.

@@ +33,3 @@
>    void StopRecording();
> +  TimeDuration RecordExecution(const TimeStamp& aNow,
> +                               bool aIsTracking,

Why switch to passing a bare bool here instead of the Timeout object?  Passing the Timeout object seems nicer to me.

@@ +50,5 @@
>    TimeStamp mStart;
>    TimeStamp mLastCollection;
>  };
>  
> +static TimeoutBudgetManager gTimeoutBudgetManager;

This emits a static initializer executed at program start.  We generally try to avoid these.

Either do this:

  StaticAutoPtr<TimeoutBudgetManager> gTimeoutBudgetManager;

  TimeoutBudgetManager&
  TimeoutBudgetManager::Get()
  {
    MOZ_ASSERT(NS_IsMainThread());
    if (!gTimeoutBudgetManager) {
      gTimeoutBudgetManager = new TimeoutBudgetManager();
    }
    return *gTimeoutBudgetManager;
  }

Or this:

  TimeoutBudgetManager&
  TimeoutBudgetManager::Get()
  {
    static TimeoutBudgetManager gRef;
    return gRef;
  }

This can be a follow-up.

@@ +97,5 @@
>        mTelemetryData.mForegroundNonTracking += duration;
>      }
>    }
> +
> +  return duration;

Why does this return a duration?  AFAICT its not used.

@@ +267,5 @@
> +  TimeStamp now = TimeStamp::Now();
> +  if (aRunningTimeout) {
> +    // If we're running a timeout callback, record any execution until
> +    // now.
> +    TimeoutBudgetManager::Get().RecordExecution(

Should this be using the return value from RecordExecution() somehow?
Attachment #8878516 - Flags: review?(bkelly) → review+

Comment 5

6 months ago
Comment on attachment 8878517 [details] [diff] [review]
0002-Bug-1373536-Move-TimeoutBudgetManager-to-its-own-fil.patch

Review of attachment 8878517 [details] [diff] [review]:
-----------------------------------------------------------------

rs+ as I assume this is just a straight move.

I think the suggestion to use simple functions instead of a static Get()+methods should be easier now.
Attachment #8878517 - Flags: review?(bkelly) → review+
(Assignee)

Comment 6

6 months ago
I'll make it happen. Thanks
(Assignee)

Comment 7

6 months ago
Created attachment 8879041 [details] [diff] [review]
0002-Bug-1373536-Move-TimeoutBudgetManager-to-its-own-fil.patch

Added a missing include. Letting the r+ carry over.
Attachment #8878517 - Attachment is obsolete: true
Attachment #8879041 - Flags: review+
(Assignee)

Comment 8

6 months ago
Created attachment 8879044 [details] [diff] [review]
0003-Bug-1373536-Clean-up-static-API-of-TimeoutBudgetMana.patch

Clean up TimeoutBudgetManager API.
Attachment #8879044 - Flags: review?(bkelly)

Comment 9

6 months ago
Comment on attachment 8879044 [details] [diff] [review]
0003-Bug-1373536-Clean-up-static-API-of-TimeoutBudgetMana.patch

Review of attachment 8879044 [details] [diff] [review]:
-----------------------------------------------------------------

Overall looks reasonable, but I don't know enough about the telemetry decision to know if using mWindow.IsBackgroundInternal() is ok instead of IsBackground().  It seems to me we should not change the meaning of a telemetry probe without good reason.

Redirecting review to Olli since he originally reviewed the telemetry patch.

::: dom/base/TimeoutManager.cpp
@@ +154,5 @@
>      return;
>    }
>  
> +  TimeoutBudgetManager::RecordExecution(
> +    aRunningTimeout, aTimeout, mWindow.IsBackgroundInternal());

This changes from IsBackground() to mWindow.IsBackgroundInternal().
Attachment #8879044 - Flags: review?(bugs)
Attachment #8879044 - Flags: review?(bkelly)
Attachment #8879044 - Flags: feedback+
(Assignee)

Comment 10

6 months ago
This change would make timeouts from a background window playing audio count as actually being in the background. I think it's actually wrong to count them as foreground as we do now, since then we can't know if the dip we saw after throttling tracking timeouts were due to throttling or due to how we counted windows playing audio. This becomes more important when the budget comes into play, i.e. that it's important to be very strict in criteria. Making this change now, and being aware of that change is better to do no than later.
Comment on attachment 8879044 [details] [diff] [review]
0003-Bug-1373536-Clean-up-static-API-of-TimeoutBudgetMana.patch

>+TimeoutBudgetManager::RecordExecutionImpl(const Timeout* aRunningTimeout,
>+                                          const Timeout* aTimeout,
What these params mean?

>+                                          bool aIsBackground)
> {
>-  mStart = aNow;
>-}
>+  TimeStamp now = TimeStamp::Now();
>+  if (aRunningTimeout) {
>+    // If we're running a timeout callback, record any execution until
>+    // now.
>+    RecordExecution(now, aRunningTimeout, aIsBackground);
>+    MaybeCollectTelemetry(now);
>+  }
> 

I'm having trouble to understand what RecordExecutionImpl means vs RecordExecution.
From their names I would have assumed RecordExecution to call RecordExecutionImpl, but apparently it is other way round.



The old code looks easier to understand. Start and Stop and Record. The new naming isn't clear, at least not for me.
Attachment #8879044 - Flags: review?(bugs) → review-
(Assignee)

Comment 12

6 months ago
Created attachment 8879442 [details] [diff] [review]
0003-Bug-1373536-Clean-up-static-API-of-TimeoutBudgetMana.patch

Sure, let's run with the old way.

 Made sure to move the static initialization of the TimeoutBudgetManager and making the constructor private. Didn't re-work the API to be static though, since that doesn't make any sense since all calls are coming through TimeoutManager::RecordExecution anyway.
Attachment #8879044 - Attachment is obsolete: true
Attachment #8879442 - Flags: review?(bugs)
Attachment #8879442 - Flags: review?(bugs) → review+
(Assignee)

Comment 13

6 months ago
Try looks ok: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a485d243de30c53d95ca0683c9ad3011c1f60fab
(Assignee)

Updated

6 months ago
Keywords: checkin-needed

Comment 14

6 months ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/10676fd9e6ad
Unify execution measurements. r=bkelly
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bde0b2dd768
Move TimeoutBudgetManager to its own file. r=bkelly
https://hg.mozilla.org/integration/mozilla-inbound/rev/d377000dbcc9
Clean up static API of TimeoutBudgetManager. r=smaug
Keywords: checkin-needed
Backed out for failing asan-fuzzing at dom/base/TimeoutHandler.h:26 with unknown type name 'NS_DECL_CYCLE_COLLECTION_CLASS' and more:

https://hg.mozilla.org/integration/mozilla-inbound/rev/ae3f01ce98394cbde7d0aaabf72b687b716493e3
https://hg.mozilla.org/integration/mozilla-inbound/rev/6fa30b94ca3dac394470710ec66ae87324476505
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e783d1f2c6b48949732543bf3fbac9c434debd7

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=d377000dbcc95a630fb9b01eef81cbe5eaaaed65&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=108551139&repo=mozilla-inbound

task 2017-06-20T14:53:42.826522Z] 14:53:42     INFO -  In file included from /home/worker/workspace/build/src/obj-firefox/dom/base/Unified_cpp_dom_base4.cpp:2:
[task 2017-06-20T14:53:42.826623Z] 14:53:42     INFO -  In file included from /home/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:7:
[task 2017-06-20T14:53:42.826705Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.h:26:3: error: unknown type name 'NS_DECL_CYCLE_COLLECTION_CLASS'
[task 2017-06-20T14:53:42.826749Z] 14:53:42     INFO -    NS_DECL_CYCLE_COLLECTION_CLASS(TimeoutHandler)
[task 2017-06-20T14:53:42.826773Z] 14:53:42     INFO -    ^
[task 2017-06-20T14:53:42.826829Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.h:26:49: error: expected ';' at end of declaration list
[task 2017-06-20T14:53:42.826879Z] 14:53:42     INFO -    NS_DECL_CYCLE_COLLECTION_CLASS(TimeoutHandler)
[task 2017-06-20T14:53:42.826923Z] 14:53:42     INFO -                                                  ^
[task 2017-06-20T14:53:42.826968Z] 14:53:42     INFO -                                                  ;
[task 2017-06-20T14:53:42.830095Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.h:42:3: error: unknown type name 'nsCString'
[task 2017-06-20T14:53:42.830154Z] 14:53:42     INFO -    nsCString mFileName;
[task 2017-06-20T14:53:42.830184Z] 14:53:42     INFO -    ^
[task 2017-06-20T14:53:42.830239Z] 14:53:42     INFO -  In file included from /home/worker/workspace/build/src/obj-firefox/dom/base/Unified_cpp_dom_base4.cpp:2:
[task 2017-06-20T14:53:42.830308Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:15:3: error: use of undeclared identifier 'nsJSUtils'
[task 2017-06-20T14:53:42.830362Z] 14:53:42     INFO -    nsJSUtils::GetCallingLocation(aCx, mFileName, &mLineNo, &mColumn);
[task 2017-06-20T14:53:42.830396Z] 14:53:42     INFO -    ^
[task 2017-06-20T14:53:42.832074Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:19:17: error: out-of-line definition of 'Call' does not match any declaration in 'mozilla::dom::TimeoutHandler'
[task 2017-06-20T14:53:42.832127Z] 14:53:42     INFO -  TimeoutHandler::Call()
[task 2017-06-20T14:53:42.832163Z] 14:53:42     INFO -                  ^~~~
[task 2017-06-20T14:53:42.837714Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:33:1: error: unknown type name 'NS_IMPL_CYCLE_COLLECTION_0'
[task 2017-06-20T14:53:42.837792Z] 14:53:42     INFO -  NS_IMPL_CYCLE_COLLECTION_0(TimeoutHandler)
[task 2017-06-20T14:53:42.837823Z] 14:53:42     INFO -  ^
[task 2017-06-20T14:53:42.837881Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:33:43: error: expected ';' after top level declarator
[task 2017-06-20T14:53:42.837930Z] 14:53:42     INFO -  NS_IMPL_CYCLE_COLLECTION_0(TimeoutHandler)
[task 2017-06-20T14:53:42.837975Z] 14:53:42     INFO -                                            ^
[task 2017-06-20T14:53:42.838021Z] 14:53:42     INFO -                                            ;
[task 2017-06-20T14:53:42.840076Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:35:33: error: 'TimeoutHandler' does not refer to a value
[task 2017-06-20T14:53:42.840485Z] 14:53:42     INFO -  NS_IMPL_CYCLE_COLLECTING_ADDREF(TimeoutHandler)
[task 2017-06-20T14:53:42.840841Z] 14:53:42     INFO -                                  ^
[task 2017-06-20T14:53:42.841211Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.h:20:7: note: declared here
[task 2017-06-20T14:53:42.841537Z] 14:53:42     INFO -  class TimeoutHandler : public nsITimeoutHandler
[task 2017-06-20T14:53:42.841858Z] 14:53:42     INFO -        ^
[task 2017-06-20T14:53:42.842198Z] 14:53:42     INFO -  In file included from /home/worker/workspace/build/src/obj-firefox/dom/base/Unified_cpp_dom_base4.cpp:2:
[task 2017-06-20T14:53:42.842547Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:36:34: error: 'TimeoutHandler' does not refer to a value
[task 2017-06-20T14:53:42.842874Z] 14:53:42     INFO -  NS_IMPL_CYCLE_COLLECTING_RELEASE(TimeoutHandler)
[task 2017-06-20T14:53:42.844746Z] 14:53:42     INFO -                                   ^
[task 2017-06-20T14:53:42.845348Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.h:20:7: note: declared here
[task 2017-06-20T14:53:42.845686Z] 14:53:42     INFO -  class TimeoutHandler : public nsITimeoutHandler
[task 2017-06-20T14:53:42.846011Z] 14:53:42     INFO -        ^
[task 2017-06-20T14:53:42.846359Z] 14:53:42     INFO -  In file included from /home/worker/workspace/build/src/obj-firefox/dom/base/Unified_cpp_dom_base4.cpp:2:
[task 2017-06-20T14:53:42.850243Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:38:1: error: unknown type name 'NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION'
[task 2017-06-20T14:53:42.850472Z] 14:53:42     INFO -  NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(TimeoutHandler)
[task 2017-06-20T14:53:42.850685Z] 14:53:42     INFO -  ^
[task 2017-06-20T14:53:42.850954Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:38:56: error: expected ';' after top level declarator
[task 2017-06-20T14:53:42.851172Z] 14:53:42     INFO -  NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(TimeoutHandler)
[task 2017-06-20T14:53:42.851406Z] 14:53:42     INFO -                                                         ^
[task 2017-06-20T14:53:42.851649Z] 14:53:42     INFO -                                                         ;
[task 2017-06-20T14:53:42.851887Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:39:3: error: expected unqualified-id
[task 2017-06-20T14:53:42.852103Z] 14:53:42     INFO -    NS_INTERFACE_MAP_ENTRY(nsITimeoutHandler)
[task 2017-06-20T14:53:42.852324Z] 14:53:42     INFO -    ^
[task 2017-06-20T14:53:42.852592Z] 14:53:42     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/nsISupportsImpl.h:943:49: note: expanded from macro 'NS_INTERFACE_MAP_ENTRY'
[task 2017-06-20T14:53:42.852817Z] 14:53:42     INFO -  #define NS_INTERFACE_MAP_ENTRY(_interface)      NS_IMPL_QUERY_BODY(_interface)
[task 2017-06-20T14:53:42.853046Z] 14:53:42     INFO -                                                  ^
[task 2017-06-20T14:53:42.853371Z] 14:53:42     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/nsISupportsImpl.h:866:3: note: expanded from macro 'NS_IMPL_QUERY_BODY'
[task 2017-06-20T14:53:42.853601Z] 14:53:42     INFO -    else
[task 2017-06-20T14:53:42.853824Z] 14:53:42     INFO -    ^
[task 2017-06-20T14:53:42.854084Z] 14:53:42     INFO -  In file included from /home/worker/workspace/build/src/obj-firefox/dom/base/Unified_cpp_dom_base4.cpp:2:
[task 2017-06-20T14:53:42.854308Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:40:1: error: expected unqualified-id
[task 2017-06-20T14:53:42.854517Z] 14:53:42     INFO -  NS_INTERFACE_MAP_END
[task 2017-06-20T14:53:42.854727Z] 14:53:42     INFO -  ^
[task 2017-06-20T14:53:42.854980Z] 14:53:42     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/nsISupportsImpl.h:949:49: note: expanded from macro 'NS_INTERFACE_MAP_END'
[task 2017-06-20T14:53:42.855195Z] 14:53:42     INFO -  #define NS_INTERFACE_MAP_END                    NS_IMPL_QUERY_TAIL_GUTS
[task 2017-06-20T14:53:42.855416Z] 14:53:42     INFO -                                                  ^
[task 2017-06-20T14:53:42.855670Z] 14:53:42     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/nsISupportsImpl.h:887:3: note: expanded from macro 'NS_IMPL_QUERY_TAIL_GUTS'
[task 2017-06-20T14:53:42.855877Z] 14:53:42     INFO -    if ( !foundInterface )                                                      \
[task 2017-06-20T14:53:42.856084Z] 14:53:42     INFO -    ^
[task 2017-06-20T14:53:42.856380Z] 14:53:42     INFO -  In file included from /home/worker/workspace/build/src/obj-firefox/dom/base/Unified_cpp_dom_base4.cpp:2:
[task 2017-06-20T14:53:42.856610Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:40:1: error: expected unqualified-id
[task 2017-06-20T14:53:42.856837Z] 14:53:42     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/nsISupportsImpl.h:949:49: note: expanded from macro 'NS_INTERFACE_MAP_END'
[task 2017-06-20T14:53:42.857042Z] 14:53:42     INFO -  #define NS_INTERFACE_MAP_END                    NS_IMPL_QUERY_TAIL_GUTS
[task 2017-06-20T14:53:42.857255Z] 14:53:42     INFO -                                                  ^
[task 2017-06-20T14:53:42.857507Z] 14:53:42     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/nsISupportsImpl.h:893:3: note: expanded from macro 'NS_IMPL_QUERY_TAIL_GUTS'
[task 2017-06-20T14:53:42.857721Z] 14:53:42     INFO -    else                                                                        \
[task 2017-06-20T14:53:42.857944Z] 14:53:42     INFO -    ^
[task 2017-06-20T14:53:42.858212Z] 14:53:42     INFO -  In file included from /home/worker/workspace/build/src/obj-firefox/dom/base/Unified_cpp_dom_base4.cpp:2:
[task 2017-06-20T14:53:42.858440Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:40:1: error: C++ requires a type specifier for all declarations
[task 2017-06-20T14:53:42.858665Z] 14:53:42     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/nsISupportsImpl.h:949:49: note: expanded from macro 'NS_INTERFACE_MAP_END'
[task 2017-06-20T14:53:42.858872Z] 14:53:42     INFO -  #define NS_INTERFACE_MAP_END                    NS_IMPL_QUERY_TAIL_GUTS
[task 2017-06-20T14:53:42.859068Z] 14:53:42     INFO -                                                  ^
[task 2017-06-20T14:53:42.859291Z] 14:53:42     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/nsISupportsImpl.h:898:4: note: expanded from macro 'NS_IMPL_QUERY_TAIL_GUTS'
[task 2017-06-20T14:53:42.859478Z] 14:53:42     INFO -    *aInstancePtr = foundInterface;                                             \
[task 2017-06-20T14:53:42.859657Z] 14:53:42     INFO -     ^
[task 2017-06-20T14:53:42.868072Z] 14:53:42     INFO -  In file included from /home/worker/workspace/build/src/obj-firefox/dom/base/Unified_cpp_dom_base4.cpp:2:
[task 2017-06-20T14:53:42.868378Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:40:1: error: use of undeclared identifier 'foundInterface'
[task 2017-06-20T14:53:42.868623Z] 14:53:42     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/nsISupportsImpl.h:949:49: note: expanded from macro 'NS_INTERFACE_MAP_END'
[task 2017-06-20T14:53:42.868827Z] 14:53:42     INFO -  #define NS_INTERFACE_MAP_END                    NS_IMPL_QUERY_TAIL_GUTS
[task 2017-06-20T14:53:42.869025Z] 14:53:42     INFO -                                                  ^
[task 2017-06-20T14:53:42.869351Z] 14:53:42     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/nsISupportsImpl.h:898:19: note: expanded from macro 'NS_IMPL_QUERY_TAIL_GUTS'
[task 2017-06-20T14:53:42.869700Z] 14:53:42     INFO -    *aInstancePtr = foundInterface;                                             \
[task 2017-06-20T14:53:42.869980Z] 14:53:42     INFO -                    ^
[task 2017-06-20T14:53:42.870295Z] 14:53:42     INFO -  In file included from /home/worker/workspace/build/src/obj-firefox/dom/base/Unified_cpp_dom_base4.cpp:2:
[task 2017-06-20T14:53:42.870598Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:40:1: error: expected unqualified-id
[task 2017-06-20T14:53:42.870923Z] 14:53:42     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/nsISupportsImpl.h:949:49: note: expanded from macro 'NS_INTERFACE_MAP_END'
[task 2017-06-20T14:53:42.871221Z] 14:53:42     INFO -  #define NS_INTERFACE_MAP_END                    NS_IMPL_QUERY_TAIL_GUTS
[task 2017-06-20T14:53:42.871507Z] 14:53:42     INFO -                                                  ^
[task 2017-06-20T14:53:42.871820Z] 14:53:42     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/nsISupportsImpl.h:899:3: note: expanded from macro 'NS_IMPL_QUERY_TAIL_GUTS'
[task 2017-06-20T14:53:42.872112Z] 14:53:42     INFO -    return status;                                                              \
[task 2017-06-20T14:53:42.872437Z] 14:53:42     INFO -    ^
[task 2017-06-20T14:53:42.872751Z] 14:53:42     INFO -  In file included from /home/worker/workspace/build/src/obj-firefox/dom/base/Unified_cpp_dom_base4.cpp:2:
[task 2017-06-20T14:53:42.873071Z] 14:53:42     INFO -  /home/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:43:1: error: extraneous closing brace ('}')
[task 2017-06-20T14:53:42.873348Z] 14:53:42     INFO -  } // namespace mozilla
[task 2017-06-20T14:53:42.873644Z] 14:53:42     INFO -  ^
Flags: needinfo?(afarre)
(Assignee)

Comment 16

6 months ago
Created attachment 8879948 [details] [diff] [review]
0004-Bug-1373536-Add-missing-includes-due-to-added-file.-.patch

Build error fallout due to missing includes only visible after adding new file.
Flags: needinfo?(afarre)
Attachment #8879948 - Flags: review?(bugs)
Attachment #8879948 - Flags: review?(bugs) → review+
(Assignee)

Comment 17

6 months ago
Try looks good again:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b56d9b13bffa905f83ac0df99b5f7c4fa7a2f27

The Windows 10 x64 asan failures can't possibly be correct to begin with.
(Assignee)

Updated

6 months ago
Keywords: checkin-needed

Comment 18

6 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b2e22a96e7c5
Unify execution measurements. r=bkelly
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7e6ff8f2f3a
Move TimeoutBudgetManager to its own file. r=bkelly
https://hg.mozilla.org/integration/mozilla-inbound/rev/5cc17c25b240
Clean up static API of TimeoutBudgetManager. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/be145d9c3ff4
Add missing includes due to added file. r=smaug
Keywords: checkin-needed

Comment 19

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b2e22a96e7c5
https://hg.mozilla.org/mozilla-central/rev/a7e6ff8f2f3a
https://hg.mozilla.org/mozilla-central/rev/5cc17c25b240
https://hg.mozilla.org/mozilla-central/rev/be145d9c3ff4
Status: NEW → RESOLVED
Last Resolved: 6 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Blocks: 1410146
(Assignee)

Updated

2 months ago
No longer blocks: 1410146
You need to log in before you can comment on or make changes to this bug.