* SUMMARY Now that the backend can have extra agents, inform the frontend about them so the frontend can lazily activate the domains and do any associated work that may be required.
<rdar://problem/18833087>
Created attachment 240720 [details] [PATCH] Proposed Fix This is the final functionality patch. After this should just be some cleanup and maybe a few bug fixes for the extra agents case as needed.
Comment on attachment 240720 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=240720&action=review > Source/JavaScriptCore/inspector/protocol/Inspector.json:41 > + "description": "Fired when the backend has alternate domain that need to be activated.", Typo: "domain" => "domains"
This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)
Comment on attachment 240720 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=240720&action=review > Source/WebInspectorUI/UserInterface/Protocol/InspectorObserver.js:89 > + case "DOMStorage": > + storageHandling = true; > + break; FWIW I was just including DOMStorage for testing. I have a feeling I'll be able to remove all the cases for DOMStorage (objc generator and here).
Comment on attachment 240720 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=240720&action=review Aside from handleExtraDomainActivation methods (which are not a big deal), it looks good to me. Any comments from Tim? > Source/JavaScriptCore/inspector/agents/InspectorAgent.cpp:130 > + for (auto domainName : extraDomains) auto& > Source/WebInspectorUI/UserInterface/Controllers/FrameResourceManager.js:70 > + PageAgent.enable(); Why do the managers decide what agents to enable, instead of WebInspector in Main.js? > Source/WebInspectorUI/UserInterface/Protocol/InspectorObserver.js:76 > + var resourceHandling = false; can/shouldHandleResources? > Source/WebInspectorUI/UserInterface/Protocol/InspectorObserver.js:98 > + WebInspector.resourceSidebarPanel.handleExtraDomainActivation(); Is this a layering violation? It seems to me like the manager should fire an event. In fact, why can't we enable agents here, and have the managers/UI listen for an event if they need to do anything else? At a glance it's two steps removed to see what agents actually get enabled per-domain. > Source/WebInspectorUI/UserInterface/Protocol/InspectorObserver.js:105 > + WebInspector.CSSCompletions.requestCSSNameCompletions(); same comment as above.
(In reply to comment #6) > Comment on attachment 240720 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=240720&action=review > > Aside from handleExtraDomainActivation methods (which are not a big deal), > it looks good to me. Any comments from Tim? > > > Source/JavaScriptCore/inspector/agents/InspectorAgent.cpp:130 > > + for (auto domainName : extraDomains) > > auto& > > > Source/WebInspectorUI/UserInterface/Controllers/FrameResourceManager.js:70 > > + PageAgent.enable(); > > Why do the managers decide what agents to enable, instead of WebInspector in > Main.js? I think that is just a holdover from the old frontend. That said, I want to eliminate enable/disable in the future. The backend should just auto-enable each agent. > > Source/WebInspectorUI/UserInterface/Protocol/InspectorObserver.js:76 > > + var resourceHandling = false; > > can/shouldHandleResources? > > > Source/WebInspectorUI/UserInterface/Protocol/InspectorObserver.js:98 > > + WebInspector.resourceSidebarPanel.handleExtraDomainActivation(); > > Is this a layering violation? It seems to me like the manager should fire an > event. Doh! You're totally right. > In fact, why can't we enable agents here, and have the managers/UI listen > for an event if they need to do anything else? At a glance it's two steps > removed to see what agents actually get enabled per-domain. I think that sounds fine. I'll clean this up and send out a new patch. Thanks
Created attachment 240750 [details] [PATCH] Proposed Fix Addressed review comments.
Created attachment 240752 [details] Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-14 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 240758 [details] [PATCH] Proposed Fix
Comment on attachment 240758 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=240758&action=review > Source/WebInspectorUI/ChangeLog:17 > + are activated, re-perform agent specific one time initialization, "When extra domains are activated, some agents need to be re-initialized. Dispatch a model event so views also know to re-initialize." > Source/WebInspectorUI/ChangeLog:18 > + and dispatch and event so others may perform re-initialization. typo: an event
http://trac.webkit.org/changeset/175478