Last Comment Bug 778501 - Clean up nsPresContext
: Clean up nsPresContext
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla17
Assigned To: David Zbarsky (:dzbarsky)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-28 18:03 PDT by David Zbarsky (:dzbarsky)
Modified: 2012-07-31 06:14 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part 1: mLanguage should be an nsCOMPtr (2.74 KB, patch)
2012-07-28 18:10 PDT, David Zbarsky (:dzbarsky)
roc: review+
Details | Diff | Splinter Review
PArt 2: mEventManager should be nsRefPtr (3.50 KB, patch)
2012-07-28 18:15 PDT, David Zbarsky (:dzbarsky)
roc: review+
Details | Diff | Splinter Review
Part 3: Clean up includes (7.60 KB, patch)
2012-07-28 18:52 PDT, David Zbarsky (:dzbarsky)
no flags Details | Diff | Splinter Review
Part 3: Clean up includes (9.38 KB, patch)
2012-07-28 19:14 PDT, David Zbarsky (:dzbarsky)
roc: review+
Details | Diff | Splinter Review
Part 4: DeviceContext should be nsRefPtr (4.49 KB, patch)
2012-07-29 18:44 PDT, David Zbarsky (:dzbarsky)
roc: review+
Details | Diff | Splinter Review

Description David Zbarsky (:dzbarsky) 2012-07-28 18:03:43 PDT

    
Comment 1 David Zbarsky (:dzbarsky) 2012-07-28 18:10:14 PDT
Created attachment 646929 [details] [diff] [review]
Part 1: mLanguage should be an nsCOMPtr
Comment 2 David Zbarsky (:dzbarsky) 2012-07-28 18:15:48 PDT
Created attachment 646930 [details] [diff] [review]
PArt 2: mEventManager should be nsRefPtr
Comment 3 David Zbarsky (:dzbarsky) 2012-07-28 18:52:27 PDT
Created attachment 646932 [details] [diff] [review]
Part 3: Clean up includes
Comment 4 David Zbarsky (:dzbarsky) 2012-07-28 19:14:53 PDT
Created attachment 646933 [details] [diff] [review]
Part 3: Clean up includes
Comment 5 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-28 20:04:33 PDT
Comment on attachment 646933 [details] [diff] [review]
Part 3: Clean up includes

Review of attachment 646933 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with that

::: layout/base/nsPresContext.cpp
@@ +1547,5 @@
> +#ifdef _IMPL_NS_LAYOUT
> +  return BidiEnabledInternal();
> +#else
> +  return BidiEnabledExternal();
> +#endif

_IMPL_NS_LAYOUT is always defined in this file, so this defeats the whole point of BidiEnabledExternal. I think you should keep BidiEnabled as it was, inline in nsPresContext.h.
Comment 6 David Zbarsky (:dzbarsky) 2012-07-29 18:44:08 PDT
Created attachment 647039 [details] [diff] [review]
Part 4: DeviceContext should be nsRefPtr

Note You need to log in before you can comment on or make changes to this bug.