author | Markus Stange <mstange@themasta.com> |
Tue, 18 Feb 2020 16:23:50 +0000 | |
changeset 514455 | df596657bebcb96b917d75ff452316bbe8140a1a |
parent 514454 | 8dbb70090704721d93881f5a4d419a3722abac8f |
child 514456 | a169b2355d8d1e37b5bd76404155ddf72bc0b620 |
push id | 107600 |
push user | mstange@themasta.com |
push date | Tue, 18 Feb 2020 18:02:52 +0000 |
treeherder | autoland@df596657bebc [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | gregtatum |
bugs | 1615436 |
milestone | 75.0a1 |
first release with | nightly linux32
df596657bebc
/
75.0a1
/
20200218213359
/
files
nightly linux64
df596657bebc
/
75.0a1
/
20200218213359
/
files
nightly mac
df596657bebc
/
75.0a1
/
20200218213359
/
files
nightly win32
df596657bebc
/
75.0a1
/
20200218213359
/
files
nightly win64
df596657bebc
/
75.0a1
/
20200218213359
/
files
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
releases | nightly linux32
75.0a1
/
20200218213359
/
pushlog to previous
nightly linux64
75.0a1
/
20200218213359
/
pushlog to previous
nightly mac
75.0a1
/
20200218213359
/
pushlog to previous
nightly win32
75.0a1
/
20200218213359
/
pushlog to previous
nightly win64
75.0a1
/
20200218213359
/
pushlog to previous
|
--- a/devtools/client/performance-new/@types/perf.d.ts +++ b/devtools/client/performance-new/@types/perf.d.ts @@ -84,20 +84,18 @@ export type RecordingState = | "available-to-record" // An async request has been sent to start the profiler. | "request-to-start-recording" // An async request has been sent to get the profile and stop the profiler. | "request-to-get-profile-and-stop-profiler" // An async request has been sent to stop the profiler. | "request-to-stop-profiler" // The profiler notified us that our request to start it actually started - // it. + // it, or it was already started. | "recording" - // Some other code with access to the profiler started it. - | "other-is-recording" // Profiling is not available when in private browsing mode. | "locked-by-private-browsing"; // We are currently migrating to a new UX workflow with about:profiling. // This type provides an easy way to change the implementation based // on context. export type PageContext = "popup" | "devtools" | "aboutprofiling";
--- a/devtools/client/performance-new/components/ProfilerEventHandling.js +++ b/devtools/client/performance-new/components/ProfilerEventHandling.js @@ -37,17 +37,16 @@ */ "use strict"; const { PureComponent } = require("devtools/client/shared/vendor/react"); const { connect } = require("devtools/client/shared/vendor/react-redux"); const actions = require("devtools/client/performance-new/store/actions"); const selectors = require("devtools/client/performance-new/store/selectors"); -const { UnhandledCaseError } = require("devtools/client/performance-new/utils"); /** * This component state changes for the performance recording. e.g. If the profiler * suddenly becomes unavailable, it needs to react to those changes, and update the * recordingState in the store. * * @extends {React.PureComponent<Props>} */ @@ -61,17 +60,17 @@ class ProfilerEventHandling extends Pure this ); this.handlePrivateBrowsingEnding = this.handlePrivateBrowsingEnding.bind( this ); } componentDidMount() { - const { perfFront, reportProfilerReady, pageContext } = this.props; + const { perfFront, reportProfilerReady } = this.props; // Ask for the initial state of the profiler. Promise.all([ perfFront.isActive(), perfFront.isSupportedPlatform(), perfFront.isLockedForPrivateBrowsing(), ]).then(results => { const [ @@ -82,32 +81,17 @@ class ProfilerEventHandling extends Pure let recordingState = this.props.recordingState; // It's theoretically possible we got an event that already let us know about // the current state of the profiler. if (recordingState === "not-yet-known" && isSupportedPlatform) { if (isLockedForPrivateBrowsing) { recordingState = "locked-by-private-browsing"; } else if (isActive) { - switch (pageContext) { - case "popup": - case "aboutprofiling": - // These page contexts are in global control of the profiler, so allow it - // to take control of recording. - recordingState = "recording"; - break; - case "devtools": - // The DevTools performance recording shouldn't take control of others - // use of the Gecko Profiler, since it's more interested in the current - // page. - recordingState = "other-is-recording"; - break; - default: - throw new UnhandledCaseError(pageContext, "PageContext"); - } + recordingState = "recording"; } else { recordingState = "available-to-record"; } } reportProfilerReady(isSupportedPlatform, recordingState); // If this component is inside the popup, then report it being ready so that // it will show. This defers the initial visibility of the popup until the @@ -137,67 +121,50 @@ class ProfilerEventHandling extends Pure componentWillUnmount() { switch (this.props.recordingState) { case "not-yet-known": case "available-to-record": case "request-to-stop-profiler": case "request-to-get-profile-and-stop-profiler": case "locked-by-private-browsing": - case "other-is-recording": // Do nothing for these states. break; case "recording": case "request-to-start-recording": this.props.perfFront.stopProfilerAndDiscardProfile(); break; default: throw new Error("Unhandled recording state."); } } handleProfilerStarting() { - const { changeRecordingState, recordingState, pageContext } = this.props; + const { changeRecordingState, recordingState } = this.props; switch (recordingState) { case "not-yet-known": // We couldn't have started it yet, so it must have been someone // else. (fallthrough) case "available-to-record": // We aren't recording, someone else started it up. (fallthrough) case "request-to-stop-profiler": // We requested to stop the profiler, but someone else already started // it up. (fallthrough) case "request-to-get-profile-and-stop-profiler": - switch (pageContext) { - case "popup": - case "aboutprofiling": - // These page contexts are in global control of the profiler, so allow it - // to take control of recording. - changeRecordingState("recording"); - break; - case "devtools": - // The DevTools performance recording shouldn't take control of others - // use of the Gecko Profiler, since it's more interested in the current - // page. - changeRecordingState("other-is-recording"); - break; - default: - throw new UnhandledCaseError(pageContext, "PageContext"); - } + changeRecordingState("recording"); break; case "request-to-start-recording": // Wait for the profiler to tell us that it has started. changeRecordingState("recording"); break; case "locked-by-private-browsing": - case "other-is-recording": case "recording": // These state cases don't make sense to happen, and means we have a logical // fallacy somewhere. throw new Error( "The profiler started recording, when it shouldn't have " + `been able to. Current state: "${recordingState}"` ); default: @@ -206,17 +173,16 @@ class ProfilerEventHandling extends Pure } handleProfilerStopping() { const { changeRecordingState, recordingState } = this.props; switch (recordingState) { case "not-yet-known": case "request-to-get-profile-and-stop-profiler": case "request-to-stop-profiler": - case "other-is-recording": changeRecordingState("available-to-record"); break; case "request-to-start-recording": // Highly unlikely, but someone stopped the recorder, this is fine. // Do nothing (fallthrough). case "locked-by-private-browsing": // The profiler is already locked, so we know about this already. @@ -241,17 +207,16 @@ class ProfilerEventHandling extends Pure const { recordingState, changeRecordingState } = this.props; switch (recordingState) { case "request-to-get-profile-and-stop-profiler": // This one is a tricky case. Go ahead and act like nothing went wrong, maybe // it will resolve correctly? (fallthrough) case "request-to-stop-profiler": case "available-to-record": - case "other-is-recording": case "not-yet-known": changeRecordingState("locked-by-private-browsing"); break; case "request-to-start-recording": case "recording": changeRecordingState("locked-by-private-browsing", { didRecordingUnexpectedlyStopped: false,
--- a/devtools/client/performance-new/components/RecordingButton.js +++ b/devtools/client/performance-new/components/RecordingButton.js @@ -180,23 +180,16 @@ class RecordingButton extends PureCompon onClick: this._getProfileAndStopProfiler, disabled: recordingState === "request-to-start-recording", additionalButton: { label: "Cancel recording", onClick: stopProfilerAndDiscardProfile, }, }); - case "other-is-recording": - return this.renderButton({ - label: "Stop and discard the other recording", - onClick: stopProfilerAndDiscardProfile, - additionalMessage: "Another tool is currently recording.", - }); - case "locked-by-private-browsing": return this.renderButton({ label: span( null, img({ className: "perf-button-image", src: "chrome://devtools/skin/images/tool-profiler.svg", }),
--- a/devtools/client/performance-new/test/chrome/test_perf-state-02.html +++ b/devtools/client/performance-new/test/chrome/test_perf-state-02.html @@ -35,22 +35,23 @@ await perfFrontMock._flushAsyncQueue(); mountComponent(); is(getRecordingState(), "not-yet-known", "The component at first is in an unknown state."); await perfFrontMock._flushAsyncQueue(); - is(getRecordingState(), "other-is-recording", - "The profiler is not available to record."); + is(getRecordingState(), "recording", + "The profiler is reflecting the recording state."); - const button = container.querySelector("button"); - ok(button, "Selected a button on the component"); - button.click(); + const [captureButton, stopButton] = container.querySelectorAll("button"); + ok(captureButton, "Selected capture button on the component"); + ok(stopButton, "Selected stop button on the component"); + stopButton.click(); is(getRecordingState(), "request-to-stop-profiler", "We can request to stop the profiler."); await perfFrontMock._flushAsyncQueue(); is(getRecordingState(), "available-to-record", "The profiler is now available to record."); }); </script>