Bug 139981

Summary: Web Inspector: pass parameters to inspector instrumentation by reference where possible
Product: WebKit Reporter: Brian Burg <burg>
Component: Web InspectorAssignee: Brian Burg <burg>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, graouts, joepeck, jonowells, kling, mattbaker, nvasilyev, ossy, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 140098    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Fix EFL/GTK kling: review+

Description Brian Burg 2014-12-28 15:03:44 PST
Cleanup work.
Comment 1 Radar WebKit Bug Importer 2014-12-28 15:03:55 PST
<rdar://problem/19353559>
Comment 2 Brian Burg 2014-12-28 15:36:29 PST
Created attachment 243787 [details]
Patch
Comment 3 WebKit Commit Bot 2014-12-28 15:38:59 PST
Attachment 243787 [details] did not pass style-queue:


ERROR: Source/WebCore/inspector/InspectorInstrumentation.cpp:46:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 42 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Andreas Kling 2014-12-29 16:05:33 PST
Comment on attachment 243787 [details]
Patch

r=me, this is really neat!

Please fix the EFL & Gtk+ build failures before landing:

../../Source/WebCore/page/EventHandler.cpp: In member function 'bool WebCore::EventHandler::handleTouchEvent(const WebCore::PlatformTouchEvent&)':
../../Source/WebCore/page/EventHandler.cpp:3869:82: error: no matching function for call to 'WebCore::InspectorInstrumentation::handleTouchEvent(WebCore::Frame&, WebCore::Node&)'
             if (node && InspectorInstrumentation::handleTouchEvent(m_frame, *node))
                                                                                  ^
../../Source/WebCore/page/EventHandler.cpp:3869:82: note: candidate is:
In file included from ../../Source/WebCore/page/EventHandler.cpp:63:0:
../../Source/WebCore/inspector/InspectorInstrumentation.h:746:17: note: static bool WebCore::InspectorInstrumentation::handleTouchEvent(WebCore::Page&, WebCore::Node&)
 inline bool InspectorInstrumentation::handleTouchEvent(Page& page, Node& node)
                 ^
../../Source/WebCore/inspector/InspectorInstrumentation.h:746:17: note:   no known conversion for argument 1 from 'WebCore::Frame' to 'WebCore::Page&'
Comment 5 Brian Burg 2014-12-30 05:38:05 PST
Created attachment 243822 [details]
Fix EFL/GTK
Comment 6 WebKit Commit Bot 2014-12-30 05:39:35 PST
Attachment 243822 [details] did not pass style-queue:


ERROR: Source/WebCore/inspector/InspectorInstrumentation.cpp:46:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 42 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Brian Burg 2015-01-05 13:30:32 PST
Committed r177925: <http://trac.webkit.org/changeset/177925>
Comment 8 Csaba Osztrogon√°c 2015-01-05 14:51:54 PST
(In reply to comment #7)
> Committed r177925: <http://trac.webkit.org/changeset/177925>

It broke the Apple Windows build:
     1>WebKit.exp : error LNK2001: unresolved external symbol "class WebCore::InstrumentingAgents * __cdecl WebCore::instrumentationForPage(class WebCore::Page *)" (?instrumentationForPage@WebCore@@YAPAVInstrumentingAgents@1@PAVPage@1@@Z)
     1>WebKit.exp : error LNK2001: unresolved external symbol "private: static class WebCore::InstrumentingAgents * __cdecl WebCore::InspectorInstrumentation::instrumentingAgentsForPage(class WebCore::Page *)" (?instrumentingAgentsForPage@InspectorInstrumentation@WebCore@@CAPAVInstrumentingAgents@2@PAVPage@2@@Z)
     1>C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\bin32\WebKit.dll : fatal error LNK1120: 2 unresolved externals
     1>Done Building Project "C:\cygwin\home\buildbot\slave\win-release\build\Source\WebKit\WebKit.vcxproj\WebKit\WebKit.vcxproj" (Build target(s)) -- FAILED.
Comment 9 Brent Fulgham 2015-01-05 16:54:16 PST
A small build fix for Windows was needed:

http://trac.webkit.org/changeset/177940

Looks like you also found a crufty symbol export we don't need anymore! :-)