|
Description
Darin Adler
2008-03-02 12:42:40 PST
Created attachment 19485 [details]
eliminate the WebCore -> WebKit half of the bridge
Comment on attachment 19485 [details]
eliminate the WebCore -> WebKit half of the bridge
There's a whole lotta patch here! Any chance we can get this broken up and reviewed/landed incrementally?
(In reply to comment #2) > There's a whole lotta patch here! Any chance we can get this broken up and > reviewed/landed incrementally? Sure, it could be broken up into 2 pieces, or 10, or 30. Created attachment 19496 [details]
smaller piece for Adam to review
Comment on attachment 19496 [details]
smaller piece for Adam to review
r=me
Comment on attachment 19496 [details]
smaller piece for Adam to review
Landed.
Created attachment 19497 [details]
work in progress (ready to land, but needs to be broken in pieces for review)
Created attachment 19515 [details]
patch for just issuePasteCommand
Created attachment 19517 [details]
patch for just dashboardRegionsChanged
Created attachment 19518 [details]
patch moving code from the bridge to frame loader client on the WebKit side
Created attachment 19519 [details]
patch for just window method
Created attachment 19520 [details]
patch for just installInFrame method
Created attachment 19521 [details]
patch for scrollOverflow method
Created attachment 19522 [details]
patch for createFrameView method
Created attachment 19523 [details]
patch for reapplyStyles method
Comment on attachment 19515 [details]
patch for just issuePasteCommand
r=me
Comment on attachment 19517 [details]
patch for just dashboardRegionsChanged
r=me
Comment on attachment 19518 [details]
patch moving code from the bridge to frame loader client on the WebKit side
This patch seems to be missing the removal of the implementation of -[WebFrameBridge windowObjectCleared]
Other than that, r=me
Comment on attachment 19519 [details]
patch for just window method
r=me
Comment on attachment 19520 [details]
patch for just installInFrame method
r=me
Comment on attachment 19521 [details]
patch for scrollOverflow method
r=me
Comment on attachment 19522 [details]
patch for createFrameView method
r=me
Comment on attachment 19523 [details]
patch for reapplyStyles method
r=me
Comment on attachment 19515 [details]
patch for just issuePasteCommand
Landed.
Comment on attachment 19496 [details]
smaller piece for Adam to review
Landed.
Comment on attachment 19517 [details]
patch for just dashboardRegionsChanged
Landed.
Comment on attachment 19518 [details]
patch moving code from the bridge to frame loader client on the WebKit side
Landed.
Comment on attachment 19519 [details]
patch for just window method
Landed.
Comment on attachment 19520 [details]
patch for just installInFrame method
Landed.
Comment on attachment 19521 [details]
patch for scrollOverflow method
Landed.
Comment on attachment 19522 [details]
patch for createFrameView method
Landed.
Comment on attachment 19523 [details]
patch for reapplyStyles method
Landed.
Created attachment 19524 [details]
work in progress
Created attachment 19525 [details]
patch for runOpenPanel
Comment on attachment 19525 [details]
patch for runOpenPanel
+void ChromeClient::runOpenPanel(PassRefPtr<FileChooser>)
+{
+}
Why is this empty implementation needed in ChromeMac.mm? Can't you make this method declaration pure virtual and just have WebChromeClient implement it?
r=me regardless
(In reply to comment #35) > Why is this empty implementation needed in ChromeMac.mm? Can't you make this > method declaration pure virtual and just have WebChromeClient implement it? I could, but then I'd have to add code to the "SVG empty clients". I'm really not sure what the best design for this is going forward, but for now I'm not going to make this pure virtual. Comment on attachment 19525 [details]
patch for runOpenPanel
Clearing review flag since I committed revision 30875.
Created attachment 19606 [details]
patch for custom highlight calls
Comment on attachment 19606 [details]
patch for custom highlight calls
Committed revision 30900.
Created attachment 19609 [details]
patch that moves Java applet loading off the bridge
Created attachment 19610 [details]
patch that moves keyboard mode off the bridge
Comment on attachment 19609 [details]
patch that moves Java applet loading off the bridge
I am really not sure what is the correct possition for the * here, but since the following line has it next to the type, also for an ObjC type, it would be nice to be consistent.
#if PLATFORM(MAC)
+ virtual jobject javaApplet(NSView *) { return 0; }
virtual NSCachedURLResponse* willCacheResponse(DocumentLoader*, unsigned long identifier, NSCachedURLResponse*) const = 0;
r=me
Comment on attachment 19610 [details]
patch that moves keyboard mode off the bridge
I am not too crazy about having ChromeClient implementations in ChromeMac.mm, but I assume it is to get around having to implement it for the SVGImageEmptyClient. If that is the case, we should have a new file, perhaps ChromeClientMac.mm, to contain these base implementations.
Could you also explain in the changelog why it is okay to remove
- ASSERT_MAIN_THREAD();
from the finalize method of WebFrameBridge.
r=me
(In reply to comment #43) > I am not too crazy about having ChromeClient implementations in ChromeMac.mm, > but I assume it is to get around having to implement it for the > SVGImageEmptyClient. Sure, that's right. Not just for SVGImage, but in general to have a policy of default behavior rather than not compiling if you choose not to implement one of the client functions. I'm not completely sure whether pure virtual functions or functions with a default implementation are best. > If that is the case, we should have a new file, perhaps > ChromeClientMac.mm, to contain these base implementations. Sure, I will create a new file. > Could you also explain in the changelog why it is okay to remove > - ASSERT_MAIN_THREAD(); > from the finalize method of WebFrameBridge. Yes. Comment on attachment 19609 [details]
patch that moves Java applet loading off the bridge
Committed revision 30935.
Comment on attachment 19610 [details]
patch that moves keyboard mode off the bridge
Committed revision 30939.
Created attachment 19656 [details]
patch that removes the rest of the WebKit -> WebCore bridge
Created attachment 19674 [details]
patch that removes use of the bridge to go from an NSView to a WebCore::Frame
Created attachment 19677 [details]
revised patch that removes use of the bridge to go from an NSView to a WebCore::Frame
Comment on attachment 19656 [details]
patch that removes the rest of the WebKit -> WebCore bridge
Committed revision 30975.
Comment on attachment 19677 [details]
revised patch that removes use of the bridge to go from an NSView to a WebCore::Frame
r=me
Comment on attachment 19677 [details]
revised patch that removes use of the bridge to go from an NSView to a WebCore::Frame
Committed revision 30976.
Created attachment 19693 [details]
patch that removes every last use of the bridge in WebCore
Comment on attachment 19693 [details]
patch that removes every last use of the bridge in WebCore
r=me. YEAH!
Comment on attachment 19693 [details]
patch that removes every last use of the bridge in WebCore
Committed revision 30980.
Created attachment 19700 [details]
patch that drives a stake through the heart of the bridge
Comment on attachment 19700 [details]
patch that drives a stake through the heart of the bridge
r=me
I can't land this until I get home tonight, because I saw at least one crash running regression tests. It should be trivial to fix. So Hyatt, feel free to land your patch. I'll merge with it. Committed revision 31014. |