Bug 113341

Summary: Web Inspector: [Mac] Support dock-to-right
Product: WebKit Reporter: Timothy Hatcher <timothy>
Component: Web Inspector (Deprecated)Assignee: Timothy Hatcher <timothy>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, apavlov, cmarcelo, dglazkov, gns, graouts, gtk-ews, gyuyoung.kim, joepeck, keishi, loislo, menard, peter+ews, pfeldman, philn, pmuellr, rakuco, rego+ews, rniwa, vsevik, web-inspector-bugs, webkit-ews, webkit.review.bot, xan.lopez, yurys, zan
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Macintosh   
OS: All   
Attachments:
Description Flags
Proposed Change
timothy: commit-queue-
Proposed Change (Take 2)
timothy: commit-queue-
Archive of layout-test-results from gce-cr-linux-03 for chromium-linux-x86_64
none
Proposed Change (Take 3)
eflews.bot: commit-queue-
Proposed Change (Take 4)
gtk-ews: commit-queue-
Proposed Change (Take 5)
none
Proposed Change (Take 6) joepeck: review+

Description Timothy Hatcher 2013-03-26 14:29:41 PDT 
It is time to support dock to right on Mac.
Comment 1 Timothy Hatcher 2013-03-26 14:30:01 PDT 
<rdar://problem/10368152>
Comment 2 Timothy Hatcher 2013-03-26 14:40:28 PDT 
Created attachment 195166 [details]
Proposed Change

This is likely going to brake some builds. Throwing to the EWS to find out what needs tweaked. The changes should be all ready to review still.
Comment 3 WebKit Review Bot 2013-03-26 14:45:25 PDT 
Attachment 195166 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/inspector/InspectorFrontendClient.h', u'Source/WebCore/inspector/InspectorFrontendClientLocal.cpp', u'Source/WebCore/inspector/InspectorFrontendClientLocal.h', u'Source/WebCore/inspector/InspectorFrontendHost.cpp', u'Source/WebCore/inspector/InspectorFrontendHost.h', u'Source/WebCore/inspector/InspectorFrontendHost.idl', u'Source/WebCore/inspector/front-end/InspectorFrontendHostStub.js', u'Source/WebCore/inspector/front-end/externs.js', u'Source/WebCore/testing/Internals.cpp', u'Source/WebKit/mac/ChangeLog', u'Source/WebKit/mac/Resources/Dock.pdf', u'Source/WebKit/mac/WebCoreSupport/WebInspectorClient.h', u'Source/WebKit/mac/WebCoreSupport/WebInspectorClient.mm', u'Source/WebKit/mac/WebInspector/WebInspectorFrontend.mm', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/Resources/Dock.pdf', u'Source/WebKit2/Resources/DockBottom.pdf', u'Source/WebKit2/Resources/DockRight.pdf', u'Source/WebKit2/Shared/WebPreferencesStore.h', u'Source/WebKit2/UIProcess/WebInspectorProxy.cpp', u'Source/WebKit2/UIProcess/WebInspectorProxy.h', u'Source/WebKit2/UIProcess/WebInspectorProxy.messages.in', u'Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm', u'Source/WebKit2/WebKit2.xcodeproj/project.pbxproj', u'Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorFrontendClient.cpp', u'Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorFrontendClient.h', u'Source/WebKit2/WebProcess/WebPage/WebInspector.cpp', u'Source/WebKit2/WebProcess/WebPage/WebInspector.h', u'Source/WebKit2/WebProcess/WebPage/WebInspector.messages.in']" exit_code: 1
Source/WebCore/inspector/InspectorFrontendHost.h:68:  The parameter name "width" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Early Warning System Bot 2013-03-26 14:53:51 PDT 
Comment on attachment 195166 [details]
Proposed Change

Attachment 195166 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17251643
Comment 5 WebKit Review Bot 2013-03-26 15:00:40 PDT 
Comment on attachment 195166 [details]
Proposed Change

Attachment 195166 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17296304
Comment 6 WebKit Review Bot 2013-03-26 15:01:28 PDT 
Comment on attachment 195166 [details]
Proposed Change

Attachment 195166 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17290734
Comment 7 Early Warning System Bot 2013-03-26 15:06:34 PDT 
Comment on attachment 195166 [details]
Proposed Change

Attachment 195166 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17334092
Comment 8 kov's GTK+ EWS bot 2013-03-26 15:08:37 PDT 
Comment on attachment 195166 [details]
Proposed Change

Attachment 195166 [details] did not pass gtk-ews (gtk):
Output: http://webkit-commit-queue.appspot.com/results/17132987
Comment 9 Peter Beverloo (cr-android ews) 2013-03-26 15:20:29 PDT 
Comment on attachment 195166 [details]
Proposed Change

Attachment 195166 [details] did not pass cr-android-ews (chromium-android):
Output: http://webkit-commit-queue.appspot.com/results/17246582
Comment 10 Joseph Pecoraro 2013-03-26 15:39:09 PDT 
Comment on attachment 195166 [details]
Proposed Change

View in context: https://bugs.webkit.org/attachment.cgi?id=195166&action=review

Looks good to me assuming other builds get fixed. There will need to be a WK2 Owner to r+ the WK2 parts.

> Source/WebCore/inspector/InspectorFrontendClientLocal.h:108
> +    void setAttachedWindow(DockSide);

This will break Qt WK1, which also extends InspectorFrontendClientLocal:
WebKit/qt/WebCoreSupport/InspectorClientQt.cpp

The changes would be minor.
Comment 11 Timothy Hatcher 2013-03-26 15:47:45 PDT 
Created attachment 195176 [details]
Proposed Change (Take 2)

With some build fixes.
Comment 12 WebKit Review Bot 2013-03-26 15:50:28 PDT 
Attachment 195176 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/inspector/InspectorFrontendClient.h', u'Source/WebCore/inspector/InspectorFrontendClientLocal.cpp', u'Source/WebCore/inspector/InspectorFrontendClientLocal.h', u'Source/WebCore/inspector/InspectorFrontendHost.cpp', u'Source/WebCore/inspector/InspectorFrontendHost.h', u'Source/WebCore/inspector/InspectorFrontendHost.idl', u'Source/WebCore/inspector/front-end/InspectorFrontendHostStub.js', u'Source/WebCore/inspector/front-end/externs.js', u'Source/WebCore/testing/Internals.cpp', u'Source/WebKit/chromium/src/InspectorFrontendClientImpl.cpp', u'Source/WebKit/chromium/src/InspectorFrontendClientImpl.h', u'Source/WebKit/efl/WebCoreSupport/InspectorClientEfl.cpp', u'Source/WebKit/efl/WebCoreSupport/InspectorClientEfl.h', u'Source/WebKit/gtk/WebCoreSupport/InspectorClientGtk.cpp', u'Source/WebKit/gtk/WebCoreSupport/InspectorClientGtk.h', u'Source/WebKit/mac/ChangeLog', u'Source/WebKit/mac/Resources/Dock.pdf', u'Source/WebKit/mac/WebCoreSupport/WebInspectorClient.h', u'Source/WebKit/mac/WebCoreSupport/WebInspectorClient.mm', u'Source/WebKit/mac/WebInspector/WebInspectorFrontend.mm', u'Source/WebKit/qt/WebCoreSupport/InspectorClientQt.cpp', u'Source/WebKit/qt/WebCoreSupport/InspectorClientQt.h', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/Resources/Dock.pdf', u'Source/WebKit2/Resources/DockBottom.pdf', u'Source/WebKit2/Resources/DockRight.pdf', u'Source/WebKit2/Shared/WebPreferencesStore.h', u'Source/WebKit2/UIProcess/WebInspectorProxy.cpp', u'Source/WebKit2/UIProcess/WebInspectorProxy.h', u'Source/WebKit2/UIProcess/WebInspectorProxy.messages.in', u'Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm', u'Source/WebKit2/WebKit2.xcodeproj/project.pbxproj', u'Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorFrontendClient.cpp', u'Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorFrontendClient.h', u'Source/WebKit2/WebProcess/WebPage/WebInspector.cpp', u'Source/WebKit2/WebProcess/WebPage/WebInspector.h', u'Source/WebKit2/WebProcess/WebPage/WebInspector.messages.in']" exit_code: 1
Source/WebCore/inspector/InspectorFrontendHost.h:68:  The parameter name "width" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 35 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 Early Warning System Bot 2013-03-26 16:00:04 PDT 
Comment on attachment 195176 [details]
Proposed Change (Take 2)

Attachment 195176 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17329187
Comment 14 Build Bot 2013-03-26 16:12:56 PDT 
Comment on attachment 195176 [details]
Proposed Change (Take 2)

Attachment 195176 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17232829
Comment 15 EFL EWS Bot 2013-03-26 16:25:22 PDT 
Comment on attachment 195176 [details]
Proposed Change (Take 2)

Attachment 195176 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17141970
Comment 16 WebKit Review Bot 2013-03-26 17:07:31 PDT 
Comment on attachment 195176 [details]
Proposed Change (Take 2)

Attachment 195176 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17258543

New failing tests:
fast/js/dfg-inline-resolve.html
Comment 17 WebKit Review Bot 2013-03-26 17:07:40 PDT 
Created attachment 195192 [details]
Archive of layout-test-results from gce-cr-linux-03 for chromium-linux-x86_64

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-03  Port: chromium-linux-x86_64  Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04
Comment 18 Timothy Hatcher 2013-03-27 06:34:55 PDT 
Created attachment 195302 [details]
Proposed Change (Take 3)
Comment 19 WebKit Review Bot 2013-03-27 06:38:48 PDT 
Attachment 195302 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/inspector/InspectorFrontendClient.h', u'Source/WebCore/inspector/InspectorFrontendClientLocal.cpp', u'Source/WebCore/inspector/InspectorFrontendClientLocal.h', u'Source/WebCore/inspector/InspectorFrontendHost.cpp', u'Source/WebCore/inspector/InspectorFrontendHost.h', u'Source/WebCore/inspector/InspectorFrontendHost.idl', u'Source/WebCore/inspector/front-end/InspectorFrontendHostStub.js', u'Source/WebCore/inspector/front-end/externs.js', u'Source/WebCore/testing/Internals.cpp', u'Source/WebKit/chromium/src/InspectorFrontendClientImpl.cpp', u'Source/WebKit/chromium/src/InspectorFrontendClientImpl.h', u'Source/WebKit/efl/WebCoreSupport/InspectorClientEfl.cpp', u'Source/WebKit/efl/WebCoreSupport/InspectorClientEfl.h', u'Source/WebKit/gtk/WebCoreSupport/InspectorClientGtk.cpp', u'Source/WebKit/gtk/WebCoreSupport/InspectorClientGtk.h', u'Source/WebKit/mac/ChangeLog', u'Source/WebKit/mac/Resources/Dock.pdf', u'Source/WebKit/mac/WebCoreSupport/WebInspectorClient.h', u'Source/WebKit/mac/WebCoreSupport/WebInspectorClient.mm', u'Source/WebKit/mac/WebInspector/WebInspectorFrontend.mm', u'Source/WebKit/qt/WebCoreSupport/InspectorClientQt.cpp', u'Source/WebKit/qt/WebCoreSupport/InspectorClientQt.h', u'Source/WebKit/win/WebCoreSupport/WebInspectorClient.cpp', u'Source/WebKit/win/WebCoreSupport/WebInspectorClient.h', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/Resources/Dock.pdf', u'Source/WebKit2/Resources/DockBottom.pdf', u'Source/WebKit2/Resources/DockRight.pdf', u'Source/WebKit2/Shared/WebPreferencesStore.h', u'Source/WebKit2/UIProcess/WebInspectorProxy.cpp', u'Source/WebKit2/UIProcess/WebInspectorProxy.h', u'Source/WebKit2/UIProcess/WebInspectorProxy.messages.in', u'Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm', u'Source/WebKit2/UIProcess/qt/WebInspectorProxyQt.cpp', u'Source/WebKit2/WebKit2.xcodeproj/project.pbxproj', u'Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorFrontendClient.cpp', u'Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorFrontendClient.h', u'Source/WebKit2/WebProcess/WebPage/WebInspector.cpp', u'Source/WebKit2/WebProcess/WebPage/WebInspector.h', u'Source/WebKit2/WebProcess/WebPage/WebInspector.messages.in']" exit_code: 1
Source/WebKit/win/WebCoreSupport/WebInspectorClient.cpp:47:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 38 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 20 EFL EWS Bot 2013-03-27 06:48:32 PDT 
Comment on attachment 195302 [details]
Proposed Change (Take 3)

Attachment 195302 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17288664
Comment 21 kov's GTK+ EWS bot 2013-03-27 06:51:20 PDT 
Comment on attachment 195302 [details]
Proposed Change (Take 3)

Attachment 195302 [details] did not pass gtk-ews (gtk):
Output: http://webkit-commit-queue.appspot.com/results/17327165
Comment 22 Timothy Hatcher 2013-03-27 07:35:59 PDT 
Created attachment 195319 [details]
Proposed Change (Take 4)
Comment 23 kov's GTK+ EWS bot 2013-03-27 08:07:56 PDT 
Comment on attachment 195319 [details]
Proposed Change (Take 4)

Attachment 195319 [details] did not pass gtk-ews (gtk):
Output: http://webkit-commit-queue.appspot.com/results/17340034
Comment 24 Timothy Hatcher 2013-03-27 08:25:21 PDT 
Can someone from GTK and EFL ports explain the build failures? I'm exporting __ZN7WebCore28InspectorFrontendClientLocal25changeAttachedWindowWidthEj and it works fine on other ports.
Comment 25 Timothy Hatcher 2013-03-27 09:28:37 PDT 
Looks like GTK is the only issue now. Windows TBD.
Comment 26 Build Bot 2013-03-27 10:34:04 PDT 
Comment on attachment 195319 [details]
Proposed Change (Take 4)

Attachment 195319 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17253575
Comment 27 Timothy Hatcher 2013-03-27 10:57:44 PDT 
Created attachment 195356 [details]
Proposed Change (Take 5)

Win build fix. Still need advice on GTK's build failure.
Comment 28 Zan Dobersek 2013-03-27 12:02:44 PDT 
The GTK build is failing due to the undefined InspectorFrontendClientLocal::changeAttachedWindowWidth, to fix that you should list the related symbol (__ZN7WebCore28InspectorFrontendClientLocal25changeAttachedWindowWidthEj) in Source/autotools/symbols.filter.

Thanks for not breaking the GTK build!
Comment 29 Timothy Hatcher 2013-03-27 12:13:05 PDT 
Created attachment 195375 [details]
Proposed Change (Take 6)
Comment 30 Anders Carlsson 2013-03-27 13:15:19 PDT 
Comment on attachment 195375 [details]
Proposed Change (Take 6)

WebKit 2 parts look good.
Comment 31 Joseph Pecoraro 2013-03-27 13:17:40 PDT 
Comment on attachment 195375 [details]
Proposed Change (Take 6)

r=me, looks like you might be missing a few ChangeLog entires for the recent port build fixes.