Bug 102354

Summary: Avoid copying of ViewportArguments in computeViewportAttributes function
Product: WebKit Reporter: Mikhail Pozdnyakov <mikhail.pozdnyakov>
Component: WebCore Misc.Assignee: Mikhail Pozdnyakov <mikhail.pozdnyakov>
Status: REOPENED    
Severity: Normal CC: dbates, kenneth, noam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 102472, 102473    
Bug Blocks:    
Attachments:
Description Flags
patch
none
run EWS (don't put any flags)
buildbot: commit-queue-
patch v3 none

Mikhail Pozdnyakov
Reported 2012-11-15 01:44:57 PST
Since r134749 we do not need copying of ViewportArguments parameter in computeViewportAttributes() as we're not modifying it any more.
Attachments
patch (2.75 KB, patch)
2012-11-15 01:53 PST, Mikhail Pozdnyakov
no flags
run EWS (don't put any flags) (6.09 KB, patch)
2012-11-15 05:22 PST, Mikhail Pozdnyakov
buildbot: commit-queue-
patch v3 (7.57 KB, patch)
2012-11-15 06:24 PST, Mikhail Pozdnyakov
no flags
Mikhail Pozdnyakov
Comment 1 2012-11-15 01:53:40 PST
Kenneth Rohde Christiansen
Comment 2 2012-11-15 01:58:34 PST
Are you sure you dont need to change export symbols?
Build Bot
Comment 3 2012-11-15 02:42:32 PST
Mikhail Pozdnyakov
Comment 4 2012-11-15 05:22:33 PST
Created attachment 174411 [details] run EWS (don't put any flags)
Build Bot
Comment 5 2012-11-15 06:01:37 PST
Comment on attachment 174411 [details] run EWS (don't put any flags) Attachment 174411 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14838698
Mikhail Pozdnyakov
Comment 6 2012-11-15 06:24:38 PST
Created attachment 174420 [details] patch v3 Now bots shall be glad.
WebKit Review Bot
Comment 7 2012-11-16 00:03:30 PST
Comment on attachment 174420 [details] patch v3 Clearing flags on attachment: 174420 Committed r134908: <http://trac.webkit.org/changeset/134908>
WebKit Review Bot
Comment 8 2012-11-16 00:03:34 PST
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 9 2012-11-16 01:03:23 PST
Re-opened since this is blocked by bug 102472
Daniel Bates
Comment 10 2012-11-16 01:18:45 PST
(In reply to comment #9) > Re-opened since this is blocked by bug 102472 This change broke the Apple Windows Debug build: 12> Creating library C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\lib\DumpRenderTree.lib and object C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\lib\DumpRenderTree.exp 12>WebCoreTestSupport.lib(InternalSettings.obj) : error LNK2019: unresolved external symbol "struct WebCore::ViewportAttributes __cdecl WebCore::computeViewportAttributes(struct WebCore::ViewportArguments,int,int,int,float,class WebCore::IntSize)" (?computeViewportAttributes@WebCore@@YA?AUViewportAttributes@1@UViewportArguments@1@HHHMVIntSize@1@@Z) referenced in function "public: class WTF::String __thiscall WebCore::InternalSettings::configurationForViewport(float,int,int,int,int,int &)" (?configurationForViewport@InternalSettings@WebCore@@QAE?AVString@WTF@@MHHHHAAH@Z) <http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/58041/steps/compile-webkit/logs/stdio> After discussing this issue with Mikhail Pozdnyakov on IRC this morning, we decided to rollout this change so as to fix it offline.
Daniel Bates
Comment 11 2012-11-16 01:21:12 PST
(In reply to comment #9) > Re-opened since this is blocked by bug 102472 For completeness, I tried to fix the build in <http://trac.webkit.org/changeset/134913>, but it didn't work out :( I reverted this attempt in the patch for bug #102472.
Mikhail Pozdnyakov
Comment 12 2012-11-22 08:36:49 PST
(In reply to comment #11) > (In reply to comment #9) > > Re-opened since this is blocked by bug 102472 > > For completeness, I tried to fix the build in <http://trac.webkit.org/changeset/134913>, but it didn't work out :( I reverted this attempt in the patch for bug #102472. I've carefully checked exported symbols and did not find any reason it should cause build failure (all .def files are updated), logs say: unresolved external symbol "struct WebCore::ViewportAttributes __cdecl WebCore::computeViewportAttributes(struct WebCore::ViewportArguments,int,int,int,float,class WebCore::IntSize) which does not exist already, so I suppose the problem could have been solved by clean build triggering.
Eric Seidel (no email)
Comment 13 2013-01-04 00:43:26 PST
Comment on attachment 174372 [details] patch Cleared Kenneth Rohde Christiansen's review+ from obsolete attachment 174372 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Note You need to log in before you can comment on or make changes to this bug.