Bug 32331 - Web Inspector: Get rid of synchrnonous debuggerEnabled, profilerEnabled calls.
Summary: Web Inspector: Get rid of synchrnonous debuggerEnabled, profilerEnabled calls.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-12-09 10:56 PST by Pavel Feldman
Modified: 2010-03-03 09:25 PST (History)
5 users (show)

See Also:


Attachments
[PATCH] Proposed change. (17.78 KB, patch)
2009-12-09 11:04 PST, Pavel Feldman
no flags Details | Formatted Diff | Diff
[PATCH] Proposed change. (10.00 KB, patch)
2010-03-02 22:59 PST, Pavel Feldman
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2009-12-09 10:56:08 PST
Two reasons behind the change:

1) debuggerEnabled / profilerEnabled / resourceTracking enabled are sync methods on InspectorBackend and we don't like it for obvious reasons
2) corresponding settings really make sense only when frontend is present. Like debuggerEnabled is really not handled well in inspector contorller - all it does is sets the flag on frontend to enable debugger from within JavaScript. It would be more logical for frontend to read the setting itself and enable / disable debugger depending on the value.

Btw, in Chromium, we don't enable / disable debugger and profiler panels. They are just always enabled when frontend is up. Do you think it still makes sense for Web Inspector to have explicit control over those?
Comment 1 Pavel Feldman 2009-12-09 11:04:31 PST
Created attachment 44547 [details]
[PATCH] Proposed change.
Comment 2 Eric Seidel (no email) 2009-12-28 23:22:32 PST
Ping?  Looks like this has been r+'d for about 2 weeks now.  I assume folks are just busy with the holidays?
Comment 3 Pavel Feldman 2009-12-29 12:15:28 PST
Comment on attachment 44547 [details]
[PATCH] Proposed change.

Things got changed a bit, so I'll need to do some merging. Clearing the flag for now.
Comment 4 Pavel Feldman 2010-03-02 22:53:02 PST
While making this change I thought that maybe single place for storing settings should reside in backend, not the frontend, so I did not land it and refactored settings instead.

Now it is time to revisit it. Goal is still the same, no sync operations between the backend and fronend.
Comment 5 Pavel Feldman 2010-03-02 22:59:23 PST
Created attachment 49881 [details]
[PATCH] Proposed change.
Comment 6 Pavel Feldman 2010-03-03 09:25:01 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/inspector/InspectorBackend.cpp
	M	WebCore/inspector/InspectorBackend.h
	M	WebCore/inspector/InspectorBackend.idl
	M	WebCore/inspector/InspectorController.cpp
	M	WebCore/inspector/front-end/BreakpointsSidebarPane.js
	M	WebCore/inspector/front-end/ProfilesPanel.js
	M	WebCore/inspector/front-end/ScriptsPanel.js
	M	WebCore/inspector/front-end/Settings.js
	M	WebCore/inspector/front-end/inspector.js
	M	WebKit/chromium/src/js/DevTools.js
Committed r55464