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.
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 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 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.
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.
2008-03-02 13:18 PST, Darin Adler
2008-03-03 10:38 PST, Darin Adler
2008-03-03 10:48 PST, Darin Adler
2008-03-04 03:25 PST, Darin Adler
2008-03-04 03:59 PST, Darin Adler
2008-03-04 04:11 PST, Darin Adler
2008-03-04 04:16 PST, Darin Adler
2008-03-04 04:33 PST, Darin Adler
2008-03-04 04:42 PST, Darin Adler
2008-03-04 04:52 PST, Darin Adler
2008-03-04 05:01 PST, Darin Adler
2008-03-04 07:41 PST, Darin Adler
2008-03-04 08:54 PST, Darin Adler
2008-03-07 19:41 PST, Darin Adler
2008-03-08 07:17 PST, Darin Adler
2008-03-08 07:29 PST, Darin Adler
2008-03-10 22:13 PDT, Darin Adler
2008-03-11 12:34 PDT, Darin Adler
2008-03-11 14:34 PDT, Darin Adler
2008-03-11 20:35 PDT, Darin Adler
2008-03-12 10:03 PDT, Darin Adler