Summary: | [BlackBerry] Upstream GLES2Context.{h, cpp} | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Leo Yang <leo.yang> | ||||||||
Component: | New Bugs | Assignee: | Leo Yang <leo.yang> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | charles.wei, rwlbuis, staikos, tonikitoo, yong.li.webkit, zimmermann | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 73144 | ||||||||||
Attachments: |
|
Description
Leo Yang
2012-02-21 22:17:12 PST
Created attachment 128155 [details]
Patch
Comment on attachment 128155 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=128155&action=review Still some things to improve. > Source/WebKit/blackberry/WebKitSupport/GLES2Context.cpp:42 > +#include "WebPage_p.h" All needed? > Source/WebKit/blackberry/WebKitSupport/GLES2Context.cpp:44 > +#include <BlackBerryPlatformWindow.h> Already included in header. > Source/WebKit/blackberry/WebKitSupport/GLES2Context.cpp:113 > + // Sniff, this is so sad. Seems a strange comment, maybe explain why it is sad > Source/WebKit/blackberry/WebKitSupport/GLES2Context.h:39 > +#include <wtf/OwnPtr.h> Is this one needed? (In reply to comment #2) > (From update of attachment 128155 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=128155&action=review > > Still some things to improve. > > > Source/WebKit/blackberry/WebKitSupport/GLES2Context.cpp:42 > > +#include "WebPage_p.h" > > All needed? > Yes. WebPagePrivate is being used in the constructor. > > Source/WebKit/blackberry/WebKitSupport/GLES2Context.cpp:44 > > +#include <BlackBerryPlatformWindow.h> > > Already included in header. I'll remove it. > > > Source/WebKit/blackberry/WebKitSupport/GLES2Context.cpp:113 > > + // Sniff, this is so sad. > > Seems a strange comment, maybe explain why it is sad > I'll make a clear comment. > > Source/WebKit/blackberry/WebKitSupport/GLES2Context.h:39 > > +#include <wtf/OwnPtr.h> > > Is this one needed? I'll remove it. Created attachment 128435 [details]
Patch v2
Created attachment 128436 [details]
Patch v3
Removing some unnecessary blank lines.
Comment on attachment 128436 [details] Patch v3 View in context: https://bugs.webkit.org/attachment.cgi?id=128436&action=review LGTM. > Source/WebKit/blackberry/ChangeLog:1 > +2012-02-22 Leo Yang <leo.yang@torchmobile.com.cn> It is 23 feb now. > Source/WebKit/blackberry/WebKitSupport/GLES2Context.cpp:60 > + if (surface) Can just do if (BackingStoreCompositingSurface* surface = compositingSurface()) > Source/WebKit/blackberry/WebKitSupport/GLES2Context.cpp:91 > + if (surface) Ditto Committed r108711: <http://trac.webkit.org/changeset/108711> Do we know why the 2 files are put in WebKitSupport rather than WebCoreSupport? |