Bug 46771 - Rename chromium's GraphicsContext3D.cpp to match others.
Summary: Rename chromium's GraphicsContext3D.cpp to match others.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-28 18:00 PDT by Alexey Marinichev
Modified: 2010-09-30 00:08 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.60 KB, patch)
2010-09-28 18:23 PDT, Alexey Marinichev
no flags Details | Formatted Diff | Diff
Patch (1.53 KB, patch)
2010-09-29 11:25 PDT, Alexey Marinichev
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Marinichev 2010-09-28 18:00:55 PDT
There are two of those in the webkit tree:

  WebCore/platform/graphics/GraphicsContext3D.cpp, which implements functions shared by all implementations,

and

  WebKit/chromium/src/GraphicsContext3D.cpp, which implements chromium's version.

It would be more consistent and easier to follow, if the first one was renamed to, say, GraphicsContext3DCommon.cpp, and the second to GraphicsContext3DChromium.cpp.
Comment 1 Alexey Marinichev 2010-09-28 18:23:22 PDT
Created attachment 69148 [details]
Patch
Comment 2 Kenneth Russell 2010-09-28 18:35:41 PDT
Comment on attachment 69148 [details]
Patch

You need to take into consideration that platform/graphics/GraphicsContext3D.cpp is currently compiled in to all platforms. You will minimally need to also modify WebCore/WebCore.xcodeproj/project.pbxproj . I don't know whether you need to strictly make this modification in Xcode or whether you can do it in a text editor. You should also do a recursive grep on all flat files in the WebKit workspace to see where else this file name is referenced. Since the bots apparently can't build on Mac unless you're a WebKit committer I strongly suggest testing your build modifications on Mac OS X -- or your reviewer will need to do so for safety.
Comment 3 Chris Marrin 2010-09-28 19:42:39 PDT
Comment on attachment 69148 [details]
Patch

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

> WebCore/ChangeLog:10
> +        * platform/graphics/GraphicsContext3DCommon.cpp: Renamed from WebCore/platform/graphics/GraphicsContext3D.cpp.

I'm very much against this renaming. We have many places where we use the current naming convention: GraphicsLayer, GraphicsContext, FloatRect, and virtually every other "common" file in platoform/graphics, as well as other platform specific code throughout the tree. Yes, the Chromium specific version should be renamed, but please do not change the name of the above file
Comment 4 Alexey Marinichev 2010-09-29 11:25:15 PDT
Ouch.  Won't touch WebCore.
Comment 5 Alexey Marinichev 2010-09-29 11:25:37 PDT
Created attachment 69229 [details]
Patch
Comment 6 Kenneth Russell 2010-09-29 13:25:53 PDT
Comment on attachment 69229 [details]
Patch

Looks fine. Assuming you've built this locally to test.
Comment 7 WebKit Commit Bot 2010-09-30 00:08:46 PDT
Comment on attachment 69229 [details]
Patch

Clearing flags on attachment: 69229

Committed r68759: <http://trac.webkit.org/changeset/68759>
Comment 8 WebKit Commit Bot 2010-09-30 00:08:51 PDT
All reviewed patches have been landed.  Closing bug.