Bug 26265

Created attachment 31106 [details] patch set 1

(In reply to comment #0) > This is a spin-off of bug 23296, patch part 8: > + * platform/android/SystemTimeAndroid.cpp: Added. > + * platform/android/TextBoundaries.cpp: Added. > + * platform/android/TextBreakIteratorInternalICU.cpp: Added. > + * platform/android/WidgetAndroid.cpp: Added. > > Original comments: > > (From update of attachment 30004 [details] [review] [review]) > I'm surprised we have any time code in WebCore? Isn't that all down in WTF? > Time code seems necessary to link WebCore/history/PageCache.cpp, there is a similar file, WebCore/platform/win/SystemTimeWin.cpp. > Why does andriod need its own different ICU code? TextBoundaries.cpp? > You are right, these files should under platform/text, removed. Will add them in a separate CL. > IntRect() is the same as 54 return IntRect(0, 0, 0, 0); > > Isn't there an easier way to get the FrameView than your manual crawl? > Only function similar is Widget::root(), but not the exact same. ScrollView* Widget::root() const { const Widget* top = this; while (top->parent()) top = top->parent(); if (top->isFrameView()) return const_cast<ScrollView*>(static_cast<const ScrollView*>(top)); return 0; } root() directly goes to the root Widget, but in screenWidth(), it searches the first FrameView ancestor. I don't know if it is possible to have a non-FrameView ancestor that's not a root, but both code are different. (it will be great if that's true). > r- for the nits and questions. >

Comment on attachment 31106 [details] patch set 1 This probably should have a notImplemented() call: + // Needed for PageCache, which we currently have disabled. + return 0.0F; I'm also surprised to see an F there instead of an 'f'. I didn't even know F was valid. + // FIXME: use m_frame instead? + if (!platformWidget()) + return IntRect(0, 0, 0, 0); IntRect() is the same Looks fine. Please fix those nits when landing.

Landed in @r44586.